Bug 718238 - Part 0: Cleanup and basic improvements. r=nalexander
authorRichard Newman <rnewman@mozilla.com>
Thu, 23 Feb 2012 08:14:05 -0800
changeset 87544 95675530bf288aea0b3e0dad160b01bb640754d7
parent 87543 5e18c023268af4dc7fb1778f6951dacfeb714cf0
child 87545 549c09c28b9402e91709c3f17c981b1a1693e8c2
push id22130
push userrnewman@mozilla.com
push dateFri, 24 Feb 2012 02:35:54 +0000
treeherdermozilla-central@d23600a1d4a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander
bugs718238
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 718238 - Part 0: Cleanup and basic improvements. r=nalexander
mobile/android/base/sync/CryptoRecord.java
mobile/android/base/sync/repositories/RepositorySession.java
mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java
mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java
mobile/android/base/sync/repositories/android/BrowserContract.java
mobile/android/base/sync/repositories/android/RepoUtils.java
mobile/android/base/sync/repositories/domain/BookmarkRecord.java
--- a/mobile/android/base/sync/CryptoRecord.java
+++ b/mobile/android/base/sync/CryptoRecord.java
@@ -171,17 +171,17 @@ public class CryptoRecord extends Record
     String collection          = (String) jsonRecord.get(KEY_COLLECTION);
     ExtendedJSONObject payload = jsonRecord.getJSONObject(KEY_PAYLOAD);
     CryptoRecord record = new CryptoRecord(payload);
     record.guid         = id;
     record.collection   = collection;
     if (jsonRecord.containsKey(KEY_MODIFIED)) {
       record.lastModified = jsonRecord.getTimestamp(KEY_MODIFIED);
     }
