Bug 1291821 - Add storeIncomplete to RepositorySession interface r=rnewman draft
authorGrisha Kruglov <gkruglov@mozilla.com>
Wed, 02 Nov 2016 16:40:15 -0700
changeset 489482 b4ae0a3c286963f4c476c7c0ed768ad5a2b5bb8b
parent 489481 d23b3f157fe7cf42e30a40c034970a355098c68c
child 489483 1b3f102b011fe171f8cafab0cf47ca69b2eb9b93
push id46825
push usergkruglov@mozilla.com
push dateFri, 24 Feb 2017 21:13:39 +0000
reviewersrnewman
bugs1291821
milestone54.0a1
Bug 1291821 - Add storeIncomplete to RepositorySession interface r=rnewman MozReview-Commit-ID: 68ty7KlP5NR
mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/MiddlewareRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/ConcurrentRecordConsumer.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/RecordsChannel.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/MiddlewareRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/middleware/MiddlewareRepositorySession.java
@@ -149,16 +149,21 @@ public abstract class MiddlewareReposito
 
   @Override
   public void guidsSince(long timestamp, RepositorySessionGuidsSinceDelegate delegate) {
     // TODO: need to do anything here?
     inner.guidsSince(timestamp, delegate);
   }
 
   @Override
+  public void storeIncomplete() {
+    inner.storeIncomplete();
+  }
+
+  @Override
   public void storeDone() {
     inner.storeDone();
   }
 
   @Override
   public void storeDone(long storeEnd) {
     inner.storeDone(storeEnd);
   }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
@@ -121,16 +121,18 @@ public abstract class RepositorySession 
    * Store success calls are not guaranteed.
    */
   public void setStoreDelegate(RepositorySessionStoreDelegate delegate) {
     Logger.debug(LOG_TAG, "Setting store delegate to " + delegate);
     this.delegate = delegate;
   }
   public abstract void store(Record record) throws NoStoreDelegateException;
 
