Bug 1408710 - Pre: for clarity, rename session's delegateQueue to a more appropriate name r=rnewman draft
authorGrigory Kruglov <gkruglov@mozilla.com>
Fri, 05 Jan 2018 16:54:53 -0500
changeset 760193 5f4092c2629414fc4f9051d8f8f0c113f1d6910d
parent 760192 d789ac49c365336c104c4e66c9826bf387d85465
child 760194 62f5f7940bb8db9a18704edfd0b9cb38eb410b71
push id100565
push userbmo:gkruglov@mozilla.com
push dateMon, 26 Feb 2018 23:36:11 +0000
reviewersrnewman
bugs1408710
milestone60.0a1
Bug 1408710 - Pre: for clarity, rename session's delegateQueue to a more appropriate name r=rnewman From the point of view of the session itself, this queue is named correctly (e.g. session's abort and finish methods make sure invoke success and failure delegate methods from this queue). However, from the point of view of every concrete implementation of the session, the current naming isn't clear. New name is for symmetry with the store queue, and the above ambiquity is addressed in the comment. MozReview-Commit-ID: 61j7ZCNdr4x
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/RepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15RepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FennecTabsRepository.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FormHistoryRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/HistoryRepositorySession.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java
--- 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
@@ -49,19 +49,21 @@ public abstract class RepositorySession 
   private static final String LOG_TAG = "RepositorySession";
 
 
   private SessionStatus status = SessionStatus.UNSTARTED;
   protected Repository repository;
   protected RepositorySessionStoreDelegate storeDelegate;
 
   /**
-   * A queue of Runnables which call out into delegates.
+   * A queue of Runnables which perform fetching (see fetch* methods).
+   * At the same time, this is also a queue of Runnables which call out into session's delegates.
+   * See {@link #abort(RepositorySessionFinishDelegate)} and {@link #finish(RepositorySessionFinishDelegate)}.
    */
-  protected ExecutorService delegateQueue  = Executors.newSingleThreadExecutor();
+  protected ExecutorService fetchWorkQueue = Executors.newSingleThreadExecutor();
 
   /**
    * A queue of Runnables which effect storing.
    * This includes actual store work, and also the consequences of storeDone.
    * This provides strict ordering.
    */
   protected ExecutorService storeWorkQueue = Executors.newSingleThreadExecutor();
 
