ProfileSyncService.java cleanups

- Remove getNumberOfSyncedDevices which was unused.
- Merge init() into the constructor.
- Add a few comments/TODOs where the Java PSS still diverges from the
  native version.
- Add warning comments to some potentially dangerous methods (notably
  getChosenDataTypes and getPreferredDataTypes).
- Mark getPreferredDataTypes as deprecated. Its only usage was an error
  (see crbug.com/1157046).

Bug: 1135552
Change-Id: I8d0d1e8165086ef6af498a2010a887ba25ae7efd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582083Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836034}

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

@@ -40,6 +40,12 @@ public class ProfileSyncService {
         public void syncStateChanged();
     }
 
+    /**
+     * ModelTypes that the user can directly select in settings.
+     * Logically, this is a subset of the native UserSelectableTypeSet, but it
+     * uses values from the ModelType enum instead.
+     * TODO(crbug.com/985290): Resolve this inconsistency.
+     */
     private static final int[] ALL_SELECTABLE_TYPES = new int[] {
         ModelType.AUTOFILL,
         ModelType.BOOKMARKS,
@@ -104,15 +110,6 @@ public class ProfileSyncService {
     }
 
     protected ProfileSyncService() {
-        init();
-    }
-
-    /**
-     * This is called pretty early in our application. Avoid any blocking operations here. init()
-     * is a separate function to enable a test subclass of ProfileSyncService to completely stub out
-     * ProfileSyncService.
-     */
-    protected void init() {
         ThreadUtils.assertOnUiThread();
 
         // This may cause us to create ProfileSyncService even if sync has not
@@ -123,7 +120,11 @@ public class ProfileSyncService {
     }
 
     /**
-     * Checks if the sync engine is initialized.
+     * Checks if the sync engine is initialized. Note that this refers to
+     * Sync-the-transport, i.e. it can be true even if the user has *not*
+     * enabled Sync-the-feature.
+     * This mostly needs to be checked as a precondition for the various
+     * encryption-related methods (see below).
      *
      * @return true if the sync engine is initialized.
      */
@@ -212,10 +213,13 @@ public class ProfileSyncService {
     }
 
     /**
+     * DEPRECATED: Use getChosenDataTypes() instead.
+     *
      * Gets the set of data types that are "preferred" in sync. Those are the
      * chosen ones (see getChosenDataTypes), plus any that are implied by them.
      *
-     * This is unaffected by whether sync is on.
+     * NOTE: This returns "all types" by default, even if the user has never
+     *       enabled Sync, or if only Sync-the-transport is running.
      *
      * @return Set of preferred data types.
      */
@@ -239,10 +243,15 @@ public class ProfileSyncService {
     }
 
     /**
-     * Gets the set of data types that are enabled in sync. This will always
-     * return a subset of syncer::UserSelectableTypes().
+     * Gets the set of data types that the user has chosen to enable. This
+     * corresponds to the native GetSelectedTypes() / UserSelectableTypeSet, but
+     * every UserSelectableType is mapped to the corresponding canonical
+     * ModelType.
+     * TODO(crbug.com/985290): Expose UserSelectableType to Java and return that
+     * instead.
      *
-     * This is unaffected by whether sync is on.
+     * NOTE: This returns "all types" by default, even if the user has never
+     *       enabled Sync, or if only Sync-the-transport is running.
      *
      * @return Set of chosen types.
      */
@@ -555,16 +564,6 @@ public class ProfileSyncService {
                 mNativeProfileSyncServiceAndroid, ProfileSyncService.this, sessionTag);
     }
 
-    /**
-     * Gets the number of devices known to sync.
-     *
-     * @return number of syncing devices
-     */
-    public int getNumberOfSyncedDevices() {
-        return ProfileSyncServiceJni.get().getNumberOfSyncedDevices(
-                mNativeProfileSyncServiceAndroid, ProfileSyncService.this);
-    }
-
     /**
      * Records TrustedVaultKeyRetrievalTrigger histogram.
      */
@@ -704,8 +703,6 @@ public class ProfileSyncService {
                 long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
         String getSyncEnterCustomPassphraseBodyText(
                 long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
-        int getNumberOfSyncedDevices(
-                long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
         int[] getActiveDataTypes(long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
         int[] getChosenDataTypes(long nativeProfileSyncServiceAndroid, ProfileSyncService caller);
         int[] getPreferredDataTypes(

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

@@ -20,7 +20,6 @@ import java.util.Set;
  */
 public class FakeProfileSyncService extends ProfileSyncService {
     private boolean mEngineInitialized;
-    private int mNumberOfSyncedDevices;
     private boolean mPassphraseRequiredForPreferredDataTypes;
     private boolean mTrustedVaultKeyRequired;
     private boolean mTrustedVaultKeyRequiredForPreferredDataTypes;
@@ -68,15 +67,6 @@ public class FakeProfileSyncService extends ProfileSyncService {
         return true;
     }
 
-    @Override
-    public int getNumberOfSyncedDevices() {
-        return mNumberOfSyncedDevices;
-    }
-
-    public void setNumberOfSyncedDevices(int numDevices) {
-        mNumberOfSyncedDevices = numDevices;
-    }
-
     @Override
     public void setChosenDataTypes(boolean syncEverything, Set<Integer> enabledTypes) {
         mChosenTypes = enabledTypes;

chrome/browser/sync/profile_sync_service_android.cc

@@ -20,7 +20,6 @@
 #include "chrome/android/chrome_jni_headers/ProfileSyncService_jni.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/sync/device_info_sync_service_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/session_sync_service_factory.h"
 #include "chrome/grit/generated_resources.h"
@@ -31,9 +30,6 @@
 #include "components/sync/base/user_selectable_type.h"
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_service_utils.h"
-#include "components/sync_device_info/device_info.h"
-#include "components/sync_device_info/device_info_sync_service.h"
-#include "components/sync_device_info/device_info_tracker.h"
 #include "components/sync_sessions/session_sync_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "google_apis/gaia/google_service_auth_error.h"
@@ -485,20 +481,6 @@ ProfileSyncServiceAndroid::GetCurrentSignedInAccountText(
                                       base::ASCIIToUTF16(sync_username)));
 }
 
-jint ProfileSyncServiceAndroid::GetNumberOfSyncedDevices(
-    JNIEnv* env,
-    const JavaParamRef<jobject>&) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  syncer::DeviceInfoSyncService* device_sync_service =
-      DeviceInfoSyncServiceFactory::GetForProfile(profile_);
-  if (!device_sync_service) {
-    return 0;
-  }
-  const std::vector<std::unique_ptr<syncer::DeviceInfo>> all_devices =
-      device_sync_service->GetDeviceInfoTracker()->GetAllDeviceInfo();
-  return all_devices.size();
-}
-
 ScopedJavaLocalRef<jstring>
 ProfileSyncServiceAndroid::GetSyncEnterCustomPassphraseBodyText(
     JNIEnv* env,

chrome/browser/sync/profile_sync_service_android.h

@@ -156,10 +156,6 @@ class ProfileSyncServiceAndroid : public syncer::SyncServiceObserver {
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& obj);
 
-  jint GetNumberOfSyncedDevices(
-      JNIEnv* env,
-      const base::android::JavaParamRef<jobject>& obj);
-
   // UI string getters.
 
   base::android::ScopedJavaLocalRef<jstring>