-    if (jsonRecord.containsKey(KEY_SORTINDEX )) {
+    if (jsonRecord.containsKey(KEY_SORTINDEX)) {
       record.sortIndex = jsonRecord.getLong(KEY_SORTINDEX);
     }
     // TODO: deleted?
     return record;
   }
 
   public void setKeyBundle(KeyBundle bundle) {
     this.keyBundle = bundle;
--- a/mobile/android/base/sync/repositories/RepositorySession.java
+++ b/mobile/android/base/sync/repositories/RepositorySession.java
@@ -45,18 +45,16 @@ import org.mozilla.gecko.sync.Logger;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFetchRecordsDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionGuidsSinceDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionStoreDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionWipeDelegate;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
-import android.util.Log;
-
 /**
  * A RepositorySession is created and used thusly:
  *
  * * Construct, with a reference to its parent Repository, by calling
  *   Repository.createSession().
  * * Populate with saved information by calling unbundle().
  * * Begin a sync by calling begin().
  * * Perform operations such as fetchSince() and store().
@@ -71,20 +69,16 @@ public abstract class RepositorySession 
     UNSTARTED,
     ACTIVE,
     ABORTED,
     DONE
   }
 
   private static final String LOG_TAG = "RepositorySession";
 
-  private static void error(String message) {
-    Logger.error(LOG_TAG, message);
-  }
-
   protected static void trace(String message) {
     Logger.trace(LOG_TAG, message);
   }
 
   protected SessionStatus status = SessionStatus.UNSTARTED;
   protected Repository repository;
   protected RepositorySessionStoreDelegate delegate;
 
@@ -136,30 +130,30 @@ public abstract class RepositorySession 
    * * Call storeDone to notify the session that no more items are forthcoming.
    * * The store delegate will be notified of error or completion.
    *
    * This arrangement of calls allows for batching at the session level.
    *
    * Store success calls are not guaranteed.
    */
   public void setStoreDelegate(RepositorySessionStoreDelegate delegate) {
-    Log.d(LOG_TAG, "Setting store delegate to " + delegate);
+    Logger.debug(LOG_TAG, "Setting store delegate to " + delegate);
     this.delegate = delegate;
   }
   public abstract void store(Record record) throws NoStoreDelegateException;
 
   public void storeDone() {
     // Our default behavior will be to assume that the Runnable is
     // executed as soon as all the stores synchronously finish, so
     // our end timestamp can just be… now.
     storeDone(now());
   }
 
   public void storeDone(final long end) {
-    Log.d(LOG_TAG, "Scheduling onStoreCompleted for after storing is done.");
+    Logger.debug(LOG_TAG, "Scheduling onStoreCompleted for after storing is done.");
     Runnable command = new Runnable() {
       @Override
       public void run() {
         delegate.onStoreCompleted(end);
       }
     };
     storeWorkQueue.execute(command);
   }
@@ -184,17 +178,17 @@ public abstract class RepositorySession 
    * Synchronously perform the shared work of beginning. Throws on failure.
    * @throws InvalidSessionTransitionException
    *
    */
   protected void sharedBegin() throws InvalidSessionTransitionException {
     if (this.status == SessionStatus.UNSTARTED) {
       this.status = SessionStatus.ACTIVE;
     } else {
-      error("Tried to begin() an already active or finished session");
+      Logger.error(LOG_TAG, "Tried to begin() an already active or finished session");
       throw new InvalidSessionTransitionException(null);
     }
   }
 
   public void begin(RepositorySessionBeginDelegate delegate) {
     try {
       sharedBegin();
       delegate.deferredBeginDelegate(delegateQueue).onBeginSucceeded(this);
@@ -215,21 +209,21 @@ public abstract class RepositorySession 
    *
    * The Synchronizer most likely wants to bump the bundle timestamp to be a value
    * return from a fetch call.
    *
    * @param optional
    * @return
    */
   protected RepositorySessionBundle getBundle(RepositorySessionBundle optional) {
-    System.out.println("RepositorySession.getBundle(optional).");
+    Logger.debug(LOG_TAG, "RepositorySession.getBundle(optional).");
     // Why don't we just persist the old bundle?
     RepositorySessionBundle bundle = (optional == null) ? new RepositorySessionBundle() : optional;
     bundle.put("timestamp", this.lastSyncTimestamp);
-    System.out.println("Setting bundle timestamp to " + this.lastSyncTimestamp);
+    Logger.debug(LOG_TAG, "Setting bundle timestamp to " + this.lastSyncTimestamp);
     return bundle;
   }
 
   /**
    * Just like finish(), but doesn't do any work that should only be performed
    * at the end of a successful sync, and can be called any time.
    *
    * @param delegate
@@ -239,21 +233,21 @@ public abstract class RepositorySession 
     delegate.deferredFinishDelegate(delegateQueue).onFinishSucceeded(this, this.getBundle(null));
   }
 
   public void finish(final RepositorySessionFinishDelegate delegate) {
     if (this.status == SessionStatus.ACTIVE) {
       this.status = SessionStatus.DONE;
       delegate.deferredFinishDelegate(delegateQueue).onFinishSucceeded(this, this.getBundle(null));
     } else {
-      Log.e(LOG_TAG, "Tried to finish() an unstarted or already finished session");
+      Logger.error(LOG_TAG, "Tried to finish() an unstarted or already finished session");
       Exception e = new InvalidSessionTransitionException(null);
       delegate.deferredFinishDelegate(delegateQueue).onFinishFailed(e);
     }
-    Log.i(LOG_TAG, "Shutting down work queues.");
+    Logger.info(LOG_TAG, "Shutting down work queues.");
  //   storeWorkQueue.shutdown();
  //   delegateQueue.shutdown();
   }
 
   public boolean isActive() {
     return status == SessionStatus.ACTIVE;
   }
 
@@ -306,35 +300,35 @@ public abstract class RepositorySession 
    *        The timestamp of the last retrieved set of local records.
    * @return
    *        A Record instance to apply, or null to apply nothing.
    */
   protected Record reconcileRecords(final Record remoteRecord,
                                     final Record localRecord,
                                     final long lastRemoteRetrieval,
                                     final long lastLocalRetrieval) {
-    Log.d(LOG_TAG, "Reconciling remote " + remoteRecord.guid + " against local " + localRecord.guid);
+    Logger.debug(LOG_TAG, "Reconciling remote " + remoteRecord.guid + " against local " + localRecord.guid);
 
     if (localRecord.equalPayloads(remoteRecord)) {
       if (remoteRecord.lastModified > localRecord.lastModified) {
-        Log.d(LOG_TAG, "Records are equal. No record application needed.");
+        Logger.debug(LOG_TAG, "Records are equal. No record application needed.");
         return null;
       }
 
       // Local wins.
       return null;
     }
 
     // TODO: Decide what to do based on:
     // * Which of the two records is modified;
     // * Whether they are equal or congruent;
     // * The modified times of each record (interpreted through the lens of clock skew);
     // * ...
     boolean localIsMoreRecent = localRecord.lastModified > remoteRecord.lastModified;
-    Log.d(LOG_TAG, "Local record is more recent? " + localIsMoreRecent);
+    Logger.debug(LOG_TAG, "Local record is more recent? " + localIsMoreRecent);
     Record donor = localIsMoreRecent ? localRecord : remoteRecord;
 
     // Modify the local record to match the remote record's GUID and values.
     // Preserve the local Android ID, and merge data where possible.
     // It sure would be nice if copyWithIDs didn't give a shit about androidID, mm?
     Record out = donor.copyWithIDs(remoteRecord.guid, localRecord.androidID);
 
     // We don't want to upload the record if the remote record was
@@ -353,9 +347,12 @@ public abstract class RepositorySession 
    * redundantly.
    *
    * The default implementation does nothing.
    *
    * @param record
    */
   protected synchronized void trackRecord(Record record) {
   }
+
+  protected synchronized void untrackRecord(Record record) {
+  }
 }
--- a/mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java
+++ b/mobile/android/base/sync/repositories/StoreTrackingRepositorySession.java
@@ -1,20 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.repositories;
 
+import org.mozilla.gecko.sync.Logger;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionBeginDelegate;
 import org.mozilla.gecko.sync.repositories.delegates.RepositorySessionFinishDelegate;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
-import android.util.Log;
-
 public abstract class StoreTrackingRepositorySession extends RepositorySession {
   private static final String LOG_TAG = "StoreTrackSession";
   protected StoreTracker storeTracker;
 
   protected static StoreTracker createStoreTracker() {
     return new HashSetStoreTracker();
   }
 
@@ -37,23 +36,33 @@ public abstract class StoreTrackingRepos
   }
 
   @Override
   protected synchronized void trackRecord(Record record) {
     if (this.storeTracker == null) {
       throw new IllegalStateException("Store tracker not yet initialized!");
     }
 
-    Log.d(LOG_TAG, "Tracking record " + record.guid +
-                   " (" + record.lastModified + ") to avoid re-upload.");
+    Logger.debug(LOG_TAG, "Tracking record " + record.guid +
+                           " (" + record.lastModified + ") to avoid re-upload.");
     // Future: we care about the timestamp…
     this.storeTracker.trackRecordForExclusion(record.guid);
   }
 
   @Override
+  protected synchronized void untrackRecord(Record record) {
+    if (this.storeTracker == null) {
+      throw new IllegalStateException("Store tracker not yet initialized!");
+    }
+
+    Logger.debug(LOG_TAG, "Un-tracking record " + record.guid + ".");
+    this.storeTracker.untrackStoredForExclusion(record.guid);
+  }
+
+  @Override
   public void abort(RepositorySessionFinishDelegate delegate) {
     this.storeTracker = null;
     super.abort(delegate);
   }
 
   @Override
   public void finish(RepositorySessionFinishDelegate delegate) {
     super.finish(delegate);
--- a/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/AndroidBrowserHistoryRepositorySession.java
@@ -1,45 +1,11 @@
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is Android Sync Client.
- *
- * The Initial Developer of the Original Code is
- * the Mozilla Foundation.
- * Portions created by the Initial Developer are Copyright (C) 2011
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- *   Jason Voll <jvoll@mozilla.com>
- *   Richard Newman <rnewman@mozilla.com>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either the GNU General Public License Version 2 or later (the "GPL"), or
- * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.repositories.android;
 
 import org.json.simple.JSONArray;
 import org.json.simple.JSONObject;
 import org.mozilla.gecko.sync.repositories.NullCursorException;
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.domain.HistoryRecord;
--- a/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java
+++ b/mobile/android/base/sync/repositories/android/AndroidBrowserPasswordsRepositorySession.java
@@ -1,44 +1,11 @@
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is Android Sync Client.
- *
- * The Initial Developer of the Original Code is
- * the Mozilla Foundation.
- * Portions created by the Initial Developer are Copyright (C) 2011
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s):
- * Jason Voll <jvoll@mozilla.com>
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either the GNU General Public License Version 2 or later (the "GPL"), or
- * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.sync.repositories.android;
 
 import org.mozilla.gecko.sync.repositories.Repository;
 import org.mozilla.gecko.sync.repositories.domain.PasswordRecord;
 import org.mozilla.gecko.sync.repositories.domain.Record;
 
 import android.content.Context;
--- a/mobile/android/base/sync/repositories/android/BrowserContract.java
+++ b/mobile/android/base/sync/repositories/android/BrowserContract.java
@@ -107,16 +107,18 @@ public class BrowserContract {
         public static final String TAGS_FOLDER_GUID = "tags";
         public static final String TOOLBAR_FOLDER_GUID = "toolbar";
         public static final String UNFILED_FOLDER_GUID = "unfiled";
 
         public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "bookmarks")
             .buildUpon().appendQueryParameter(PARAM_IS_SYNC, "true")
             .appendQueryParameter(PARAM_SHOW_DELETED, "true")
             .build();
+        public static final Uri PARENTS_CONTENT_URI = Uri.withAppendedPath(CONTENT_URI, "parents");
+        public static final Uri POSITIONS_CONTENT_URI = Uri.withAppendedPath(CONTENT_URI, "positions");
 
         public static final String CONTENT_TYPE = "vnd.android.cursor.dir/bookmark";
 
         public static final String CONTENT_ITEM_TYPE = "vnd.android.cursor.item/bookmark";
 
         public static final String IS_FOLDER = "folder";
 
         public static final String PARENT = "parent";
--- a/mobile/android/base/sync/repositories/android/RepoUtils.java
+++ b/mobile/android/base/sync/repositories/android/RepoUtils.java
@@ -20,17 +20,17 @@ import org.mozilla.gecko.sync.repositori
 
 import android.content.Context;
 import android.database.Cursor;
 import android.database.sqlite.SQLiteDatabase;
 import android.net.Uri;
 
 public class RepoUtils {
 
-  private static final String LOG_TAG = "DBUtils";
+  private static final String LOG_TAG = "RepoUtils";
 
   /**
    * An array of known-special GUIDs.
    */
   public static String[] SPECIAL_GUIDS = new String[] {
     // Mobile and desktop places roots have to come first.
     "mobile",
     "places",
@@ -200,30 +200,30 @@ public class RepoUtils {
   // Returns android id from the URI that we get after inserting a
   // bookmark into the local Android store.
   public static long getAndroidIdFromUri(Uri uri) {
     String path = uri.getPath();
     int lastSlash = path.lastIndexOf('/');
     return Long.parseLong(path.substring(lastSlash + 1));
   }
 
-  public static BookmarkRecord computeParentFields(BookmarkRecord rec, String suggestedParentID, String suggestedParentName) {
+  public static BookmarkRecord computeParentFields(BookmarkRecord rec, String suggestedParentGUID, String suggestedParentName) {
     final String guid = rec.guid;
     if (guid == null) {
       // Oh dear.
       Logger.error(LOG_TAG, "No guid in computeParentFields!");
       return null;
     }
 
     String realParent = SPECIAL_GUID_PARENTS.get(guid);
     if (realParent == null) {
       // No magic parent. Use whatever the caller suggests.
-      realParent = suggestedParentID;
+      realParent = suggestedParentGUID;
     } else {
-      Logger.debug(LOG_TAG, "Ignoring suggested parent ID " + suggestedParentID +
+      Logger.debug(LOG_TAG, "Ignoring suggested parent ID " + suggestedParentGUID +
                            " for " + guid + "; using " + realParent);
     }
 
     if (realParent == null) {
       // Oh dear.
       Logger.error(LOG_TAG, "No parent for record " + guid);
       return null;
     }
@@ -235,17 +235,17 @@ public class RepoUtils {
     }
 
     rec.parentID = realParent;
     rec.parentName = parentName;
     return rec;
   }
 
   // Create a BookmarkRecord object from a cursor on a row containing a Fennec bookmark.
-  public static BookmarkRecord bookmarkFromMirrorCursor(Cursor cur, String parentId, String parentName, JSONArray children) {
+  public static BookmarkRecord bookmarkFromMirrorCursor(Cursor cur, String parentGUID, String parentName, JSONArray children) {
 
     String guid = getStringFromCursor(cur, BrowserContract.SyncColumns.GUID);
     String collection = "bookmarks";
     long lastModified = getLongFromCursor(cur, BrowserContract.SyncColumns.DATE_MODIFIED);
     boolean deleted   = getLongFromCursor(cur, BrowserContract.SyncColumns.IS_DELETED) == 1 ? true : false;
     boolean isFolder  = getIntFromCursor(cur, BrowserContract.Bookmarks.IS_FOLDER) == 1;
     BookmarkRecord rec = new BookmarkRecord(guid, collection, lastModified, deleted);
 
--- a/mobile/android/base/sync/repositories/domain/BookmarkRecord.java
+++ b/mobile/android/base/sync/repositories/domain/BookmarkRecord.java
@@ -39,16 +39,17 @@
 package org.mozilla.gecko.sync.repositories.domain;
 
 import org.json.simple.JSONArray;
 import org.mozilla.gecko.sync.CryptoRecord;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.Logger;
 import org.mozilla.gecko.sync.NonArrayJSONException;
 import org.mozilla.gecko.sync.Utils;
+import org.mozilla.gecko.sync.repositories.android.AndroidBrowserBookmarksDataAccessor;
 import org.mozilla.gecko.sync.repositories.android.RepoUtils;
 
 import android.util.Log;
 
 /**
  * Covers the fields used by all bookmark objects.
  * @author rnewman
  *
@@ -191,21 +192,21 @@ public class BookmarkRecord extends Reco
       public String queryID;
       public String siteURI;
       public String feedURI;
       public String pos;
      */
   }
 
   public boolean isBookmark() {
-    return "bookmark".equals(this.type);
+    return AndroidBrowserBookmarksDataAccessor.TYPE_BOOKMARK.equalsIgnoreCase(this.type);
   }
 
   public boolean isFolder() {
-    return "folder".equals(this.type);
+    return AndroidBrowserBookmarksDataAccessor.TYPE_FOLDER.equalsIgnoreCase(this.type);
   }
 
   @Override
   public CryptoRecord getPayload() {
     CryptoRecord rec = new CryptoRecord(this);
     rec.payload = new ExtendedJSONObject();
     rec.payload.put("id", this.guid);
     rec.payload.put("type", this.type);