[Signin][Android] Rollback support for MobileIdentityConsistency

Adds code to rollback MobileIdentityConsistency: when the feature is
disabled, SigninManager will trigger a signout if there's only primary
account, but no sync consent.

Bug: 1132290
Change-Id: Ia9f0143b908c1867743e0b89b7589ddbb03bff02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2434330
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811883}

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

@@ -23,6 +23,7 @@ import org.chromium.base.metrics.RecordUserAction;
 import org.chromium.base.task.PostTask;
 import org.chromium.chrome.browser.AppHooks;
 import org.chromium.chrome.browser.externalauth.ExternalAuthUtils;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.sync.AndroidSyncSettings;
 import org.chromium.components.signin.AccountTrackerService;
 import org.chromium.components.signin.AccountUtils;
@@ -271,6 +272,27 @@ public class SigninManager
         mIdentityManager.addObserver(this);
 
         reloadAllAccountsFromSystem();
+
+        maybeRollbackMobileIdentityConsistency();
+    }
+
+    /**
+     * Temporary code to handle rollback for {@link ChromeFeatureList#MOBILE_IDENTITY_CONSISTENCY}.
+     * TODO(https://crbug.com/1065029): Remove when the flag is removed.
+     */
+    private void maybeRollbackMobileIdentityConsistency() {
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)) return;
+        // Nothing to do if there's no primary account.
+        if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED) == null) return;
+        // Nothing to do if sync is on - this state existed before MobileIdentityConsistency.
+        if (mIdentityManager.getPrimaryAccountInfo(ConsentLevel.SYNC) != null) return;
+
+        Log.w(TAG, "Rolling back MobileIdentityConsistency: signing out.");
+        signOut(SignoutReason.MOBILE_IDENTITY_CONSISTENCY_ROLLBACK);
+        // Since AccountReconcilor currently operates in pre-MICE mode, it doesn't react to
+        // primary account changes when there's no sync consent. Log-out web accounts manually.
+        SigninManagerJni.get().logOutAllAccountsForMobileIdentityConsistencyRollback(
+                mNativeSigninManagerAndroid);
     }
 
     /**
@@ -818,6 +840,10 @@ public class SigninManager
 
         String getManagementDomain(long nativeSigninManagerAndroid);
 
+        // Temporary code to handle rollback for MobileIdentityConsistency.
+        // TODO(https://crbug.com/1065029): Remove when the flag is removed.
+        void logOutAllAccountsForMobileIdentityConsistencyRollback(long nativeSigninManagerAndroid);
+
         void wipeProfileData(long nativeSigninManagerAndroid, Runnable callback);
 
         void wipeGoogleServiceWorkerCaches(long nativeSigninManagerAndroid, Runnable callback);

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

@@ -30,7 +30,9 @@ import org.robolectric.annotation.Config;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.JniMocker;
 import org.chromium.chrome.browser.externalauth.ExternalAuthUtils;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.sync.AndroidSyncSettings;
+import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.components.signin.AccountTrackerService;
 import org.chromium.components.signin.base.AccountInfo;
 import org.chromium.components.signin.base.CoreAccountId;
@@ -48,9 +50,12 @@ import java.util.concurrent.atomic.AtomicInteger;
 /** Tests for {@link SigninManager}. */
 @RunWith(BaseRobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
+@Features.DisableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
 public class SigninManagerTest {
     @Rule
     public final JniMocker mocker = new JniMocker();
+    @Rule
+    public final Features.JUnitProcessor processor = new Features.JUnitProcessor();
 
     private static final AccountInfo ACCOUNT_INFO = new AccountInfo(
             new CoreAccountId("gaia-id-user"), "user@domain.com", "gaia-id-user", null);
@@ -250,6 +255,45 @@ public class SigninManagerTest {
         verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
     }
 
+    @Test
+    public void testRollbackForMobileIdentityConsitency() {
+        doReturn(ACCOUNT_INFO)
+                .when(mIdentityManager)
+                .getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED);
+        doReturn(null).when(mIdentityManager).getPrimaryAccountInfo(ConsentLevel.SYNC);
+        mIdentityManager.onAccountsCookieDeletedByUserAction();
+
+        // SignedIn state (without sync consent) doesn't exist pre-MobileIdentityConsistency. If the
+        // feature is disabled while in this state, SigninManager ctor should trigger sign-out.
+        createSigninManager();
+
+        verify(mIdentityMutator)
+                .clearPrimaryAccount(ClearAccountsAction.DEFAULT,
+                        SignoutReason.MOBILE_IDENTITY_CONSISTENCY_ROLLBACK,
+                        SignoutDelete.IGNORE_METRIC);
+        verify(mNativeMock).logOutAllAccountsForMobileIdentityConsistencyRollback(anyLong());
+
+        // This sign-out shouldn't wipe data.
+        verify(mNativeMock, never()).wipeProfileData(anyLong(), any());
+        verify(mNativeMock, never()).wipeGoogleServiceWorkerCaches(anyLong(), any());
+    }
+
+    @Test
+    @Features.EnableFeatures(ChromeFeatureList.MOBILE_IDENTITY_CONSISTENCY)
+    public void testNoRollbackIfMobileIdentityConsitencyIsEnabled() {
+        doReturn(ACCOUNT_INFO)
+                .when(mIdentityManager)
+                .getPrimaryAccountInfo(ConsentLevel.NOT_REQUIRED);
+        doReturn(null).when(mIdentityManager).getPrimaryAccountInfo(ConsentLevel.SYNC);
+        mIdentityManager.onAccountsCookieDeletedByUserAction();
+
+        createSigninManager();
+
+        verify(mIdentityMutator, never()).clearPrimaryAccount(anyInt(), anyInt(), anyInt());
+        verify(mNativeMock, never())
+                .logOutAllAccountsForMobileIdentityConsistencyRollback(anyLong());
+    }
+
     @Test
     public void callbackNotifiedWhenNoOperationIsInProgress() {
         createSigninManager();

chrome/browser/android/signin/signin_manager_android.cc

@@ -31,6 +31,7 @@
 #include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_pref_names.h"
+#include "components/signin/public/identity_manager/accounts_cookie_mutator.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browsing_data_filter_builder.h"
@@ -277,6 +278,13 @@ SigninManagerAndroid::GetManagementDomain(JNIEnv* env) {
   return domain;
 }
 
+void SigninManagerAndroid::
+    LogOutAllAccountsForMobileIdentityConsistencyRollback(JNIEnv* env) {
+  identity_manager_->GetAccountsCookieMutator()->LogOutAllAccounts(
+      gaia::GaiaSource::kAccountReconcilorMirror,
+      signin::AccountsCookieMutator::LogOutFromCookieCompletedCallback());
+}
+
 void SigninManagerAndroid::WipeProfileData(
     JNIEnv* env,
     const JavaParamRef<jobject>& j_callback) {

chrome/browser/android/signin/signin_manager_android.h

@@ -74,6 +74,10 @@ class SigninManagerAndroid : public KeyedService {
       JNIEnv* env,
       const base::android::JavaParamRef<jobject>& j_callback);
 
+  // Logs out all Google accounts on the web as a part of the rollback flow.
+  // TODO(https://crbug.com/1065029): Remove this along with the feature flag.
+  void LogOutAllAccountsForMobileIdentityConsistencyRollback(JNIEnv* env);
+
  private:
   friend class SigninManagerAndroidTest;
   FRIEND_TEST_ALL_PREFIXES(SigninManagerAndroidTest,

chrome/browser/signin/chrome_signin_client_unittest.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/signin/chrome_signin_client.h"
-
 #include <memory>
 #include <utility>
 
@@ -246,8 +245,9 @@ bool IsSignoutDisallowedByPolicy(
       // Allow signout for tests that want to force it.
       return false;
     case signin_metrics::ProfileSignout::USER_DELETED_ACCOUNT_COOKIES:
-      // There's no special-casing for this in ChromeSigninClient, as this only
-      // happens when there's no sync account and policies aren't enforced.
+    case signin_metrics::ProfileSignout::MOBILE_IDENTITY_CONSISTENCY_ROLLBACK:
+      // There's no special-casing for these in ChromeSigninClient, as they only
+      // happen when there's no sync account and policies aren't enforced.
       // PrimaryAccountManager won't actually invoke PreSignOut in this case,
       // thus it is fine for ChromeSigninClient to not have any special-casing.
       return true;
@@ -324,6 +324,7 @@ const signin_metrics::ProfileSignout kSignoutSources[] = {
     signin_metrics::ProfileSignout::SIGNIN_NOT_ALLOWED_ON_PROFILE_INIT,
     signin_metrics::ProfileSignout::FORCE_SIGNOUT_ALWAYS_ALLOWED_FOR_TEST,
     signin_metrics::ProfileSignout::USER_DELETED_ACCOUNT_COOKIES,
+    signin_metrics::ProfileSignout::MOBILE_IDENTITY_CONSISTENCY_ROLLBACK,
 };
 static_assert(base::size(kSignoutSources) ==
                   signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS,

components/signin/public/base/signin_metrics.h

@@ -62,6 +62,8 @@ enum ProfileSignout : int {
   // User cleared account cookies when there's no sync consent, which has caused
   // sign out.
   USER_DELETED_ACCOUNT_COOKIES,
+  // Signout triggered by MobileIdentityConsistency rollback.
+  MOBILE_IDENTITY_CONSISTENCY_ROLLBACK,
   // Keep this as the last enum.
   NUM_PROFILE_SIGNOUT_METRICS,
 };

tools/metrics/histograms/enums.xml

@@ -66082,6 +66082,10 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
     User cleared account cookies when there's no sync consent, which has caused
     sign out.
   </int>
+  <int value="13" label="MobileIdentityConsistency rollback">
+    Signout was forced because MobileIdentityConsistency feature is disabled
+    when there's a primary account without sync consent.
+  </int>
 </enum>
 
 <enum name="SigninSource">