Convert chrome.identity.getAccounts() impl away from AccountTracker

As a precursor to changing the implementation of
chrome.identity.getAccounts() to use the Identity Service, this CL
moves that implementation away from using AccountTracker::GetAccounts()
to using OAuth2TokenService::GetAccounts(). The latter is the function
that we intend to back the upcoming Identity Service API.

AccountTracker::GetAccounts() is fundamentally based on
OAuth2TokenService::GetAccounts(): they both have the basic behavior
of returning the informations of all accounts for which a refresh
token is available. However, AccountTracker::GetAccounts() layers
some extra information on top relating to the primary account (a
concept of which the OAuth2TokenService is not aware). (AccountTracker
itself gets this information via IdentityProvider; in the context of
the chrome.identity.getAccounts() impl, this is ProfileIdentityProvider,
which is backed by SigninManager).

To avoid any behavioral changes, this CL ports the particular semantics
of AccountTracker::GetAccounts() to live directly in the implementation
of chrome.identity.getAccounts():
- Returns an empty list if there is no primary account or there is no
  refresh token available for the primary account
- Otherwise, puts the primary account first in the returned list

Bug: 729536
Change-Id: Ib4f6cff0afd2f751ec3a7409d24b797b1be0ec79
Reviewed-on: https://chromium-review.googlesource.com/574709Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488177}

chrome/browser/extensions/api/identity/identity_api.cc

@@ -156,23 +156,6 @@ const IdentityAPI::CachedTokens& IdentityAPI::GetAllCachedTokens() {
   return token_cache_;
 }
 
