[WebSignin] Keep observers for sign-in error in WebSigninBridge

This CL keeps the observers in WebSigninBridge when there is a sign-in
error.

Bug: 1133704
Change-Id: Ic21b19dca228ee595816558f584519dd00dece94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2445015
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813695}

chrome/browser/android/signin/web_signin_bridge.cc

@@ -18,17 +18,14 @@ using base::android::JavaParamRef;
 void ForwardOnSigninCompletedToJava(
     const base::android::ScopedJavaGlobalRef<jobject>& j_listener,
     const GoogleServiceAuthError& error) {
+  JNIEnv* env = base::android::AttachCurrentThread();
   if (error.state() == GoogleServiceAuthError::State::NONE) {
-    Java_WebSigninBridge_onSigninSucceded(base::android::AttachCurrentThread(),
-                                          j_listener);
-    return;
+    Java_WebSigninBridge_onSigninSucceded(env, j_listener);
+  } else {
+    base::android::ScopedJavaLocalRef<jobject> j_error =
+        signin::Java_GoogleServiceAuthError_Constructor(env, error.state());
+    Java_WebSigninBridge_onSigninFailed(env, j_listener, j_error);
   }
-
-  JNIEnv* env = base::android::AttachCurrentThread();
-  base::android::ScopedJavaLocalRef<jobject> j_error =
-      signin::Java_GoogleServiceAuthError_Constructor(env, error.state());
-  Java_WebSigninBridge_onSigninFailed(base::android::AttachCurrentThread(),
-                                      j_listener, j_error);
 }
 
 WebSigninBridge::WebSigninBridge(signin::IdentityManager* identity_manager,
@@ -51,10 +48,8 @@ WebSigninBridge::WebSigninBridge(signin::IdentityManager* identity_manager,
 }
 
 WebSigninBridge::~WebSigninBridge() {
-  if (on_signin_completed_) {
-    identity_manager_->RemoveObserver(this);
-    account_reconcilor_->RemoveObserver(this);
-  }
+  identity_manager_->RemoveObserver(this);
+  account_reconcilor_->RemoveObserver(this);
 }
 
 void WebSigninBridge::OnAccountsInCookieUpdated(
@@ -84,10 +79,7 @@ void WebSigninBridge::OnStateChanged(
 }
 
 void WebSigninBridge::OnSigninCompleted(const GoogleServiceAuthError& error) {
-  std::move(on_signin_completed_).Run(error);
-  // Remove observers to make sure the callback is invoked only once.
-  identity_manager_->RemoveObserver(this);
-  account_reconcilor_->RemoveObserver(this);
+  on_signin_completed_.Run(error);
 }
 
 static jlong JNI_WebSigninBridge_Create(
@@ -104,9 +96,10 @@ static jlong JNI_WebSigninBridge_Create(
       AccountReconcilorFactory::GetForProfile(profile);
   CoreAccountInfo signin_account =
       ConvertFromJavaCoreAccountInfo(env, j_account);
-  base::OnceCallback<void(const GoogleServiceAuthError&)> on_signin_completed =
-      base::BindOnce(&ForwardOnSigninCompletedToJava,
-                     base::android::ScopedJavaGlobalRef<jobject>(j_listener));
+  base::RepeatingCallback<void(const GoogleServiceAuthError&)>
+      on_signin_completed = base::BindRepeating(
+          &ForwardOnSigninCompletedToJava,
+          base::android::ScopedJavaGlobalRef<jobject>(j_listener));
   return reinterpret_cast<intptr_t>(
       new WebSigninBridge(identity_manager, account_reconcilor, signin_account,
                           std::move(on_signin_completed)));

chrome/browser/android/signin/web_signin_bridge.h

@@ -19,7 +19,7 @@ class WebSigninBridge : public signin::IdentityManager::Observer,
                         public AccountReconcilor::Observer {
  public:
   using OnSigninCompletedCallback =
-      base::OnceCallback<void(const GoogleServiceAuthError&)>;
+      base::RepeatingCallback<void(const GoogleServiceAuthError&)>;
 
   explicit WebSigninBridge(signin::IdentityManager* identity_manager,
                            AccountReconcilor* account_reconcilor,

chrome/browser/android/signin/web_signin_bridge_unittest.cc

@@ -75,6 +75,34 @@ TEST_F(WebSigninBridgeTest,
   identity_test_env_.SetCookieAccounts({cookie_params});
 }
 
+TEST_F(
+    WebSigninBridgeTest,
+    CookiesWithSigninAccountShouldTriggerOnSigninSucceededAfterSigninFailed) {
+  AccountInfo account =
+      identity_test_env_.MakeAccountAvailable("test@gmail.com");
+  base::MockCallback<WebSigninBridge::OnSigninCompletedCallback> callback;
+  std::unique_ptr<WebSigninBridge> web_signin_bridge =
+      CreateWebSigninBridge(account, callback.Get());
+
+  EXPECT_CALL(callback,
+              Run(GoogleServiceAuthError(
+                  GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS)));
+  identity_test_env_.SetPrimaryAccount(account.email);
+  identity_test_env_.SetInvalidRefreshTokenForAccount(account.account_id);
+  identity_test_env_.UpdatePersistentErrorOfRefreshTokenForAccount(
+      account.account_id,
+      GoogleServiceAuthError(
+          GoogleServiceAuthError::State::INVALID_GAIA_CREDENTIALS));
+  account_reconcilor_->EnableReconcile();
+  EXPECT_EQ(signin_metrics::AccountReconcilorState::ACCOUNT_RECONCILOR_ERROR,
+            account_reconcilor_->GetState());
+
+  EXPECT_CALL(callback, Run(GoogleServiceAuthError()));
+  identity_test_env_.SetRefreshTokenForAccount(account.account_id);
+  signin::CookieParamsForTest cookie_params{account.email, account.gaia};
+  identity_test_env_.SetCookieAccounts({cookie_params});
+}
+
 TEST_F(WebSigninBridgeTest,
        CookiesWithoutSigninAccountDontTriggerOnSigninSucceeded) {
   AccountInfo signin_account =