Make SyncContentResolverDelegate package-private

Some code that has minimal dependencies on AndroidSyncSettings depends
unnecessarily on SyncContentResolverDelegate in its tests. This CL
removes these dependencies (either by mocking AndroidSyncSettings or by
using AndroidSyncSettingsTestUtils) and SyncContentResolverDelegate is
made a package-private class.

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

chrome/android/BUILD.gn

@@ -858,7 +858,6 @@ junit_binary("chrome_junit_tests") {
     "//components/signin/core/browser/android:java",
     "//components/signin/core/browser/android:signin_java_test_support",
     "//components/signin/public/android:java",
-    "//components/sync:sync_java_test_support",
     "//components/sync/android:sync_java",
     "//components/url_formatter/android:url_formatter_java",
     "//components/variations/android:variations_java",
@@ -1113,7 +1112,6 @@ android_library("chrome_test_java") {
     "//components/signin/core/browser/android:signin_javatests",
     "//components/signin/public/android:java",
     "//components/strictmode/android:javatests",
-    "//components/sync:sync_java_test_support",
     "//components/sync/android:sync_java",
     "//components/sync/protocol:protocol_java",
     "//components/translate/content/android:javatests",

chrome/android/chrome_java_sources.gni

@@ -1453,9 +1453,11 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/survey/SurveyController.java",
   "java/src/org/chromium/chrome/browser/sync/AndroidSyncSettings.java",
   "java/src/org/chromium/chrome/browser/sync/ProfileSyncService.java",
+  "java/src/org/chromium/chrome/browser/sync/SyncContentResolverDelegate.java",
   "java/src/org/chromium/chrome/browser/sync/SyncController.java",
   "java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java",
   "java/src/org/chromium/chrome/browser/sync/SyncUserDataWiper.java",
+  "java/src/org/chromium/chrome/browser/sync/SystemSyncContentResolverDelegate.java",
   "java/src/org/chromium/chrome/browser/sync/TrustedVaultClient.java",
   "java/src/org/chromium/chrome/browser/sync/settings/AccountManagementFragment.java",
   "java/src/org/chromium/chrome/browser/sync/settings/ClearDataProgressDialog.java",

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

@@ -26,8 +26,6 @@ import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.base.CoreAccountInfo;
 import org.chromium.components.signin.identitymanager.ConsentLevel;
 import org.chromium.components.signin.identitymanager.IdentityManager;
-import org.chromium.components.sync.SyncContentResolverDelegate;
-import org.chromium.components.sync.SystemSyncContentResolverDelegate;
 import org.chromium.content_public.browser.UiThreadTaskTraits;
 
 /**
@@ -94,7 +92,7 @@ public class AndroidSyncSettings {
      * @param syncContentResolverDelegate an implementation of {@link SyncContentResolverDelegate}.
      */
     @VisibleForTesting
-    public AndroidSyncSettings(SyncContentResolverDelegate syncContentResolverDelegate) {
+    AndroidSyncSettings(SyncContentResolverDelegate syncContentResolverDelegate) {
         this(syncContentResolverDelegate, getSyncAccount());
     }
 
@@ -110,7 +108,7 @@ public class AndroidSyncSettings {
     // (in that case, a new object would be returned, not the one constructed by the test).
     // Consider exposing them as static methods that also register a singleton instance.
     @VisibleForTesting
-    public AndroidSyncSettings(
+    AndroidSyncSettings(
             SyncContentResolverDelegate syncContentResolverDelegate, @Nullable Account account) {
         mContractAuthority = getContractAuthority();
         mSyncContentResolverDelegate = syncContentResolverDelegate;

components/sync/android/java/src/org/chromium/components/sync/SyncContentResolverDelegate.java → chrome/android/java/src/org/chromium/chrome/browser/sync/SyncContentResolverDelegate.java

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-package org.chromium.components.sync;
+package org.chromium.chrome.browser.sync;
 
 import android.accounts.Account;
 import android.content.SyncStatusObserver;
@@ -13,7 +13,7 @@ import android.os.Bundle;
  * mock out for tests. This interface wraps all the sync-related methods we use from
  * the Android ContentResolver.
  */
-public interface SyncContentResolverDelegate {
+interface SyncContentResolverDelegate {
     Object addStatusChangeListener(int mask, SyncStatusObserver callback);
 
     void removeStatusChangeListener(Object handle);

components/sync/android/java/src/org/chromium/components/sync/SystemSyncContentResolverDelegate.java → chrome/android/java/src/org/chromium/chrome/browser/sync/SystemSyncContentResolverDelegate.java

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-package org.chromium.components.sync;
+package org.chromium.chrome.browser.sync;
 
 import android.accounts.Account;
 import android.content.ContentResolver;
@@ -14,7 +14,7 @@ import android.os.Bundle;
  * Note that SyncContentResolverDelegate is not an Android concept. In particular,
  * it's not this class that will notify observers, Android will directly do that.
  */
-public class SystemSyncContentResolverDelegate implements SyncContentResolverDelegate {
+class SystemSyncContentResolverDelegate implements SyncContentResolverDelegate {
     @Override
     public Object addStatusChangeListener(int mask, SyncStatusObserver callback) {
         return ContentResolver.addStatusChangeListener(mask, callback);

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkTest.java

@@ -59,7 +59,7 @@ import org.chromium.chrome.browser.partnerbookmarks.PartnerBookmarksShim;
 import org.chromium.chrome.browser.preferences.ChromePreferenceKeys;
 import org.chromium.chrome.browser.preferences.SharedPreferencesManager;
 import org.chromium.chrome.browser.profiles.Profile;
-import org.chromium.chrome.browser.sync.AndroidSyncSettings;
+import org.chromium.chrome.browser.sync.AndroidSyncSettingsTestUtils;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.ui.messages.snackbar.Snackbar;
 import org.chromium.chrome.browser.ui.messages.snackbar.SnackbarManager;
@@ -77,7 +77,6 @@ import org.chromium.components.browser_ui.widget.listmenu.ListMenuButton;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectableListToolbar;
 import org.chromium.components.browser_ui.widget.selectable_list.SelectableListToolbar.ViewType;
 import org.chromium.components.embedder_support.util.UrlConstants;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
@@ -121,7 +120,6 @@ public class BookmarkTest {
     private BookmarkManager mManager;
     private BookmarkModel mBookmarkModel;
     private BookmarkBridge mBookmarkBridge;
-    private MockSyncContentResolverDelegate mSyncDelegate;
     private RecyclerView mItemsContainer;
     private String mTestPage;
     private String mTestPageFoo;
@@ -148,9 +146,7 @@ public class BookmarkTest {
             mBookmarkBridge = mActivityTestRule.getActivity().getBookmarkBridgeForTesting();
 
             // Stub Android master sync state to make sure promos aren't suppressed.
-            mSyncDelegate = new MockSyncContentResolverDelegate();
-            mSyncDelegate.setMasterSyncAutomatically(true);
-            AndroidSyncSettings.overrideForTests(new AndroidSyncSettings(mSyncDelegate));
+            AndroidSyncSettingsTestUtils.setUpAndroidSyncSettingsForTesting();
         });
         mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
         mTestPage = mTestServer.getURL(TEST_PAGE_URL_GOOGLE);

chrome/android/javatests/src/org/chromium/chrome/browser/browsing_data/ClearBrowsingDataFragmentBasicTest.java

@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.browsing_data;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.when;
 
 import androidx.preference.CheckBoxPreference;
 import androidx.preference.PreferenceScreen;
@@ -17,6 +18,9 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
 
 import org.chromium.base.CollectionUtil;
 import org.chromium.base.test.util.CommandLineFlags;
@@ -30,7 +34,6 @@ import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.signin.AccountManagerTestRule;
 import org.chromium.components.sync.ModelType;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.util.HashSet;
@@ -53,9 +56,14 @@ public class ClearBrowsingDataFragmentBasicTest {
     @Rule
     public final AccountManagerTestRule mAccountManagerTestRule = new AccountManagerTestRule();
 
+    @Rule
+    public final MockitoRule mMockitoRule = MockitoJUnit.rule();
+
     private static final String GOOGLE_ACCOUNT = "Google Account";
     private static final String OTHER_ACTIVITY = "other forms of browsing history";
     private static final String SIGNED_IN_DEVICES = "signed-in devices";
+    @Mock
+    private AndroidSyncSettings mAndroidSyncSettings;
 
     @Before
     public void setUp() throws InterruptedException {
@@ -84,14 +92,8 @@ public class ClearBrowsingDataFragmentBasicTest {
 
     private void setSyncable(final boolean syncable) {
         TestThreadUtils.runOnUiThreadBlocking(() -> {
-            MockSyncContentResolverDelegate delegate = new MockSyncContentResolverDelegate();
-            delegate.setMasterSyncAutomatically(syncable);
-            AndroidSyncSettings.overrideForTests(new AndroidSyncSettings(delegate));
-            if (syncable) {
-                AndroidSyncSettings.get().enableChromeSync();
-            } else {
-                AndroidSyncSettings.get().disableChromeSync();
-            }
+            when(mAndroidSyncSettings.isSyncEnabled()).thenReturn(syncable);
+            AndroidSyncSettings.overrideForTests(mAndroidSyncSettings);
 
             ProfileSyncService.overrideForTests(new StubProfileSyncService(syncable));
         });

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

@@ -31,7 +31,6 @@ import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.components.signin.AccountManagerFacadeProvider;
 import org.chromium.components.signin.AccountUtils;
 import org.chromium.components.signin.test.util.FakeAccountManagerFacade;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.util.concurrent.TimeoutException;

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

@@ -7,8 +7,6 @@ package org.chromium.chrome.browser.sync;
 import androidx.annotation.VisibleForTesting;
 
 import org.chromium.base.annotations.CalledByNative;
-import org.chromium.components.sync.SyncContentResolverDelegate;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
 import java.util.concurrent.Callable;
@@ -28,12 +26,15 @@ public class AndroidSyncSettingsTestUtils {
         setUpAndroidSyncSettingsForTesting(new MockSyncContentResolverDelegate());
     }
 
-    public static void setUpAndroidSyncSettingsForTesting(SyncContentResolverDelegate delegate) {
+    static AndroidSyncSettings setUpAndroidSyncSettingsForTesting(
+            SyncContentResolverDelegate delegate) {
         delegate.setMasterSyncAutomatically(true);
         // Explicitly pass null account to AndroidSyncSettings ctor. Normally, AndroidSyncSettings
         // ctor uses IdentityManager to get the sync account, but some native tests call this method
         // before profiles are initialized (when IdentityManager doesn't exist yet).
-        AndroidSyncSettings.overrideForTests(new AndroidSyncSettings(delegate, null));
+        AndroidSyncSettings settings = new AndroidSyncSettings(delegate, null);
+        AndroidSyncSettings.overrideForTests(settings);
+        return settings;
     }
 
     public static boolean getIsSyncEnabledOnUiThread() {

components/sync/test/android/javatests/src/org/chromium/components/sync/test/util/MockSyncContentResolverDelegate.java → chrome/android/javatests/src/org/chromium/chrome/browser/sync/MockSyncContentResolverDelegate.java

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-package org.chromium.components.sync.test.util;
+package org.chromium.chrome.browser.sync;
 
 import android.accounts.Account;
 import android.content.ContentResolver;
@@ -11,8 +11,6 @@ import android.os.Bundle;
 
 import androidx.annotation.VisibleForTesting;
 
-import org.chromium.components.sync.SyncContentResolverDelegate;
-
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -26,7 +24,7 @@ import java.util.Set;
  * observers for the SYNC_OBSERVER_TYPE_SETTINGS type and it doesn't allow querying
  * settings for a null account.
  */
-public class MockSyncContentResolverDelegate implements SyncContentResolverDelegate {
+class MockSyncContentResolverDelegate implements SyncContentResolverDelegate {
     private final Set<String> mSyncAutomaticallySet;
     private final Map<String, Boolean> mIsSyncableMap;
     private final Set<SyncStatusObserver> mObservers;
@@ -139,5 +137,4 @@ public class MockSyncContentResolverDelegate implements SyncContentResolverDeleg
             }
         }
     }
-
 }

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

@@ -27,7 +27,6 @@ import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.signin.MockChangeEventChecker;
 import org.chromium.chrome.test.util.browser.sync.SyncTestUtil;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.Criteria;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;

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

@@ -38,7 +38,6 @@ import org.chromium.components.sync.protocol.AutofillWalletSpecifics;
 import org.chromium.components.sync.protocol.EntitySpecifics;
 import org.chromium.components.sync.protocol.SyncEntity;
 import org.chromium.components.sync.protocol.WalletMaskedCreditCard;
-import org.chromium.components.sync.test.util.MockSyncContentResolverDelegate;
 import org.chromium.content_public.browser.test.util.CriteriaHelper;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 
@@ -159,7 +158,7 @@ public class SyncTestRule extends ChromeActivityTestRule<ChromeActivity> {
         return mProfileSyncService;
     }
 
-    public MockSyncContentResolverDelegate getSyncContentResolver() {
+    MockSyncContentResolverDelegate getSyncContentResolver() {
         return mSyncContentResolver;
     }
 

chrome/test/BUILD.gn

@@ -6546,7 +6546,6 @@ if (is_android) {
       "//chrome/browser/profiles/android:java",
       "//chrome/test/android:chrome_java_test_support",
       "//components/signin/public/android:java",
-      "//components/sync:sync_java_test_support",
       "//components/sync/android:sync_java",
       "//content/public/test/android:content_java_test_support",
       "//third_party/android_deps:androidx_annotation_annotation_java",
@@ -6554,6 +6553,7 @@ if (is_android) {
     ]
     sources = [
       "../android/javatests/src/org/chromium/chrome/browser/sync/AndroidSyncSettingsTestUtils.java",
+      "../android/javatests/src/org/chromium/chrome/browser/sync/MockSyncContentResolverDelegate.java",
       "../android/javatests/src/org/chromium/chrome/browser/sync/SyncTestSigninUtils.java",
     ]
   }

components/sync/BUILD.gn

@@ -591,18 +591,3 @@ source_set("unit_tests") {
     deps += [ "//chromeos/constants" ]
   }
 }
-
-if (is_android) {
-  import("//build/config/android/rules.gni")
-
-  android_library("sync_java_test_support") {
-    testonly = true
-    deps = [
-      "android:sync_java",
-      "//base:base_java",
-      "//third_party/android_deps:androidx_annotation_annotation_java",
-      "//third_party/junit",
-    ]
-    sources = [ "test/android/javatests/src/org/chromium/components/sync/test/util/MockSyncContentResolverDelegate.java" ]
-  }
-}

components/sync/android/BUILD.gn

@@ -20,8 +20,6 @@ android_library("sync_java") {
   sources = [
     "java/src/org/chromium/components/sync/Passphrase.java",
     "java/src/org/chromium/components/sync/SyncConstants.java",
-    "java/src/org/chromium/components/sync/SyncContentResolverDelegate.java",
-    "java/src/org/chromium/components/sync/SystemSyncContentResolverDelegate.java",
   ]
 }