Remove remaining AndroidSyncSettingsObservers

For all remaining implementations except SyncController, implementing
SyncStateChangedListener (~SyncServiceObserver) is enough. So this CL
converts the observer interface into a delegate interface.

Bug: 1107904
Change-Id: Iaa2acd4cbb6ce0efc6b8d9a6e441f4e2b1f8e4df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570573
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833387}

chrome/android/java/src/org/chromium/chrome/browser/sync/AndroidSyncSettings.java

@@ -15,7 +15,6 @@ import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.ContextUtils;
-import org.chromium.base.ObserverList;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.task.PostTask;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -33,7 +32,6 @@ import org.chromium.content_public.browser.UiThreadTaskTraits;
  * DecoupleSyncFromAndroidMasterSync is enabled.
  *
  * A helper class to handle the current status of sync for Chrome in Android settings.
- * It also provides an observer to be used whenever Android sync settings change.
  *
  * {@link #updateAccount(Account)} should be invoked whenever sync account is changed.
  */
@@ -57,12 +55,14 @@ public class AndroidSyncSettings {
 
     private boolean mShouldDecoupleSyncFromMasterSync;
 
-    private final ObserverList<AndroidSyncSettingsObserver> mObservers = new ObserverList<>();
+    // Is set at most once.
+    @Nullable
+    private Delegate mDelegate;
 
     /**
-     * Provides notifications when Android sync settings have changed.
+     * Propagates changes from Android sync settings to the native code.
      */
-    public interface AndroidSyncSettingsObserver {
+    public interface Delegate {
         void androidSyncSettingsChanged();
     }
 
@@ -122,9 +122,7 @@ public class AndroidSyncSettings {
                 // This is called by Android on a background thread, but AndroidSyncSettings
                 // methods should be called from the UI thread, so post a task.
                 PostTask.postTask(UiThreadTaskTraits.DEFAULT, () -> {
-                    if (updateCachedSettings()) {
-                        notifyObservers();
-                    }
+                    if (updateCachedSettings()) maybeNotifyDelegate();
                 });
             }
         };
@@ -190,9 +188,7 @@ public class AndroidSyncSettings {
         ThreadUtils.assertOnUiThread();
         mAccount = account;
         updateSyncability();
-        if (updateCachedSettings()) {
-            notifyObservers();
-        }
+        if (updateCachedSettings()) maybeNotifyDelegate();
     }
 
     /**
@@ -206,19 +202,13 @@ public class AndroidSyncSettings {
     }
 
     /**
-     * Add a new AndroidSyncSettingsObserver.
-     */
-    public void registerObserver(AndroidSyncSettingsObserver observer) {
-        ThreadUtils.assertOnUiThread();
-        mObservers.addObserver(observer);
-    }
-
-    /**
-     * Remove an AndroidSyncSettingsObserver that was previously added.
+     * Must be called at most once to set a (non-null) delegate.
      */
-    public void unregisterObserver(AndroidSyncSettingsObserver observer) {
+    public void setDelegate(Delegate delegate) {
         ThreadUtils.assertOnUiThread();
-        mObservers.removeObserver(observer);
+        assert delegate != null;
+        assert mDelegate == null;
+        mDelegate = delegate;
     }
 
     private void setChromeSyncEnabled(boolean value) {
@@ -227,7 +217,7 @@ public class AndroidSyncSettings {
         mChromeSyncEnabled = value;
 
         mSyncContentResolverDelegate.setSyncAutomatically(mAccount, mContractAuthority, value);
-        notifyObservers();
+        maybeNotifyDelegate();
     }
 
     /**
@@ -301,10 +291,8 @@ public class AndroidSyncSettings {
                 || oldMasterSyncEnabled != mMasterSyncEnabled;
     }
 
-    private void notifyObservers() {
-        for (AndroidSyncSettingsObserver observer : mObservers) {
-            observer.androidSyncSettingsChanged();
-        }
+    private void maybeNotifyDelegate() {
+        if (mDelegate != null) mDelegate.androidSyncSettingsChanged();
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/sync/SyncController.java

@@ -36,8 +36,8 @@ import org.chromium.components.sync.StopSource;
  * are careful to not change the Android Chrome sync setting so we know whether to turn sync back
  * on when it is re-enabled.
  */
-public class SyncController implements ProfileSyncService.SyncStateChangedListener,
-                                       AndroidSyncSettings.AndroidSyncSettingsObserver {
+public class SyncController
+        implements ProfileSyncService.SyncStateChangedListener, AndroidSyncSettings.Delegate {
     private static final String TAG = "SyncController";
 
     /**
@@ -58,7 +58,7 @@ public class SyncController implements ProfileSyncService.SyncStateChangedListen
     private final SyncNotificationController mSyncNotificationController;
 
     private SyncController() {
-        AndroidSyncSettings.get().registerObserver(this);
+        AndroidSyncSettings.get().setDelegate(this);
         mProfileSyncService = ProfileSyncService.get();
         mProfileSyncService.addSyncStateChangedListener(this);
 
@@ -154,7 +154,7 @@ public class SyncController implements ProfileSyncService.SyncStateChangedListen
     }
 
     /**
-     * From {@link AndroidSyncSettings.AndroidSyncSettingsObserver}.
+     * From {@link AndroidSyncSettings.Delegate}.
      */
     @Override
     public void androidSyncSettingsChanged() {

chrome/android/java/src/org/chromium/chrome/browser/sync/settings/SignInPreference.java

@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.signin.SigninUtils;
 import org.chromium.chrome.browser.signin.services.DisplayableProfileData;
 import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
 import org.chromium.chrome.browser.signin.services.SigninManager.SignInAllowedObserver;
-import org.chromium.chrome.browser.sync.AndroidSyncSettings;
 import org.chromium.chrome.browser.sync.ProfileSyncService;
 import org.chromium.chrome.browser.sync.ProfileSyncService.SyncStateChangedListener;
 import org.chromium.components.browser_ui.settings.ManagedPreferencesUtils;
@@ -48,7 +47,6 @@ import java.util.Collections;
  */
 public class SignInPreference
         extends Preference implements SignInAllowedObserver, ProfileDataCache.Observer,
-                                      AndroidSyncSettings.AndroidSyncSettingsObserver,
                                       SyncStateChangedListener, AccountsChangeObserver {
     @IntDef({State.SIGNIN_DISABLED_BY_POLICY, State.SIGNIN_DISALLOWED, State.GENERIC_PROMO,
             State.SIGNED_IN})
@@ -94,7 +92,6 @@ public class SignInPreference
                 .addSignInAllowedObserver(this);
         mProfileDataCache.addObserver(this);
         FirstRunSignInProcessor.updateSigninManagerFirstRunCheckDone();
-        AndroidSyncSettings.get().registerObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.addSyncStateChangedListener(this);
@@ -112,7 +109,6 @@ public class SignInPreference
                 .getSigninManager(Profile.getLastUsedRegularProfile())
                 .removeSignInAllowedObserver(this);
         mProfileDataCache.removeObserver(this);
-        AndroidSyncSettings.get().unregisterObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.removeSyncStateChangedListener(this);
@@ -257,12 +253,6 @@ public class SignInPreference
         update();
     }
 
-    // AndroidSyncSettings.AndroidSyncSettingsObserver implementation.
-    @Override
-    public void androidSyncSettingsChanged() {
-        update();
-    }
-
     // AccountsChangeObserver implementation.
     @Override
     public void onAccountsChanged() {

chrome/android/java/src/org/chromium/chrome/browser/sync/settings/SyncErrorCardPreference.java

@@ -18,7 +18,6 @@ import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.signin.PersonalizedSigninPromoView;
 import org.chromium.chrome.browser.signin.ProfileDataCache;
 import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
-import org.chromium.chrome.browser.sync.AndroidSyncSettings;
 import org.chromium.chrome.browser.sync.ProfileSyncService;
 import org.chromium.chrome.browser.sync.settings.SyncSettingsUtils.SyncError;
 import org.chromium.components.signin.base.CoreAccountInfo;
@@ -27,8 +26,7 @@ import org.chromium.components.signin.identitymanager.ConsentLevel;
 import java.util.Collections;
 
 public class SyncErrorCardPreference extends Preference
-        implements AndroidSyncSettings.AndroidSyncSettingsObserver,
-                   ProfileSyncService.SyncStateChangedListener, ProfileDataCache.Observer {
+        implements ProfileSyncService.SyncStateChangedListener, ProfileDataCache.Observer {
     /**
      * Listener for the buttons in the error card.
      */
@@ -68,7 +66,6 @@ public class SyncErrorCardPreference extends Preference
     public void onAttached() {
         super.onAttached();
         mProfileDataCache.addObserver(this);
-        AndroidSyncSettings.get().registerObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.addSyncStateChangedListener(this);
@@ -80,7 +77,6 @@ public class SyncErrorCardPreference extends Preference
     public void onDetached() {
         super.onDetached();
         mProfileDataCache.removeObserver(this);
-        AndroidSyncSettings.get().unregisterObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.removeSyncStateChangedListener(this);
@@ -172,14 +168,6 @@ public class SyncErrorCardPreference extends Preference
         update();
     }
 
-    /**
-     * {@link AndroidSyncSettings.AndroidSyncSettingsObserver} implementation.
-     */
-    @Override
-    public void androidSyncSettingsChanged() {
-        update();
-    }
-
     /**
      * {@link ProfileDataCache.Observer} implementation.
      */

chrome/android/java/src/org/chromium/chrome/browser/sync/settings/SyncPromoPreference.java

@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.signin.SigninPromoController;
 import org.chromium.chrome.browser.signin.SigninPromoUtil;
 import org.chromium.chrome.browser.signin.services.IdentityServicesProvider;
 import org.chromium.chrome.browser.signin.services.SigninManager.SignInAllowedObserver;
-import org.chromium.chrome.browser.sync.AndroidSyncSettings;
 import org.chromium.chrome.browser.sync.ProfileSyncService;
 import org.chromium.chrome.browser.sync.ProfileSyncService.SyncStateChangedListener;
 import org.chromium.components.signin.AccountManagerFacade;
@@ -42,7 +41,6 @@ import java.lang.annotation.RetentionPolicy;
 // TODO(https://crbug.com/1110889): Move all promos from SigninPreference to this class.
 public class SyncPromoPreference
         extends Preference implements SignInAllowedObserver, ProfileDataCache.Observer,
-                                      AndroidSyncSettings.AndroidSyncSettingsObserver,
                                       SyncStateChangedListener, AccountsChangeObserver {
     @Retention(RetentionPolicy.SOURCE)
     @IntDef({State.PROMO_HIDDEN, State.PERSONALIZED_SIGNIN_PROMO, State.PERSONALIZED_SYNC_PROMO})
@@ -83,7 +81,6 @@ public class SyncPromoPreference
                 .addSignInAllowedObserver(this);
         mProfileDataCache.addObserver(this);
         FirstRunSignInProcessor.updateSigninManagerFirstRunCheckDone();
-        AndroidSyncSettings.get().registerObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.addSyncStateChangedListener(this);
@@ -101,7 +98,6 @@ public class SyncPromoPreference
                 .getSigninManager(Profile.getLastUsedRegularProfile())
                 .removeSignInAllowedObserver(this);
         mProfileDataCache.removeObserver(this);
-        AndroidSyncSettings.get().unregisterObserver(this);
         ProfileSyncService syncService = ProfileSyncService.get();
         if (syncService != null) {
             syncService.removeSyncStateChangedListener(this);
@@ -224,12 +220,6 @@ public class SyncPromoPreference
         update();
     }
 
-    // AndroidSyncSettings.AndroidSyncSettingsObserver implementation.
-    @Override
-    public void androidSyncSettingsChanged() {
-        update();
-    }
-
     // AccountsChangeObserver implementation.
     @Override
     public void onAccountsChanged() {

chrome/android/javatests/src/org/chromium/chrome/browser/sync/AndroidSyncSettingsTest.java

@@ -31,7 +31,6 @@ import org.mockito.junit.MockitoRule;
 import org.chromium.base.test.BaseJUnit4ClassRunner;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
-import org.chromium.chrome.browser.sync.AndroidSyncSettings.AndroidSyncSettingsObserver;
 import org.chromium.chrome.test.ChromeBrowserTestRule;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
@@ -100,8 +99,8 @@ public class AndroidSyncSettingsTest {
         }
     }
 
-    // Should live on the UI thread as normal a AndroidSyncSettingsObserver.
-    private static class MockSyncSettingsObserver implements AndroidSyncSettingsObserver {
+    // Should live on the UI thread as a normal AndroidSyncSettings.Delegate.
+    private static class MockSyncSettingsDelegate implements AndroidSyncSettings.Delegate {
         private boolean mReceivedNotification;
 
         public void clearNotification() {
@@ -339,42 +338,42 @@ public class AndroidSyncSettingsTest {
     @Test
     @SmallTest
     @Feature({"Sync"})
-    public void testAndroidSyncSettingsPostsNotifications() {
+    public void testAndroidSyncSettingsNotifiesDelegate() {
         createAndroidSyncSettings();
 
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            MockSyncSettingsObserver syncSettingsObserver = new MockSyncSettingsObserver();
-            mAndroidSyncSettings.registerObserver(syncSettingsObserver);
+            MockSyncSettingsDelegate syncSettingsDelegate = new MockSyncSettingsDelegate();
+            mAndroidSyncSettings.setDelegate(syncSettingsDelegate);
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.enableChromeSync();
-            Assert.assertTrue("enableChromeSync should trigger observers",
-                    syncSettingsObserver.receivedNotification());
+            Assert.assertTrue("enableChromeSync should notify delegate",
+                    syncSettingsDelegate.receivedNotification());
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.updateAccount(mAlternateAccount);
             Assert.assertTrue("switching to account with different settings should notify",
-                    syncSettingsObserver.receivedNotification());
+                    syncSettingsDelegate.receivedNotification());
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.updateAccount(mAccount);
             Assert.assertTrue("switching to account with different settings should notify",
-                    syncSettingsObserver.receivedNotification());
+                    syncSettingsDelegate.receivedNotification());
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.enableChromeSync();
-            Assert.assertFalse("enableChromeSync shouldn't trigger observers",
-                    syncSettingsObserver.receivedNotification());
+            Assert.assertFalse("enableChromeSync shouldn't notify delegate",
+                    syncSettingsDelegate.receivedNotification());
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.disableChromeSync();
-            Assert.assertTrue("disableChromeSync should trigger observers",
-                    syncSettingsObserver.receivedNotification());
+            Assert.assertTrue("disableChromeSync should notify delegate",
+                    syncSettingsDelegate.receivedNotification());
 
-            syncSettingsObserver.clearNotification();
+            syncSettingsDelegate.clearNotification();
             mAndroidSyncSettings.disableChromeSync();
-            Assert.assertFalse("disableChromeSync shouldn't observers",
-                    syncSettingsObserver.receivedNotification());
+            Assert.assertFalse("disableChromeSync shouldn't notify delegate",
+                    syncSettingsDelegate.receivedNotification());
         });
     }