+  public void storeIncomplete() {}
+
   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) {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/AndroidBrowserBookmarksRepositorySession.java
@@ -897,56 +897,52 @@ public class AndroidBrowserBookmarksRepo
       Logger.debug(LOG_TAG, "Done applying deletions.");
     } catch (Exception e) {
       Logger.error(LOG_TAG, "Unable to apply deletions.", e);
     }
   }
 
   @SuppressWarnings("unchecked")
   private void finishUp() {
-    try {
-      flushQueues();
-      Logger.debug(LOG_TAG, "Have " + parentToChildArray.size() + " folders whose children might need repositioning.");
-      for (Entry<String, JSONArray> entry : parentToChildArray.entrySet()) {
-        String guid = entry.getKey();
-        JSONArray onServer = entry.getValue();
-        try {
-          final long folderID = getIDForGUID(guid);
-          final JSONArray inDB = new JSONArray();
-          final boolean clean = getChildrenArray(folderID, false, inDB);
-          final boolean sameArrays = Utils.sameArrays(onServer, inDB);
+    flushQueues();
+    Logger.debug(LOG_TAG, "Have " + parentToChildArray.size() + " folders whose children might need repositioning.");
+    for (Entry<String, JSONArray> entry : parentToChildArray.entrySet()) {
+      String guid = entry.getKey();
+      JSONArray onServer = entry.getValue();
+      try {
+        final long folderID = getIDForGUID(guid);
+        final JSONArray inDB = new JSONArray();
+        final boolean clean = getChildrenArray(folderID, false, inDB);
+        final boolean sameArrays = Utils.sameArrays(onServer, inDB);
 
-          // If the local children and the remote children are already
-          // the same, then we don't need to bump the modified time of the
-          // parent: we wouldn't upload a different record, so avoid the cycle.
-          if (!sameArrays) {
-            int added = 0;
-            for (Object o : inDB) {
-              if (!onServer.contains(o)) {
-                onServer.add(o);
-                added++;
-              }
+        // If the local children and the remote children are already
+        // the same, then we don't need to bump the modified time of the
+        // parent: we wouldn't upload a different record, so avoid the cycle.
+        if (!sameArrays) {
+          int added = 0;
+          for (Object o : inDB) {
+            if (!onServer.contains(o)) {
+              onServer.add(o);
+              added++;
             }
-            Logger.debug(LOG_TAG, "Added " + added + " items locally.");
-            Logger.debug(LOG_TAG, "Untracking and bumping " + guid + "(" + folderID + ")");
-            dataAccessor.bumpModified(folderID, now());
-            untrackGUID(guid);
           }
+          Logger.debug(LOG_TAG, "Added " + added + " items locally.");
+          Logger.debug(LOG_TAG, "Untracking and bumping " + guid + "(" + folderID + ")");
+          dataAccessor.bumpModified(folderID, now());
+          untrackGUID(guid);
+        }
 
-          // If the arrays are different, or they're the same but not flushed to disk,
-          // write them out now.
-          if (!sameArrays || !clean) {
-            dataAccessor.updatePositions(new ArrayList<String>(onServer));
-          }
-        } catch (Exception e) {
-          Logger.warn(LOG_TAG, "Error repositioning children for " + guid, e);
+        // If the arrays are different, or they're the same but not flushed to disk,
+        // write them out now.
+        if (!sameArrays || !clean) {
+          dataAccessor.updatePositions(new ArrayList<String>(onServer));
         }
+      } catch (Exception e) {
+        Logger.warn(LOG_TAG, "Error repositioning children for " + guid, e);
       }
-    } finally {
-      super.storeDone();
     }
   }
 
   /**
    * Hook into the deletion manager on wipe.
    */
   class BookmarkWipeRunnable extends WipeRunnable {
     public BookmarkWipeRunnable(RepositorySessionWipeDelegate delegate) {
@@ -972,17 +968,21 @@ public class AndroidBrowserBookmarksRepo
     return new BookmarkWipeRunnable(delegate);
   }
 
   @Override
   public void storeDone() {
     Runnable command = new Runnable() {
       @Override
       public void run() {
-        finishUp();
+        try {
+          finishUp();
+        } finally {
+          AndroidBrowserBookmarksRepositorySession.super.storeDone();
+        }
       }
     };
     storeWorkQueue.execute(command);
   }
 
   @Override
   protected String buildRecordString(Record record) {
     BookmarkRecord bmk = (BookmarkRecord) record;
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/ConcurrentRecordConsumer.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/ConcurrentRecordConsumer.java
@@ -60,17 +60,17 @@ class ConcurrentRecordConsumer extends R
     Logger.trace(LOG_TAG, "Record stored. Notifying.");
     synchronized (countMonitor) {
       counter++;
     }
   }
 
   private void consumerIsDone() {
     Logger.debug(LOG_TAG, "Consumer is done. Processed " + counter + ((counter == 1) ? " record." : " records."));
-    delegate.consumerIsDone(!allRecordsQueued);
+    delegate.consumerIsDone(allRecordsQueued);
   }
 
   @Override
   public void run() {
     Record record;
 
     while (true) {
       // The queue is concurrent-safe.
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/RecordsChannel.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/synchronizer/RecordsChannel.java
@@ -232,17 +232,20 @@ public class RecordsChannel implements
   }
 
 
   @Override
   public void consumerIsDone(boolean allRecordsQueued) {
     Logger.trace(LOG_TAG, "Consumer is done. Are we waiting for it? " + waitingForQueueDone);
     if (waitingForQueueDone) {
       waitingForQueueDone = false;
-      this.sink.storeDone();                 // Now we'll be waiting for onStoreCompleted.
+      if (!allRecordsQueued) {
+        this.sink.storeIncomplete();
+      }
+      this.sink.storeDone(); // Now we'll be waiting for onStoreCompleted.
     }
   }
 
   @Override
   public void onStoreCompleted(long storeEnd) {
     Logger.trace(LOG_TAG, "onStoreCompleted. Notifying delegate of onFlowCompleted. " +
                           "Fetch end is " + fetchEnd + ", store end is " + storeEnd);
     // TODO: synchronize on consumer callback?