Add reloadAccountsFromSystem on IdentityManager.java

This adds reloadAccountsFromSystem on IdentityManager.java and updates
SigninManager.java to use this instead of going through
signin_manager_android.cc. This is the last IdentityManager access from
signin_manager_android.cc remaining except for the construction of
SigninManager.java.

Bug: 934688
Change-Id: I3280102e03cb7b67b0f5ab3c569929854cc647bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1789385
Commit-Queue: Pâris Meuleman <pmeuleman@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Auto-Submit: Pâris Meuleman <pmeuleman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695978}

chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java

@@ -34,7 +34,7 @@ import org.chromium.components.signin.ChromeSigninController;
 import org.chromium.components.signin.identitymanager.ClearAccountsAction;
 import org.chromium.components.signin.identitymanager.CoreAccountInfo;
 import org.chromium.components.signin.identitymanager.IdentityManager;
-import org.chromium.components.signin.identitymanager.PrimaryAccountMutator;
+import org.chromium.components.signin.identitymanager.IdentityMutator;
 import org.chromium.components.signin.metrics.SigninAccessPoint;
 import org.chromium.components.signin.metrics.SigninReason;
 import org.chromium.components.signin.metrics.SignoutDelete;
@@ -200,7 +200,7 @@ public class SigninManager
     private final Context mContext;
     private final AccountTrackerService mAccountTrackerService;
     private final IdentityManager mIdentityManager;
-    private final PrimaryAccountMutator mPrimaryAccountMutator;
+    private final IdentityMutator mIdentityMutator;
     private final AndroidSyncSettings mAndroidSyncSettings;
     private final ObserverList<SignInStateObserver> mSignInStateObservers = new ObserverList<>();
     private final ObserverList<SignInAllowedObserver> mSignInAllowedObservers =
@@ -255,7 +255,7 @@ public class SigninManager
         mNativeSigninManagerAndroid = nativeSigninManagerAndroid;
         mAccountTrackerService = accountTrackerService;
         mIdentityManager = identityManager;
-        mPrimaryAccountMutator = identityManager.getPrimaryAccountMutator();
+        mIdentityMutator = identityManager.getIdentityMutator();
         mAndroidSyncSettings = androidSyncSettings;
 
         mSigninAllowedByPolicy =
@@ -502,7 +502,7 @@ public class SigninManager
         // This method should be called at most once per sign-in flow.
         assert mSignInState != null && mSignInState.mCoreAccountInfo != null;
 