@@ -198,17 +200,17 @@ public abstract class RepositorySession 
 
   /**
    * Synchronously perform the shared work of beginning. Throws on failure.
    * @throws InvalidSessionTransitionException
    *
    */
   protected void sharedBegin() throws InvalidSessionTransitionException {
     Logger.debug(LOG_TAG, "Shared begin.");
-    if (delegateQueue.isShutdown()) {
+    if (fetchWorkQueue.isShutdown()) {
       throw new InvalidSessionTransitionException(null);
     }
     if (storeWorkQueue.isShutdown()) {
       throw new InvalidSessionTransitionException(null);
     }
     this.transitionFrom(SessionStatus.UNSTARTED, SessionStatus.ACTIVE);
   }
 
@@ -245,69 +247,57 @@ public abstract class RepositorySession 
   }
 
   /**
    * 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.
    */
   public void abort(RepositorySessionFinishDelegate delegate) {
     this.abort();
-    delegate.deferredFinishDelegate(delegateQueue).onFinishSucceeded(this, this.getBundle());
+    delegate.deferredFinishDelegate(fetchWorkQueue).onFinishSucceeded(this, this.getBundle());
   }
 
   /**
    * Abnormally terminate the repository session, freeing or closing
    * any resources that were opened during the lifetime of the session.
    */
   public void abort() {
     // TODO: do something here.
     this.setStatus(SessionStatus.ABORTED);
     try {
       storeWorkQueue.shutdownNow();
     } catch (Exception e) {
       Logger.error(LOG_TAG, "Caught exception shutting down store work queue.", e);
     }
     try {
-      delegateQueue.shutdown();
+      fetchWorkQueue.shutdown();
     } catch (Exception e) {
       Logger.error(LOG_TAG, "Caught exception shutting down delegate queue.", e);
     }
   }
 
   /**
    * End the repository session, freeing or closing any resources
    * that were opened during the lifetime of the session.
    *
    * @param delegate notified of success or failure.
    * @throws InactiveSessionException
    */
   public void finish(final RepositorySessionFinishDelegate delegate) throws InactiveSessionException {
     try {
       this.transitionFrom(SessionStatus.ACTIVE, SessionStatus.DONE);
-      delegate.deferredFinishDelegate(delegateQueue).onFinishSucceeded(this, this.getBundle());
+      delegate.deferredFinishDelegate(fetchWorkQueue).onFinishSucceeded(this, this.getBundle());
     } catch (InvalidSessionTransitionException e) {
       Logger.error(LOG_TAG, "Tried to finish() an unstarted or already finished session");
       throw new InactiveSessionException(e);
     }
 
     Logger.trace(LOG_TAG, "Shutting down work queues.");
     storeWorkQueue.shutdown();
-    delegateQueue.shutdown();
-  }
-
-  /**
-   * Run the provided command if we're active and our delegate queue
-   * is not shut down.
-   */
-  protected synchronized void executeDelegateCommand(Runnable command)
-      throws InactiveSessionException {
-    if (!isActive() || delegateQueue.isShutdown()) {
-      throw new InactiveSessionException();
-    }
-    delegateQueue.execute(command);
+    fetchWorkQueue.shutdown();
   }
 
   public synchronized void ensureActive() throws InactiveSessionException {
     if (!isActive()) {
       throw new InactiveSessionException();
     }
   }
 
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15RepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/Server15RepositorySession.java
@@ -21,17 +21,17 @@ public class Server15RepositorySession e
   protected final Server15Repository serverRepository;
   private BatchingUploader uploader;
   private final BatchingDownloader downloader;
 
   public Server15RepositorySession(Repository repository) {
     super(repository);
     this.serverRepository = (Server15Repository) repository;
     this.downloader = new BatchingDownloader(
-            this.delegateQueue,
+            this.fetchWorkQueue,
             this.serverRepository.authHeaderProvider,
             Uri.parse(this.serverRepository.collectionURI().toString()),
             this.serverRepository.getSyncDeadline(),
             this.serverRepository.getAllowMultipleBatches(),
             this.serverRepository.getAllowHighWaterMark(),
             this.serverRepository.stateProvider,
             this);
   }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/BookmarksRepositorySession.java
@@ -138,42 +138,42 @@ public class BookmarksRepositorySession 
 
   @Override
   public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
     if (this.storeTracker == null) {
       throw new IllegalStateException("Store tracker not yet initialized!");
     }
 
     Logger.debug(LOG_TAG, "Running fetchModified.");
-    delegateQueue.execute(
+    fetchWorkQueue.execute(
             sessionHelper.getFetchModifiedRunnable(
                     now(),
                     this.storeTracker.getFilter(),
                     versioningDelegateHelper.getFetchDelegate(delegate)
             )
     );
   }
 
   @Override
   public void fetch(String[] guids, RepositorySessionFetchRecordsDelegate delegate) throws InactiveSessionException {
-    executeDelegateCommand(sessionHelper.getFetchRunnable(
+    fetchWorkQueue.execute(sessionHelper.getFetchRunnable(
             guids, now(), null, versioningDelegateHelper.getFetchDelegate(delegate))
     );
   }
 
   @Override
   public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
     if (this.storeTracker == null) {
       throw new IllegalStateException("Store tracker not yet initialized!");
     }
 
     final long since = -1L;
 
     Logger.debug(LOG_TAG, "Running fetchSince(" + since + ").");
-    delegateQueue.execute(
+    fetchWorkQueue.execute(
             sessionHelper.getFetchSinceRunnable(
                     since,
                     now(),
                     this.storeTracker.getFilter(),
                     versioningDelegateHelper.getFetchDelegate(delegate)
             )
     );
   }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FennecTabsRepository.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FennecTabsRepository.java
@@ -159,17 +159,17 @@ public class FennecTabsRepository extend
             delegate.onFetchFailed(e);
             return;
           }
           setLastFetchTimestamp(now());
           delegate.onFetchCompleted();
         }
       };
 
-      delegateQueue.execute(command);
+      fetchWorkQueue.execute(command);
     }
 
     @Override
     public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
       this.fetchSince(getLastSyncTimestamp(), delegate);
     }
 
     @Override
