Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(297)

Unified Diff: sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java

Issue 1138013008: [Sync] Make it impossible to get a reference to AndroidSyncSettings. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase and address nit. Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java
diff --git a/sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java b/sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java
index 3587e76bb1dfc41b24d7a037e5cb8d8a3ebca4dc..c36cef484eef50328b7077832ac8f6051137f981 100644
--- a/sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java
+++ b/sync/android/java/src/org/chromium/sync/AndroidSyncSettings.java
@@ -21,7 +21,9 @@ import javax.annotation.concurrent.ThreadSafe;
*
* It also provides an observer to be used whenever Android sync settings change.
*
- * To retrieve an instance of this class, call AndroidSyncSettings.get(someContext).
+ * This class is a collection of static methods so that no references to its object can be
+ * stored. This is important because tests need to be able to overwrite the object with a
+ * mock content resolver and know that no references to the old one are cached.
*
* This class must be initialized via updateAccount() on startup if the user is signed in.
*/
@@ -35,7 +37,7 @@ public class AndroidSyncSettings {
*/
private static final Object CLASS_LOCK = new Object();
- private static AndroidSyncSettings sAndroidSyncSettings;
+ private static AndroidSyncSettings sInstance;
private final Object mLock = new Object();
@@ -63,37 +65,26 @@ public class AndroidSyncSettings {
public void androidSyncSettingsChanged();
}
- /**
- * A factory method for the AndroidSyncSettings.
- *
- * It is possible to override the {@link SyncContentResolverDelegate} to use in tests for the
- * instance of the AndroidSyncSettings by calling overrideForTests(...) with
- * your {@link SyncContentResolverDelegate}.
- *
- * @param context the ApplicationContext is retrieved from the context used as an argument.
- * @return a singleton instance of the AndroidSyncSettings
- */
- public static AndroidSyncSettings get(Context context) {
+ private static void ensureInitialized(Context context) {
synchronized (CLASS_LOCK) {
- if (sAndroidSyncSettings == null) {
+ if (sInstance == null) {
SyncContentResolverDelegate contentResolver =
new SystemSyncContentResolverDelegate();
- sAndroidSyncSettings = new AndroidSyncSettings(context, contentResolver);
+ sInstance = new AndroidSyncSettings(context, contentResolver);
}
}
- return sAndroidSyncSettings;
}
@VisibleForTesting
public static void overrideForTests(Context context,
SyncContentResolverDelegate contentResolver) {
synchronized (CLASS_LOCK) {
- sAndroidSyncSettings = new AndroidSyncSettings(context, contentResolver);
+ sInstance = new AndroidSyncSettings(context, contentResolver);
}
}
/**
- * @param context the context
+ * @param context the context the ApplicationContext will be retrieved from.
* @param syncContentResolverDelegate an implementation of {@link SyncContentResolverDelegate}.
*/
private AndroidSyncSettings(Context context,
@@ -117,8 +108,9 @@ public class AndroidSyncSettings {
*
* @return true if sync is on, false otherwise
*/
- public boolean isSyncEnabled() {
- return mMasterSyncEnabled && mChromeSyncEnabled;
+ public static boolean isSyncEnabled(Context context) {
+ ensureInitialized(context);
+ return sInstance.mMasterSyncEnabled && sInstance.mChromeSyncEnabled;
}
/**
@@ -129,66 +121,74 @@ public class AndroidSyncSettings {
*
* @return true if sync is on, false otherwise
*/
- public boolean isChromeSyncEnabled() {
- return mChromeSyncEnabled;
+ public static boolean isChromeSyncEnabled(Context context) {
+ ensureInitialized(context);
+ return sInstance.mChromeSyncEnabled;
}
/**
* Checks whether the master sync flag for Android is currently enabled.
*/
- public boolean isMasterSyncEnabled() {
- return mMasterSyncEnabled;
+ public static boolean isMasterSyncEnabled(Context context) {
+ ensureInitialized(context);
+ return sInstance.mMasterSyncEnabled;
}
/**
* Make sure Chrome is syncable, and enable sync.
*/
- public void enableChromeSync() {
- setChromeSyncEnabled(true);
+ public static void enableChromeSync(Context context) {
+ ensureInitialized(context);
+ sInstance.setChromeSyncEnabled(true);
}
/**
* Disables Android Chrome sync
*/
- public void disableChromeSync() {
- setChromeSyncEnabled(false);
+ public static void disableChromeSync(Context context) {
+ ensureInitialized(context);
+ sInstance.setChromeSyncEnabled(false);
}
/**
* Must be called when a new account is signed in.
*/
- public void updateAccount(Account account) {
- synchronized (mLock) {
- mAccount = account;
- updateSyncability();
+ public static void updateAccount(Context context, Account account) {
+ ensureInitialized(context);
+ synchronized (sInstance.mLock) {
+ sInstance.mAccount = account;
+ sInstance.updateSyncability();
}
- if (updateCachedSettings()) {
- notifyObservers();
+ if (sInstance.updateCachedSettings()) {
+ sInstance.notifyObservers();
}
}
/**
* Returns the contract authority to use when requesting sync.
*/
- public String getContractAuthority() {
- return mApplicationContext.getPackageName();
+ public static String getContractAuthority(Context context) {
+ ensureInitialized(context);
+ return sInstance.getContractAuthority();
}
/**
* Add a new AndroidSyncSettingsObserver.
*/
- public void registerObserver(AndroidSyncSettingsObserver observer) {
- synchronized (mLock) {
- mObservers.addObserver(observer);
+ public static void registerObserver(Context context, AndroidSyncSettingsObserver observer) {
+ ensureInitialized(context);
+ synchronized (sInstance.mLock) {
+ sInstance.mObservers.addObserver(observer);
}
}
/**
* Remove an AndroidSyncSettingsObserver that was previously added.
*/
- public void unregisterObserver(AndroidSyncSettingsObserver observer) {
- synchronized (mLock) {
- mObservers.removeObserver(observer);
+ public static void unregisterObserver(Context context, AndroidSyncSettingsObserver observer) {
+ ensureInitialized(context);
+ synchronized (sInstance.mLock) {
+ sInstance.mObservers.removeObserver(observer);
}
}
@@ -290,4 +290,68 @@ public class AndroidSyncSettings {
observer.androidSyncSettingsChanged();
}
}
+
+ // TODO(maxbogue): make private once downstream uses are removed.
+ @Deprecated
+ public String getContractAuthority() {
+ return mApplicationContext.getPackageName();
+ }
+
+ // Deprecated section; to be removed once downstream no longer uses them.
+
+ @Deprecated
+ public static AndroidSyncSettings get(Context context) {
+ ensureInitialized(context);
+ return sInstance;
+ }
+
+ @Deprecated
+ public boolean isSyncEnabled() {
+ return mMasterSyncEnabled && mChromeSyncEnabled;
+ }
+
+ @Deprecated
+ public boolean isChromeSyncEnabled() {
+ return mChromeSyncEnabled;
+ }
+
+ @Deprecated
+ public boolean isMasterSyncEnabled() {
+ return mMasterSyncEnabled;
+ }
+
+ @Deprecated
+ public void enableChromeSync() {
+ setChromeSyncEnabled(true);
+ }
+
+ @Deprecated
+ public void disableChromeSync() {
+ setChromeSyncEnabled(false);
+ }
+
+ @Deprecated
+ public void updateAccount(Account account) {
+ synchronized (mLock) {
+ mAccount = account;
+ updateSyncability();
+ }
+ if (updateCachedSettings()) {
+ notifyObservers();
+ }
+ }
+
+ @Deprecated
+ public void registerObserver(AndroidSyncSettingsObserver observer) {
+ synchronized (mLock) {
+ mObservers.addObserver(observer);
+ }
+ }
+
+ @Deprecated
+ public void unregisterObserver(AndroidSyncSettingsObserver observer) {
+ synchronized (mLock) {
+ mObservers.removeObserver(observer);
+ }
+ }
}

Powered by Google App Engine
This is Rietveld 408576698