-std::vector<std::string> IdentityAPI::GetAccounts() const {
-  const std::vector<gaia::AccountIds> ids = account_tracker_.GetAccounts();
-  std::vector<std::string> gaia_ids;
-
-  if (switches::IsExtensionsMultiAccount()) {
-    for (std::vector<gaia::AccountIds>::const_iterator it = ids.begin();
-         it != ids.end();
-         ++it) {
-      gaia_ids.push_back(it->gaia);
-    }
-  } else if (ids.size() >= 1) {
-    gaia_ids.push_back(ids[0].gaia);
-  }
-
-  return gaia_ids;
-}
-
 void IdentityAPI::Shutdown() {
   if (get_auth_token_function_)
     get_auth_token_function_->Shutdown();
@@ -204,11 +187,6 @@ void IdentityAPI::OnAccountSignInChanged(const gaia::AccountIds& ids,
   EventRouter::Get(browser_context_)->BroadcastEvent(std::move(event));
 }
 
-void IdentityAPI::SetAccountStateForTest(gaia::AccountIds ids,
-                                         bool is_signed_in) {
-  account_tracker_.SetAccountStateForTest(ids, is_signed_in);
-}
-
 template <>
 void BrowserContextKeyedAPIFactory<IdentityAPI>::DeclareFactoryDependencies() {
   DependsOn(ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());

chrome/browser/extensions/api/identity/identity_api.h

@@ -92,9 +92,6 @@ class IdentityAPI : public BrowserContextKeyedAPI,
 
   const CachedTokens& GetAllCachedTokens();
 
-  // Account queries.
-  std::vector<std::string> GetAccounts() const;
-
   // BrowserContextKeyedAPI implementation.
   void Shutdown() override;
   static BrowserContextKeyedAPIFactory<IdentityAPI>* GetFactoryInstance();
@@ -103,8 +100,6 @@ class IdentityAPI : public BrowserContextKeyedAPI,
   void OnAccountSignInChanged(const gaia::AccountIds& ids,
                               bool is_signed_in) override;
 
-  void SetAccountStateForTest(gaia::AccountIds ids, bool is_signed_in);
-
   void set_get_auth_token_function(
       IdentityGetAuthTokenFunction* get_auth_token_function) {
     get_auth_token_function_ = get_auth_token_function;

chrome/browser/extensions/api/identity/identity_apitest.cc

@@ -391,18 +391,97 @@ gaia::AccountIds CreateIds(const std::string& email, const std::string& obfid) {
   return ids;
 }
 
-class IdentityGetAccountsFunctionTest : public ExtensionBrowserTest {
+class IdentityTestWithSignin : public AsyncExtensionBrowserTest {
+ public:
+  void SetUpInProcessBrowserTestFixture() override {
+    AsyncExtensionBrowserTest::SetUpInProcessBrowserTestFixture();
+
+    will_create_browser_context_services_subscription_ =
+        BrowserContextDependencyManager::GetInstance()
+            ->RegisterWillCreateBrowserContextServicesCallbackForTesting(
+                base::Bind(
+                    &IdentityTestWithSignin::OnWillCreateBrowserContextServices,
+                    base::Unretained(this)));
+  }
+
+  void OnWillCreateBrowserContextServices(content::BrowserContext* context) {
+    // Replace the signin manager and token service with fakes. Do this ahead of
+    // creating the browser so that a bunch of classes don't register as
+    // observers and end up needing to unregister when the fake is substituted.
+    SigninManagerFactory::GetInstance()->SetTestingFactory(
+        context, &BuildFakeSigninManagerBase);
+    ProfileOAuth2TokenServiceFactory::GetInstance()->SetTestingFactory(
+        context, &BuildFakeProfileOAuth2TokenService);
+    GaiaCookieManagerServiceFactory::GetInstance()->SetTestingFactory(
+        context, &BuildFakeGaiaCookieManagerService);
+  }
+
+  void SetUpOnMainThread() override {
+    AsyncExtensionBrowserTest::SetUpOnMainThread();
+
+    // Grab references to the fake signin manager and token service.
+    signin_manager_ = static_cast<FakeSigninManagerForTesting*>(
+        SigninManagerFactory::GetInstance()->GetForProfile(profile()));
+    ASSERT_TRUE(signin_manager_);
+    token_service_ = static_cast<FakeProfileOAuth2TokenService*>(
+        ProfileOAuth2TokenServiceFactory::GetInstance()->GetForProfile(
+            profile()));
+    ASSERT_TRUE(token_service_);
+    GaiaCookieManagerServiceFactory::GetInstance()
+        ->GetForProfile(profile())
+        ->Init();
+
+    // Ensure that the token service is initialized, as the Identity Service
+    // delays responding to requests until this initialization is complete.
+    token_service_->LoadCredentials("dummy_primary_account_id");
+  }
+
+ protected:
+  void SignIn(const std::string& account_key) {
+    SignIn(account_key, account_key);
+  }
+
+  void SignIn(const std::string& email, const std::string& gaia) {
+    AccountTrackerService* account_tracker =
+        AccountTrackerServiceFactory::GetForProfile(profile());
+    std::string account_id = account_tracker->SeedAccountInfo(gaia, email);
+
+#if defined(OS_CHROMEOS)
+    signin_manager_->SetAuthenticatedAccountInfo(gaia, email);
+#else
+    signin_manager_->SignIn(gaia, email, "password");
+#endif
+    token_service_->UpdateCredentials(account_id, "refresh_token");
+  }
+
+  void AddAccount(const std::string& email, const std::string& gaia) {
+    AccountTrackerService* account_tracker =
+        AccountTrackerServiceFactory::GetForProfile(profile());
+    std::string account_id = account_tracker->SeedAccountInfo(gaia, email);
+    token_service_->UpdateCredentials(account_id, "refresh_token");
+  }
+
+  void SeedAccountInfo(const std::string& account_key) {
+    AccountTrackerService* account_tracker =
+        AccountTrackerServiceFactory::GetForProfile(profile());
+    account_tracker->SeedAccountInfo(account_key, account_key);
+  }
+
+  FakeSigninManagerForTesting* signin_manager_;
+  FakeProfileOAuth2TokenService* token_service_;
+
+  std::unique_ptr<
+      base::CallbackList<void(content::BrowserContext*)>::Subscription>
+      will_create_browser_context_services_subscription_;
+};
+
+class IdentityGetAccountsFunctionTest : public IdentityTestWithSignin {
   void SetUpCommandLine(base::CommandLine* command_line) override {
     ExtensionBrowserTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kExtensionsMultiAccount);
   }
 
  protected:
-  void SetAccountState(gaia::AccountIds ids, bool is_signed_in) {
-    IdentityAPI::GetFactoryInstance()->Get(profile())->SetAccountStateForTest(
-        ids, is_signed_in);
-  }
-
   testing::AssertionResult ExpectGetAccounts(
       const std::vector<std::string>& accounts) {
     scoped_refptr<IdentityGetAccountsFunction> func(
@@ -485,15 +564,15 @@ IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, NoneSignedIn) {
 
 IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest,
                        PrimaryAccountSignedIn) {
-  SetAccountState(CreateIds("primary@example.com", "1"), true);
+  SignIn("primary@example.com", "1");
   std::vector<std::string> primary;
   primary.push_back("1");
   EXPECT_TRUE(ExpectGetAccounts(primary));
 }
 
 IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, TwoAccountsSignedIn) {
-  SetAccountState(CreateIds("primary@example.com", "1"), true);
-  SetAccountState(CreateIds("secondary@example.com", "2"), true);
+  SignIn("primary@example.com", "1");
+  AddAccount("secondary@example.com", "2");
   std::vector<std::string> two_accounts;
   two_accounts.push_back("1");
   two_accounts.push_back("2");
@@ -503,6 +582,7 @@ IN_PROC_BROWSER_TEST_F(IdentityGetAccountsFunctionTest, TwoAccountsSignedIn) {
 class IdentityOldProfilesGetAccountsFunctionTest
     : public IdentityGetAccountsFunctionTest {
   void SetUpCommandLine(base::CommandLine* command_line) override {
+    ExtensionBrowserTest::SetUpCommandLine(command_line);
     // Don't add the multi-account switch that parent class would have.
   }
 };
@@ -514,84 +594,13 @@ IN_PROC_BROWSER_TEST_F(IdentityOldProfilesGetAccountsFunctionTest,
 
 IN_PROC_BROWSER_TEST_F(IdentityOldProfilesGetAccountsFunctionTest,
                        TwoAccountsSignedIn) {
-  SetAccountState(CreateIds("primary@example.com", "1"), true);
-  SetAccountState(CreateIds("secondary@example.com", "2"), true);
+  SignIn("primary@example.com", "1");
+  AddAccount("secondary@example.com", "2");
   std::vector<std::string> only_primary;
   only_primary.push_back("1");
   EXPECT_TRUE(ExpectGetAccounts(only_primary));
 }
 
-class IdentityTestWithSignin : public AsyncExtensionBrowserTest {
- public:
-  void SetUpInProcessBrowserTestFixture() override {
-    AsyncExtensionBrowserTest::SetUpInProcessBrowserTestFixture();
-
-    will_create_browser_context_services_subscription_ =
-        BrowserContextDependencyManager::GetInstance()
-            ->RegisterWillCreateBrowserContextServicesCallbackForTesting(
-                base::Bind(
-                    &IdentityTestWithSignin::OnWillCreateBrowserContextServices,
-                    base::Unretained(this)));
-  }
-
-  void OnWillCreateBrowserContextServices(content::BrowserContext* context) {
-    // Replace the signin manager and token service with fakes. Do this ahead of
-    // creating the browser so that a bunch of classes don't register as
-    // observers and end up needing to unregister when the fake is substituted.
-    SigninManagerFactory::GetInstance()->SetTestingFactory(
-        context, &BuildFakeSigninManagerBase);
-    ProfileOAuth2TokenServiceFactory::GetInstance()->SetTestingFactory(
-        context, &BuildFakeProfileOAuth2TokenService);
-    GaiaCookieManagerServiceFactory::GetInstance()->SetTestingFactory(
-        context, &BuildFakeGaiaCookieManagerService);
-  }
-
-  void SetUpOnMainThread() override {
-    AsyncExtensionBrowserTest::SetUpOnMainThread();
-
-    // Grab references to the fake signin manager and token service.
-    signin_manager_ = static_cast<FakeSigninManagerForTesting*>(
-        SigninManagerFactory::GetInstance()->GetForProfile(profile()));
-    ASSERT_TRUE(signin_manager_);
-    token_service_ = static_cast<FakeProfileOAuth2TokenService*>(
-        ProfileOAuth2TokenServiceFactory::GetInstance()->GetForProfile(
-            profile()));
-    ASSERT_TRUE(token_service_);
-    GaiaCookieManagerServiceFactory::GetInstance()->GetForProfile(profile())
-        ->Init();
-
-    // Ensure that the token service is initialized, as the Identity Service
-    // delays responding to requests until this initialization is complete.
-    token_service_->LoadCredentials("dummy_primary_account_id");
-  }
-
- protected:
-  void SignIn(const std::string& account_key) {
-    SignIn(account_key, account_key);
-  }
-
-  void SignIn(const std::string& email, const std::string& gaia) {
-    AccountTrackerService* account_tracker =
-        AccountTrackerServiceFactory::GetForProfile(profile());
-    std::string account_id =
-        account_tracker->SeedAccountInfo(gaia, email);
-
-#if defined(OS_CHROMEOS)
-    signin_manager_->SetAuthenticatedAccountInfo(gaia, email);
-#else
-    signin_manager_->SignIn(gaia, email, "password");
-#endif
-    token_service_->UpdateCredentials(account_id, "refresh_token");
-  }
-
-  FakeSigninManagerForTesting* signin_manager_;
-  FakeProfileOAuth2TokenService* token_service_;
-
-  std::unique_ptr<
-      base::CallbackList<void(content::BrowserContext*)>::Subscription>
-      will_create_browser_context_services_subscription_;
-};
-
 class IdentityGetProfileUserInfoFunctionTest : public IdentityTestWithSignin {
  protected:
   std::unique_ptr<api::identity::ProfileUserInfo> RunGetProfileUserInfo() {
@@ -675,12 +684,6 @@ class GetAuthTokenFunctionTest
         base::Time::Now() + base::TimeDelta::FromSeconds(3600));
   }
 
-  void SeedAccountInfo(const std::string& account_key) {
-    AccountTrackerService* account_tracker =
-        AccountTrackerServiceFactory::GetForProfile(profile());
-    account_tracker->SeedAccountInfo(account_key, account_key);
-  }
-
  protected:
   enum OAuth2Fields {
     NONE = 0,

chrome/browser/extensions/api/identity/identity_get_accounts_function.cc

@@ -7,7 +7,13 @@
 #include "chrome/browser/extensions/api/identity/identity_api.h"
 #include "chrome/browser/extensions/api/identity/identity_constants.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/account_tracker_service_factory.h"
+#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "chrome/browser/signin/signin_manager_factory.h"
 #include "chrome/common/extensions/api/identity.h"
+#include "components/signin/core/browser/account_tracker_service.h"
+#include "components/signin/core/browser/profile_oauth2_token_service.h"
+#include "components/signin/core/browser/signin_manager.h"
 #include "components/signin/core/common/profile_management_switches.h"
 
 namespace extensions {
@@ -23,17 +29,44 @@ ExtensionFunction::ResponseAction IdentityGetAccountsFunction::Run() {
     return RespondNow(Error(identity_constants::kOffTheRecord));
   }
 
-  std::vector<std::string> gaia_ids =
-      IdentityAPI::GetFactoryInstance()->Get(GetProfile())->GetAccounts();
-  DCHECK(gaia_ids.size() < 2 || switches::IsExtensionsMultiAccount());
+  ProfileOAuth2TokenService* token_service =
+      ProfileOAuth2TokenServiceFactory::GetForProfile(GetProfile());
+  AccountTrackerService* account_tracker =
+      AccountTrackerServiceFactory::GetForProfile(GetProfile());
 
   std::unique_ptr<base::ListValue> infos(new base::ListValue());
+  std::string primary_account =
+      SigninManagerFactory::GetForProfile(GetProfile())
+          ->GetAuthenticatedAccountId();
+
+  // If there is no primary account, short-circuit out.
+  if (primary_account.empty() ||
+      !token_service->RefreshTokenIsAvailable(primary_account))
+    return RespondNow(OneArgument(std::move(infos)));
+
+  // Set the primary account as the first entry.
+  std::string primary_gaia_id =
+      account_tracker->GetAccountInfo(primary_account).gaia;
+  api::identity::AccountInfo primary_account_info;
+  primary_account_info.id = primary_gaia_id;
+  infos->Append(primary_account_info.ToValue());
+
+  // If extensions are not multi-account, ignore any other accounts.
+  if (!switches::IsExtensionsMultiAccount())
+    return RespondNow(OneArgument(std::move(infos)));
+
+  // Otherwise, add the other accounts.
+  std::vector<std::string> accounts = token_service->GetAccounts();
+  for (std::vector<std::string>::const_iterator it = accounts.begin();
+       it != accounts.end(); ++it) {
+    std::string gaia_id = account_tracker->GetAccountInfo(*it).gaia;
+    if (gaia_id.empty())
+      continue;
+    if (gaia_id == primary_gaia_id)
+      continue;
 
-  for (std::vector<std::string>::const_iterator it = gaia_ids.begin();
-       it != gaia_ids.end();
-       ++it) {
     api::identity::AccountInfo account_info;
-    account_info.id = *it;
+    account_info.id = gaia_id;
     infos->Append(account_info.ToValue());
   }