@@ -196,17 +196,17 @@ public class FennecTabsRepository extend
     }
 
     @Override
     public void fetch(final String[] guids,
                       final RepositorySessionFetchRecordsDelegate delegate) {
       // Bug 783692: Now that Bug 730039 has landed, we could implement this,
       // but it's not a priority since it's not used (yet).
       Logger.warn(LOG_TAG, "Not returning anything from fetch");
-      delegateQueue.execute(new Runnable() {
+      fetchWorkQueue.execute(new Runnable() {
         @Override
         public void run() {
           delegate.onFetchCompleted();
         }
       });
     }
 
     private static final String TABS_CLIENT_GUID_IS = BrowserContract.Tabs.CLIENT_GUID + " = ?";
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FormHistoryRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/FormHistoryRepositorySession.java
@@ -212,17 +212,17 @@ public class FormHistoryRepositorySessio
           }
         }
 
         setLastFetchTimestamp(end);
         delegate.onFetchCompleted();
       }
     };
 
-    delegateQueue.execute(command);
+    fetchWorkQueue.execute(command);
   }
 
   protected static String regularBetween(long start, long end) {
     return FormHistory.FIRST_USED + " >= " + Long.toString(1000 * start) + " AND " +
            FormHistory.FIRST_USED + " <= " + Long.toString(1000 * end); // Microseconds.
   }
 
   protected static String deletedBetween(long start, long end) {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/HistoryRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/HistoryRepositorySession.java
@@ -61,31 +61,31 @@ public class HistoryRepositorySession ex
 
   @Override
   public void fetchModified(RepositorySessionFetchRecordsDelegate delegate) {
     this.fetchSince(getLastSyncTimestamp(), delegate);
   }
 
   @Override
   public void fetch(String[] guids, RepositorySessionFetchRecordsDelegate delegate) throws InactiveSessionException {
-    executeDelegateCommand(sessionHelper.getFetchRunnable(guids, now(), null, delegate));
+    fetchWorkQueue.execute(sessionHelper.getFetchRunnable(guids, now(), null, delegate));
   }
 
   @Override
   public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
     this.fetchSince(-1, delegate);
   }
 
   private void fetchSince(long timestamp, RepositorySessionFetchRecordsDelegate delegate) {
     if (this.storeTracker == null) {
       throw new IllegalStateException("Store tracker not yet initialized!");
     }
 
     Logger.debug(LOG_TAG, "Running fetchSince(" + timestamp + ").");
-    delegateQueue.execute(
+    fetchWorkQueue.execute(
             sessionHelper.getFetchSinceRunnable(
                     timestamp, now(), this.storeTracker.getFilter(), delegate
             )
     );
   }
 
   @Override
   public void store(Record record) throws NoStoreDelegateException {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/PasswordsRepositorySession.java
@@ -105,34 +105,34 @@ public class PasswordsRepositorySession 
           }
         } catch (Exception e) {
           Logger.error(LOG_TAG, "Exception in fetch.");
           delegate.onFetchFailed(e);
         }
       }
     };
 
-    delegateQueue.execute(fetchSinceRunnable);
+    fetchWorkQueue.execute(fetchSinceRunnable);
   }
 
   @Override
   public void fetchModified(final RepositorySessionFetchRecordsDelegate delegate) {
     this.fetchSince(getLastSyncTimestamp(), delegate);
   }
 
   @Override
   public void fetchAll(RepositorySessionFetchRecordsDelegate delegate) {
     this.fetchSince(-1, delegate);
   }
 
   @Override
   public void fetch(final String[] guids, final RepositorySessionFetchRecordsDelegate delegate) {
     if (guids == null || guids.length < 1) {
       Logger.error(LOG_TAG, "No guids to be fetched.");
-      delegateQueue.execute(new Runnable() {
+      fetchWorkQueue.execute(new Runnable() {
         @Override
         public void run() {
           delegate.onFetchCompleted();
         }
       });
       return;
     }
 
@@ -172,17 +172,17 @@ public class PasswordsRepositorySession 
 
         } catch (Exception e) {
           Logger.error(LOG_TAG, "Exception in fetch.");
           delegate.onFetchFailed(e);
         }
       }
     };
 
-    delegateQueue.execute(fetchRunnable);
+    fetchWorkQueue.execute(fetchRunnable);
   }
 
   @Override
   public void store(final Record record) throws NoStoreDelegateException {
     if (storeDelegate == null) {
       Logger.error(LOG_TAG, "No store delegate.");
       throw new NoStoreDelegateException();
     }