Do not push Primary Account to ARC just after provisioning

After ARC has just been successfully provisioned, there is no need to
push the Primary Account to ARC again, because it will trigger an
account check otherwise, and may cause performance issues. Please check
the attached bug for details.

Bug: 1001518
Test: browser_tests --gtest_filter="*ArcAuthService*Test*"
Change-Id: I694f1c56bd27ddfb716db2f7330a2b943a4ad71e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1796344Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695340}

chrome/browser/chromeos/arc/auth/arc_auth_service.cc

@@ -282,7 +282,7 @@ void ArcAuthService::OnConnectionReady() {
   // |ArcSessionManager::Get()->IsArcProvisioned()| will be |true|.
   if (arc::IsArcProvisioned(profile_)) {
     TriggerAccountManagerMigrationsIfRequired(profile_);
-    TriggerAccountsPushToArc();
+    TriggerAccountsPushToArc(false /* filter_primary_account */);
   }
 
   if (pending_get_arc_accounts_callback_)
@@ -585,7 +585,7 @@ void ArcAuthService::OnExtendedAccountInfoRemoved(
 }
 
 void ArcAuthService::OnArcInitialStart() {
-  TriggerAccountsPushToArc();
+  TriggerAccountsPushToArc(true /* filter_primary_account */);
 }
 
 void ArcAuthService::Shutdown() {
@@ -774,14 +774,18 @@ void ArcAuthService::SkipMergeSessionForTesting() {
   skip_merge_session_for_testing_ = true;
 }
 
-void ArcAuthService::TriggerAccountsPushToArc() {
+void ArcAuthService::TriggerAccountsPushToArc(bool filter_primary_account) {
   if (!chromeos::IsAccountManagerAvailable(profile_))
     return;
 
   const std::vector<CoreAccountInfo> accounts =
       identity_manager_->GetAccountsWithRefreshTokens();
-  for (const CoreAccountInfo& account : accounts)
+  for (const CoreAccountInfo& account : accounts) {
+    if (filter_primary_account && IsPrimaryGaiaAccount(account.gaia))
+      continue;
+
     OnRefreshTokenUpdatedForAccount(account);
+  }
 }
 
 void ArcAuthService::DispatchAccountsInArc(

chrome/browser/chromeos/arc/auth/arc_auth_service.h

@@ -182,7 +182,9 @@ class ArcAuthService : public KeyedService,
   void DeletePendingTokenRequest(ArcFetcherBase* fetcher);
 
   // Triggers an async push of the accounts in IdentityManager to ARC.
-  void TriggerAccountsPushToArc();
+  // If |filter_primary_account| is set to |true|, the Primary Account in Chrome
+  // OS Account Manager will not be pushed to ARC as part of this call.
+  void TriggerAccountsPushToArc(bool filter_primary_account);
 
   // Issues a request to ARC, which will complete callback with the list of
   // Google accounts in ARC.

chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

@@ -74,7 +74,6 @@
 namespace {
 
 constexpr char kFakeUserName[] = "test@example.com";
-constexpr char kFakeGaiaId[] = "1234567890";
 constexpr char kSecondaryAccountEmail[] = "email.111@gmail.com";
 constexpr char kFakeAuthCode[] = "fake-auth-code";
 
@@ -147,8 +146,8 @@ class FakeAuthInstance : public mojom::AuthInstance {
 
   void GetGoogleAccounts(GetGoogleAccountsCallback callback) override {
     std::vector<mojom::ArcAccountInfoPtr> accounts;
-    accounts.emplace_back(
-        mojom::ArcAccountInfo::New(kFakeUserName, kFakeGaiaId));
+    accounts.emplace_back(mojom::ArcAccountInfo::New(
+        kFakeUserName, signin::GetTestGaiaIdForEmail(kFakeUserName)));
     std::move(callback).Run(std::move(accounts));
   }
 
@@ -222,8 +221,8 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
     // a dangling pointer to the User.
     // TODO(nya): Consider removing all users from ProfileHelper in the
     // destructor of FakeChromeUserManager.
-    const AccountId account_id(
-        AccountId::FromUserEmailGaiaId(kFakeUserName, kFakeGaiaId));
+    const AccountId account_id(AccountId::FromUserEmailGaiaId(
+        kFakeUserName, signin::GetTestGaiaIdForEmail(kFakeUserName)));
     GetFakeUserManager()->RemoveUserFromList(account_id);
     // Since ArcServiceLauncher is (re-)set up with profile() in
     // SetUpOnMainThread() it is necessary to Shutdown() before the profile()
@@ -249,8 +248,8 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
   }
 
   void SetAccountAndProfile(const user_manager::UserType user_type) {
-    const AccountId account_id(
-        AccountId::FromUserEmailGaiaId(kFakeUserName, kFakeGaiaId));
+    const AccountId account_id(AccountId::FromUserEmailGaiaId(
+        kFakeUserName, signin::GetTestGaiaIdForEmail(kFakeUserName)));
     const user_manager::User* user = nullptr;
     switch (user_type) {
       case user_manager::USER_TYPE_CHILD:
@@ -545,7 +544,8 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchGoogleAccountsFromArc) {
   EXPECT_TRUE(arc_google_accounts_callback_called());
   ASSERT_EQ(1UL, arc_google_accounts().size());
   EXPECT_EQ(kFakeUserName, arc_google_accounts()[0]->email);
-  EXPECT_EQ(kFakeGaiaId, arc_google_accounts()[0]->gaia_id);
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kFakeUserName),
+            arc_google_accounts()[0]->gaia_id);
 }
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
@@ -567,7 +567,8 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
   EXPECT_TRUE(arc_google_accounts_callback_called());
   ASSERT_EQ(1UL, arc_google_accounts().size());
   EXPECT_EQ(kFakeUserName, arc_google_accounts()[0]->email);
-  EXPECT_EQ(kFakeGaiaId, arc_google_accounts()[0]->gaia_id);
+  EXPECT_EQ(signin::GetTestGaiaIdForEmail(kFakeUserName),
+            arc_google_accounts()[0]->gaia_id);
 }
 
 // Tests that need Chrome OS Account Manager feature to be enabled.
@@ -595,6 +596,21 @@ class ArcAuthServiceAccountManagerTest : public ArcAuthServiceTest {
   DISALLOW_COPY_AND_ASSIGN(ArcAuthServiceAccountManagerTest);
 };
 
+IN_PROC_BROWSER_TEST_F(
+    ArcAuthServiceAccountManagerTest,
+    PrimaryAccountReauthIsNotAttemptedJustAfterProvisioning) {
+  SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
+  const int initial_num_calls = auth_instance().num_account_upserted_calls();
+  // Our test setup manually sets the device as provisioned and invokes
+  // |ArcAuthService::OnConnectionReady|. Hence, we would have received an
+  // update for the Primary Account.
+  EXPECT_EQ(1, initial_num_calls);
+
+  // Simulate ARC first time provisioning call.
+  auth_service().OnArcInitialStart();
+  EXPECT_EQ(initial_num_calls, auth_instance().num_account_upserted_calls());
+}
+
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceAccountManagerTest,
                        UnAuthenticatedAccountsAreNotPropagated) {
   const AccountInfo account_info = SetupGaiaAccount(kSecondaryAccountEmail);