Reduce unnecessary applyCustomRunConditions after service state change (

#435) Ref.: #7e7b2c9 Improve RunConditionMonitor#updateShouldRunDecision ( 7e7b2c9)
Catfriend1 · Jul 11, 2019 · 6b9745c · 6b9745c
1 parent 0046a75
commit 6b9745c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 28 deletions.
diff --git a/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java b/app/src/main/java/com/nutomic/syncthingandroid/service/RunConditionMonitor.java
@@ -253,19 +253,39 @@ public void onReceive(Context context, Intent intent) {
      * We then need to decide if syncthing should run.
      */
     public void updateShouldRunDecision() {
-        // Check if the current conditions changed the result of decideShouldRun()
-        // compared to the last determined result.
         boolean newShouldRun = decideShouldRun();
+        if (newShouldRun) {
+            /**
+             * Trigger:
+             *  a) Sync pre-conditions changed
+             *      a1) AND SyncthingService.State should remain ACTIVE
+             *      a2) AND SyncthingService.State should transition from INIT/DISABLED to ACTIVE
+             *  b) Sync pre-conditions did not change
+             *      b1) AND SyncthingService.State should remain ACTIVE
+             *          because a reevaluation of the run conditions was forced from code.
+             * Action:
+             *  SyncthingService will evaluate custom per-object run conditions
+             *  and pause/unpause objects accordingly.
+             */
+             if (mOnSyncPreconditionChangedListener != null) {
+                 mOnSyncPreconditionChangedListener.onSyncPreconditionChanged(this);
+             }
+        }
+
+        /**
+         * Check if the current conditions changed the result of decideShouldRun()
+         * compared to the last determined result.
+         */
         if (newShouldRun != lastDeterminedShouldRun) {
+            /**
+             * Notify SyncthingService in case it has to transition from
+             * a) INIT/DISABLED => STARTING => ACTIVE
+             * b) ACTIVE => DISABLED
+             */
             if (mOnShouldRunChangedListener != null) {
                 mOnShouldRunChangedListener.onShouldRunDecisionChanged(newShouldRun);
+                lastDeterminedShouldRun = newShouldRun;
             }
-            lastDeterminedShouldRun = newShouldRun;
-        }
-
-        // Notify about changed preconditions.
-        if (mOnSyncPreconditionChangedListener != null) {
-            mOnSyncPreconditionChangedListener.onSyncPreconditionChanged(this);
         }
     }
 
@@ -461,6 +481,7 @@ private boolean decideShouldRun() {
 
     /**
      * Check if an object's individual sync conditions are met.
+     * Precondition: Object must own pref "...CustomSyncConditionsEnabled == true".
      */
     public Boolean checkObjectSyncConditions(String objectPrefixAndId) {
         // Sync on mobile data?

diff --git a/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java b/app/src/main/java/com/nutomic/syncthingandroid/service/SyncthingService.java
@@ -252,19 +252,6 @@ public int onStartCommand(Intent intent, int flags, int startId) {
             return START_NOT_STICKY;
         }
 
-        /**
-         * Send current service state to listening endpoints.
-         * This is required that components know about the service State.DISABLED
-         * if RunConditionMonitor does not send a "shouldRun = true" callback
-         * to start the binary according to preferences shortly after its creation.
-         * See {@link mLastDeterminedShouldRun} defaulting to "false".
-         */
-        if (mCurrentState == State.DISABLED) {
-            synchronized (mStateLock) {
-                onServiceStateChange(mCurrentState);
-            }
-        }
-
         if (mPrefBroadcastServiceControl) {
             Log.i(TAG, "onStartCommand: mPrefBroadcastServiceControl == true, RunConditionMonitor is disabled.");
             /**
@@ -584,9 +571,6 @@ private void onApiAvailable() {
             }
             onServiceStateChange(State.ACTIVE);
         }
-        if (mRestApi != null && mRunConditionMonitor != null) {
-            mRestApi.applyCustomRunConditions(mRunConditionMonitor);
-        }
 
         if (mEventProcessor == null) {
             mEventProcessor = new EventProcessor(SyncthingService.this, mRestApi);
@@ -705,8 +689,10 @@ public void evaluateRunConditions() {
      * @see #unregisterOnServiceStateChangeListener
      */
     public void registerOnServiceStateChangeListener(OnServiceStateChangeListener listener) {
-        // Make sure we don't send an invalid state or syncthing might show a "disabled" message
-        // when it's just starting up.
+        /**
+         * Initially send the current state to the new subscriber to make sure it doesn't stay
+         * in undefined state forever until the state next change occurs.
+         */
         listener.onServiceStateChange(mCurrentState);
         mOnServiceStateChangeListeners.add(listener);
     }
@@ -726,9 +712,9 @@ public void unregisterOnServiceStateChangeListener(OnServiceStateChangeListener
     private void onServiceStateChange(State newState) {
         if (newState == mCurrentState) {
             Log.d(TAG, "onServiceStateChange: Called with unchanged state " + newState);
-        } else {
-            Log.i(TAG, "onServiceStateChange: from " + mCurrentState + " to " + newState);
+            return;
         }
+        Log.i(TAG, "onServiceStateChange: from " + mCurrentState + " to " + newState);
         mCurrentState = newState;
         mHandler.post(() -> {
             mNotificationHandler.updatePersistentNotification(this);