-        if (!mPrimaryAccountMutator.setPrimaryAccount(mSignInState.mCoreAccountInfo.getId())) {
+        if (!mIdentityMutator.setPrimaryAccount(mSignInState.mCoreAccountInfo.getId())) {
             Log.w(TAG, "Failed to set the PrimaryAccount in IdentityManager, aborting signin");
             abortSignIn();
             return;
@@ -519,7 +519,7 @@ public class SigninManager
         }
 
         // Trigger token requests via native.
-        logInSignedInUser();
+        mIdentityMutator.reloadAccountsFromSystem();
 
         if (mSignInState.isInteractive()) {
             // If signin was a user action, record that it succeeded.
@@ -545,7 +545,7 @@ public class SigninManager
      * Implements {@link IdentityManager.Observer}: take action when primary account is set.
      * Simply verify that the request is ongoing (mSignInState != null), as only SigninManager
      * should update IdentityManager. This is triggered by the call to
-     * PrimaryAccountMutator.setPrimaryAccount
+     * IdentityMutator.setPrimaryAccount
      */
     @VisibleForTesting
     @Override
@@ -637,7 +637,7 @@ public class SigninManager
 
         // User data will be wiped in disableSyncAndWipeData(), called from
         // onPrimaryAccountcleared().
-        mPrimaryAccountMutator.clearPrimaryAccount(ClearAccountsAction.DEFAULT, signoutSource,
+        mIdentityMutator.clearPrimaryAccount(ClearAccountsAction.DEFAULT, signoutSource,
                 // Always use IGNORE_METRIC for the profile deletion argument. Chrome
                 // Android has just a single-profile which is never deleted upon
                 // sign-out.
@@ -651,11 +651,6 @@ public class SigninManager
         return SigninManagerJni.get().getManagementDomain(mNativeSigninManagerAndroid);
     }
 
-    @VisibleForTesting
-    void logInSignedInUser() {
-        SigninManagerJni.get().logInSignedInUser(mNativeSigninManagerAndroid);
-    }
-
     public void clearLastSignedInUser() {
         SigninManagerJni.get().clearLastSignedInUser(mNativeSigninManagerAndroid);
     }
@@ -801,8 +796,6 @@ public class SigninManager
 
         void clearLastSignedInUser(long nativeSigninManagerAndroid);
 
-        void logInSignedInUser(long nativeSigninManagerAndroid);
-
         String extractDomainName(String email);
 
         boolean isMobileIdentityConsistencyEnabled();

chrome/android/junit/src/org/chromium/chrome/browser/signin/SigninManagerTest.java

@@ -37,7 +37,7 @@ import org.chromium.components.signin.AccountTrackerService;
 import org.chromium.components.signin.identitymanager.CoreAccountId;
 import org.chromium.components.signin.identitymanager.CoreAccountInfo;
 import org.chromium.components.signin.identitymanager.IdentityManager;
-import org.chromium.components.signin.identitymanager.PrimaryAccountMutator;
+import org.chromium.components.signin.identitymanager.IdentityMutator;
 import org.chromium.components.signin.metrics.SignoutReason;
 import org.chromium.components.sync.AndroidSyncSettings;
 
@@ -55,7 +55,7 @@ public class SigninManagerTest {
 
     private AccountTrackerService mAccountTrackerService;
     private IdentityManager mIdentityManager;
-    private PrimaryAccountMutator mPrimaryAccountMutator;
+    private IdentityMutator mIdentityMutator;
     private SigninManager mSigninManager;
     private CoreAccountInfo mAccount;
 
@@ -69,10 +69,10 @@ public class SigninManagerTest {
 
         mAccountTrackerService = mock(AccountTrackerService.class);
 
-        mPrimaryAccountMutator = mock(PrimaryAccountMutator.class);
+        mIdentityMutator = mock(IdentityMutator.class);
 
         mIdentityManager =
-                spy(new IdentityManager(0 /* nativeIdentityManager */, mPrimaryAccountMutator));
+                spy(new IdentityManager(0 /* nativeIdentityManager */, mIdentityMutator));
 
         AndroidSyncSettings androidSyncSettings = mock(AndroidSyncSettings.class);
 
@@ -192,7 +192,7 @@ public class SigninManagerTest {
             mIdentityManager.onPrimaryAccountCleared(mAccount);
             return null;
         })
-                .when(mPrimaryAccountMutator)
+                .when(mIdentityMutator)
                 .clearPrimaryAccount(anyInt(), anyInt(), anyInt());
 
         mSigninManager.signOut(SignoutReason.SIGNOUT_TEST);
@@ -221,8 +221,8 @@ public class SigninManagerTest {
         doReturn(account)
                 .when(mIdentityManager)
                 .findExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(any());
-        doReturn(true).when(mPrimaryAccountMutator).setPrimaryAccount(any());
-        doNothing().when(mNativeMock).logInSignedInUser(anyLong());
+        doReturn(true).when(mIdentityMutator).setPrimaryAccount(any());
+        doNothing().when(mIdentityMutator).reloadAccountsFromSystem();
 
         mSigninManager.onFirstRunCheckDone(); // Allow sign-in.
 

chrome/browser/android/signin/signin_manager_android.cc

@@ -165,13 +165,6 @@ void SigninManagerAndroid::ClearLastSignedInUser(JNIEnv* env) {
   ClearLastSignedInUserForProfile(profile_);
 }
 
-void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env) {
-  // With the account consistency enabled let the account Reconcilor handles
-  // everything.
-  // TODO(https://crbug.com/930094): Determine the right long-term flow here.
-  identity_manager_->LegacyReloadAccountsFromSystem();
-}
-
 bool SigninManagerAndroid::IsSigninAllowed() const {
   return signin_allowed_.GetValue();
 }

chrome/browser/android/signin/signin_manager_android.h

@@ -45,8 +45,6 @@ class SigninManagerAndroid : public KeyedService {
 
   base::android::ScopedJavaLocalRef<jobject> GetJavaObject();
 
-  void LogInSignedInUser(JNIEnv* env);
-
   void ClearLastSignedInUser(JNIEnv* env);
 
   jboolean IsSigninAllowedByPolicy(JNIEnv* env) const;

components/signin/internal/identity_manager/android/BUILD.gn

@@ -6,6 +6,6 @@ generate_jni("jni_headers") {
     "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java",
     "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java",
     "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java",
-    "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/PrimaryAccountMutator.java",
+    "//components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java",
   ]
 }

components/signin/public/identity_manager/android/BUILD.gn

@@ -18,7 +18,7 @@ android_library("java") {
     "java/src/org/chromium/components/signin/identitymanager/CoreAccountId.java",
     "java/src/org/chromium/components/signin/identitymanager/CoreAccountInfo.java",
     "java/src/org/chromium/components/signin/identitymanager/IdentityManager.java",
-    "java/src/org/chromium/components/signin/identitymanager/PrimaryAccountMutator.java",
+    "java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java",
   ]
 
   annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]

components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityManager.java

@@ -37,27 +37,26 @@ public class IdentityManager {
     }
 
     private long mNativeIdentityManager;
-    private PrimaryAccountMutator mPrimaryAccountMutator;
+    private IdentityMutator mIdentityMutator;
 
     private final ObserverList<Observer> mObservers = new ObserverList<>();
 
     /**
      * Called by native to create an instance of IdentityManager.
-     * @param primaryAccountMutator can be null if native's IdentityManager received a null
-     * PrimaryAccountMutator, this happens in tests.
+     * @param identityMutator can be null if native's IdentityManager received a null
+     * IdentityMutator, this happens in tests.
      */
     @CalledByNative
     static private IdentityManager create(
-            long nativeIdentityManager, @Nullable PrimaryAccountMutator primaryAccountMutator) {
+            long nativeIdentityManager, @Nullable IdentityMutator identityMutator) {
         assert nativeIdentityManager != 0;
-        return new IdentityManager(nativeIdentityManager, primaryAccountMutator);
+        return new IdentityManager(nativeIdentityManager, identityMutator);
     }
 
     @VisibleForTesting
-    public IdentityManager(
-            long nativeIdentityManager, PrimaryAccountMutator primaryAccountMutator) {
+    public IdentityManager(long nativeIdentityManager, IdentityMutator identityMutator) {
         mNativeIdentityManager = nativeIdentityManager;
-        mPrimaryAccountMutator = primaryAccountMutator;
+        mIdentityMutator = identityMutator;
     }
 
     /**
@@ -125,8 +124,8 @@ public class IdentityManager {
      * Returns pointer to the object used to change the signed-in state of the
      * primary account.
      */
-    public PrimaryAccountMutator getPrimaryAccountMutator() {
-        return mPrimaryAccountMutator;
+    public IdentityMutator getIdentityMutator() {
+        return mIdentityMutator;
     }
 
     @NativeMethods

components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/PrimaryAccountMutator.java → components/signin/public/identity_manager/android/java/src/org/chromium/components/signin/identitymanager/IdentityMutator.java

@@ -10,18 +10,21 @@ import org.chromium.components.signin.metrics.SignoutDelete;
 import org.chromium.components.signin.metrics.SignoutReason;
 
 /**
- * PrimaryAccountMutator is the interface to set and clear the primary account (see IdentityManager
- * for more information).
+ * IdentityMutator is the write interface of IdentityManager, see identity_manager.h and
+ * primary_account_mutator.h for more information.
  */
-public class PrimaryAccountMutator {
-    private static final String TAG = "PrimaryAccountMuta";
+public class IdentityMutator {
+    private static final String TAG = "IdentityMutator";
 
     private final long mNativePrimaryAccountMutator;
+    private final long mNativeIdentityManager;
 
     @CalledByNative
-    private PrimaryAccountMutator(long nativePrimaryAccountMutator) {
+    private IdentityMutator(long nativePrimaryAccountMutator, long nativeIdentityManager) {
         assert nativePrimaryAccountMutator != 0;
+        assert nativeIdentityManager != 0;
         mNativePrimaryAccountMutator = nativePrimaryAccountMutator;
+        mNativeIdentityManager = nativeIdentityManager;
     }
 
     /**
@@ -33,8 +36,7 @@ public class PrimaryAccountMutator {
      *   - there is not already a primary account set.
      */
     public boolean setPrimaryAccount(CoreAccountId accountId) {
-        return PrimaryAccountMutatorJni.get().setPrimaryAccount(
-                mNativePrimaryAccountMutator, accountId);
+        return IdentityMutatorJni.get().setPrimaryAccount(mNativePrimaryAccountMutator, accountId);
     }
 
     /**
@@ -43,15 +45,24 @@ public class PrimaryAccountMutator {
      */
     public boolean clearPrimaryAccount(@ClearAccountsAction int action,
             @SignoutReason int sourceMetric, @SignoutDelete int deleteMetric) {
-        return PrimaryAccountMutatorJni.get().clearPrimaryAccount(
+        return IdentityMutatorJni.get().clearPrimaryAccount(
                 mNativePrimaryAccountMutator, action, sourceMetric, deleteMetric);
     }
 
+    /**
+     * Reloads the accounts in the token service from the system accounts. This API calls
+     * ProfileOAuth2TokenServiceDelegate::ReloadAccountsFromSystem.
+     */
+    public void reloadAccountsFromSystem() {
+        IdentityMutatorJni.get().reloadAccountsFromSystem(mNativeIdentityManager);
+    }
+
     @NativeMethods
     interface Natives {
         public boolean setPrimaryAccount(long nativePrimaryAccountMutator, CoreAccountId accountId);
         public boolean clearPrimaryAccount(long nativePrimaryAccountMutator,
                 @ClearAccountsAction int action, @SignoutReason int sourceMetric,
                 @SignoutDelete int deleteMetric);
+        public void reloadAccountsFromSystem(long nativeIdentityManager);
     }
 }

components/signin/public/identity_manager/identity_manager.cc

@@ -25,6 +25,7 @@
 #if defined(OS_ANDROID)
 #include "base/android/jni_string.h"
 #include "components/signin/internal/identity_manager/android/jni_headers/IdentityManager_jni.h"
+#include "components/signin/internal/identity_manager/android/jni_headers/IdentityMutator_jni.h"
 #include "components/signin/internal/identity_manager/oauth2_token_service_delegate_android.h"
 #elif !defined(OS_IOS)
 #include "components/signin/internal/identity_manager/mutable_profile_oauth2_token_service_delegate.h"
@@ -105,12 +106,17 @@ IdentityManager::IdentityManager(
     UpdateUnconsentedPrimaryAccount();
 
 #if defined(OS_ANDROID)
-  base::android::ScopedJavaLocalRef<jobject> java_primary_account_mutator =
-      primary_account_mutator_ ? primary_account_mutator_->GetJavaObject()
-                               : nullptr;
+  base::android::ScopedJavaLocalRef<jobject> java_identity_mutator =
+      primary_account_mutator_
+          ? Java_IdentityMutator_Constructor(
+                base::android::AttachCurrentThread(),
+                reinterpret_cast<intptr_t>(primary_account_mutator_.get()),
+                reinterpret_cast<intptr_t>(this))
+          : nullptr;
+
   java_identity_manager_ = Java_IdentityManager_create(
       base::android::AttachCurrentThread(), reinterpret_cast<intptr_t>(this),
-      java_primary_account_mutator);
+      java_identity_mutator);
 #endif
 }
 
@@ -448,6 +454,10 @@ base::android::ScopedJavaLocalRef<jobject> IdentityManager::
     return nullptr;
   return ConvertToJavaCoreAccountInfo(env, account_info.value());
 }
+
+void IdentityManager::ReloadAccountsFromSystem(JNIEnv* env) {
+  LegacyReloadAccountsFromSystem();
+}
 #endif
 
 PrimaryAccountManager* IdentityManager::GetPrimaryAccountManager() {

components/signin/public/identity_manager/identity_manager.h

@@ -464,6 +464,8 @@ class IdentityManager : public KeyedService,
   FindExtendedAccountInfoForAccountWithRefreshTokenByEmailAddress(
       JNIEnv* env,
       const base::android::JavaParamRef<jstring>& j_email) const;
+
+  void ReloadAccountsFromSystem(JNIEnv* env);
 #endif
 
  private:

components/signin/public/identity_manager/primary_account_mutator.cc

@@ -8,17 +8,11 @@
 
 #if defined(OS_ANDROID)
 #include "base/android/jni_string.h"
-#include "components/signin/internal/identity_manager/android/jni_headers/PrimaryAccountMutator_jni.h"
 #endif
 
 namespace signin {
 
-PrimaryAccountMutator::PrimaryAccountMutator() {
-#if defined(OS_ANDROID)
-  java_primary_account_mutator_ = Java_PrimaryAccountMutator_Constructor(
-      base::android::AttachCurrentThread(), reinterpret_cast<intptr_t>(this));
-#endif
-}
+PrimaryAccountMutator::PrimaryAccountMutator() {}
 
 PrimaryAccountMutator::~PrimaryAccountMutator() {}
 
@@ -38,13 +32,6 @@ bool PrimaryAccountMutator::ClearPrimaryAccount(JNIEnv* env,
       static_cast<signin_metrics::ProfileSignout>(source_metric),
       static_cast<signin_metrics::SignoutDelete>(delete_metric));
 }
-
-base::android::ScopedJavaLocalRef<jobject>
-PrimaryAccountMutator::GetJavaObject() {
-  DCHECK(java_primary_account_mutator_);
-  return base::android::ScopedJavaLocalRef<jobject>(
-      java_primary_account_mutator_);
-}
 #endif
 
 }  // namespace signin