0

Use a dedicated struct to initialize IdentityManager

To make the IdentityManager's constructor easily extendable for
different platforms, use a dedicated struct -
|IdentityManager::InitParameters|.
It helps in passing platform specific fields to IdentityManager,
like |chromeos::AccountManager|.

Bug: 1068240
Change-Id: I48eb10cb6f32c447692a9af045cfcedd66b4052d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212050
Commit-Queue: Anastasiia N <anastasiian@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Kush Sinha <sinhak@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774585}
This commit is contained in:
Andrei Salavei
2020-06-03 12:46:26 +00:00
committed by Commit Bot
parent 3da02ff595
commit fc9539ff66
8 changed files with 288 additions and 87 deletions

@ -95,6 +95,7 @@ source_set("unit_tests") {
"accounts_cookie_mutator_unittest.cc",
"accounts_mutator_unittest.cc",
"diagnostics_provider_unittest.cc",
"identity_manager_builder_unittest.cc",
"identity_manager_unittest.cc",
"identity_test_environment_unittest.cc",
"identity_utils_unittest.cc",
@ -121,6 +122,10 @@ source_set("unit_tests") {
"//testing/gmock",
"//testing/gtest",
]
if (is_ios) {
deps += [ "//components/signin/public/identity_manager/ios:test_support" ]
}
}
source_set("test_support") {

@ -33,27 +33,24 @@
namespace signin {
IdentityManager::IdentityManager(
std::unique_ptr<AccountTrackerService> account_tracker_service,
std::unique_ptr<ProfileOAuth2TokenService> token_service,
std::unique_ptr<GaiaCookieManagerService> gaia_cookie_manager_service,
std::unique_ptr<PrimaryAccountManager> primary_account_manager,
std::unique_ptr<AccountFetcherService> account_fetcher_service,
std::unique_ptr<PrimaryAccountMutator> primary_account_mutator,
std::unique_ptr<AccountsMutator> accounts_mutator,
std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator,
std::unique_ptr<DiagnosticsProvider> diagnostics_provider,
std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer)
: account_tracker_service_(std::move(account_tracker_service)),
token_service_(std::move(token_service)),
gaia_cookie_manager_service_(std::move(gaia_cookie_manager_service)),
primary_account_manager_(std::move(primary_account_manager)),
account_fetcher_service_(std::move(account_fetcher_service)),
identity_mutator_(std::move(primary_account_mutator),
std::move(accounts_mutator),
std::move(accounts_cookie_mutator),
std::move(device_accounts_synchronizer)),
diagnostics_provider_(std::move(diagnostics_provider)) {
IdentityManager::InitParameters::InitParameters() = default;
IdentityManager::InitParameters::InitParameters(InitParameters&&) = default;
IdentityManager::InitParameters::~InitParameters() = default;
IdentityManager::IdentityManager(IdentityManager::InitParameters&& parameters)
: account_tracker_service_(std::move(parameters.account_tracker_service)),
token_service_(std::move(parameters.token_service)),
gaia_cookie_manager_service_(
std::move(parameters.gaia_cookie_manager_service)),
primary_account_manager_(std::move(parameters.primary_account_manager)),
account_fetcher_service_(std::move(parameters.account_fetcher_service)),
identity_mutator_(std::move(parameters.primary_account_mutator),
std::move(parameters.accounts_mutator),
std::move(parameters.accounts_cookie_mutator),
std::move(parameters.device_accounts_synchronizer)),
diagnostics_provider_(std::move(parameters.diagnostics_provider)) {
DCHECK(account_fetcher_service_);
DCHECK(diagnostics_provider_);

@ -364,22 +364,35 @@ class IdentityManager : public KeyedService,
void RemoveDiagnosticsObserver(DiagnosticsObserver* observer);
// **************************************************************************
// NOTE: All public methods methods below are either intended to be used only
// by signin code, or are slated for deletion. Most IdentityManager consumers
// should not need to interact with any methods below this line.
// NOTE: All public methods and structures below are either intended to be
// used only by signin code, or are slated for deletion. Most IdentityManager
// consumers should not need to interact with any methods or structures below
// this line.
// **************************************************************************
IdentityManager(
std::unique_ptr<AccountTrackerService> account_tracker_service,
std::unique_ptr<ProfileOAuth2TokenService> token_service,
std::unique_ptr<GaiaCookieManagerService> gaia_cookie_manager_service,
std::unique_ptr<PrimaryAccountManager> primary_account_manager,
std::unique_ptr<AccountFetcherService> account_fetcher_service,
std::unique_ptr<PrimaryAccountMutator> primary_account_mutator,
std::unique_ptr<AccountsMutator> accounts_mutator,
std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator,
std::unique_ptr<DiagnosticsProvider> diagnostics_provider,
std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer);
// The struct contains all fields required to initialize the
// IdentityManager instance.
struct InitParameters {
std::unique_ptr<ProfileOAuth2TokenService> token_service;
std::unique_ptr<AccountTrackerService> account_tracker_service;
std::unique_ptr<AccountFetcherService> account_fetcher_service;
std::unique_ptr<GaiaCookieManagerService> gaia_cookie_manager_service;
std::unique_ptr<AccountsCookieMutator> accounts_cookie_mutator;
std::unique_ptr<PrimaryAccountManager> primary_account_manager;
std::unique_ptr<PrimaryAccountMutator> primary_account_mutator;
std::unique_ptr<AccountsMutator> accounts_mutator;
std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer;
std::unique_ptr<DiagnosticsProvider> diagnostics_provider;
InitParameters();
InitParameters(InitParameters&&);
~InitParameters();
InitParameters(const InitParameters&) = delete;
InitParameters& operator=(const InitParameters&) = delete;
};
explicit IdentityManager(IdentityManager::InitParameters&& parameters);
~IdentityManager() override;
// Performs initialization that is dependent on the network being
@ -536,6 +549,7 @@ class IdentityManager : public KeyedService,
// order to drive its behavior.
// TODO(https://crbug.com/943135): Find a better way to accomplish this.
friend IdentityManagerTest;
FRIEND_TEST_ALL_PREFIXES(IdentityManagerTest, Construct);
FRIEND_TEST_ALL_PREFIXES(IdentityManagerTest,
PrimaryAccountInfoAfterSigninAndAccountRemoval);
FRIEND_TEST_ALL_PREFIXES(IdentityManagerTest,

@ -23,7 +23,6 @@
#include "components/signin/public/base/signin_client.h"
#include "components/signin/public/identity_manager/accounts_mutator.h"
#include "components/signin/public/identity_manager/device_accounts_synchronizer.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#if !defined(OS_ANDROID)
#include "components/signin/public/webdata/token_web_data.h"
@ -106,7 +105,7 @@ IdentityManagerBuildParams::IdentityManagerBuildParams() = default;
IdentityManagerBuildParams::~IdentityManagerBuildParams() = default;
std::unique_ptr<IdentityManager> BuildIdentityManager(
IdentityManager::InitParameters BuildIdentityManagerInitParameters(
IdentityManagerBuildParams* params) {
std::unique_ptr<AccountTrackerService> account_tracker_service =
BuildAccountTrackerService(params->pref_service, params->profile_path);
@ -138,40 +137,48 @@ std::unique_ptr<IdentityManager> BuildIdentityManager(
account_tracker_service.get(),
token_service.get(), params->local_state);
auto primary_account_mutator = std::make_unique<PrimaryAccountMutatorImpl>(
account_tracker_service.get(), primary_account_manager.get(),
params->pref_service);
IdentityManager::InitParameters init_params;
std::unique_ptr<AccountsMutator> accounts_mutator =
init_params.primary_account_mutator =
std::make_unique<PrimaryAccountMutatorImpl>(account_tracker_service.get(),
primary_account_manager.get(),
params->pref_service);
init_params.accounts_mutator =
BuildAccountsMutator(params->pref_service, account_tracker_service.get(),
token_service.get(), primary_account_manager.get());
auto accounts_cookie_mutator = std::make_unique<AccountsCookieMutatorImpl>(
params->signin_client, token_service.get(),
gaia_cookie_manager_service.get(), account_tracker_service.get());
init_params.accounts_cookie_mutator =
std::make_unique<AccountsCookieMutatorImpl>(
params->signin_client, token_service.get(),
gaia_cookie_manager_service.get(), account_tracker_service.get());
auto diagnostics_provider = std::make_unique<DiagnosticsProviderImpl>(
init_params.diagnostics_provider = std::make_unique<DiagnosticsProviderImpl>(
token_service.get(), gaia_cookie_manager_service.get());
std::unique_ptr<AccountFetcherService> account_fetcher_service =
BuildAccountFetcherService(params->signin_client, token_service.get(),
account_tracker_service.get(),
std::move(params->image_decoder));
init_params.account_fetcher_service = BuildAccountFetcherService(
params->signin_client, token_service.get(), account_tracker_service.get(),
std::move(params->image_decoder));
std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer;
#if defined(OS_IOS) || defined(OS_ANDROID)
device_accounts_synchronizer =
init_params.device_accounts_synchronizer =
std::make_unique<DeviceAccountsSynchronizerImpl>(
token_service->GetDelegate());
#endif
init_params.account_tracker_service = std::move(account_tracker_service);
init_params.gaia_cookie_manager_service =
std::move(gaia_cookie_manager_service);
init_params.primary_account_manager = std::move(primary_account_manager);
init_params.token_service = std::move(token_service);
return init_params;
}
std::unique_ptr<IdentityManager> BuildIdentityManager(
IdentityManagerBuildParams* params) {
return std::make_unique<IdentityManager>(
std::move(account_tracker_service), std::move(token_service),
std::move(gaia_cookie_manager_service),
std::move(primary_account_manager), std::move(account_fetcher_service),
std::move(primary_account_mutator), std::move(accounts_mutator),
std::move(accounts_cookie_mutator), std::move(diagnostics_provider),
std::move(device_accounts_synchronizer));
BuildIdentityManagerInitParameters(params));
}
} // namespace signin

@ -9,6 +9,7 @@
#include "base/files/file_path.h"
#include "build/build_config.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#if !defined(OS_ANDROID)
#include "base/memory/scoped_refptr.h"
@ -47,7 +48,6 @@ class AccountManager;
namespace signin {
enum class AccountConsistencyMethod;
class IdentityManager;
struct IdentityManagerBuildParams {
IdentityManagerBuildParams();
@ -82,6 +82,10 @@ struct IdentityManagerBuildParams {
#endif
};
// Builds all required dependencies to initialize the IdentityManager instance.
IdentityManager::InitParameters BuildIdentityManagerInitParameters(
IdentityManagerBuildParams* params);
// Builds an IdentityManager instance from the supplied embedder-level
// dependencies.
std::unique_ptr<IdentityManager> BuildIdentityManager(

@ -0,0 +1,131 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "components/signin/public/identity_manager/identity_manager_builder.h"
#include "base/files/scoped_temp_dir.h"
#include "base/test/task_environment.h"
#include "build/build_config.h"
#include "components/image_fetcher/core/fake_image_decoder.h"
#include "components/signin/internal/identity_manager/account_fetcher_service.h"
#include "components/signin/public/base/test_signin_client.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "services/network/test/test_network_connection_tracker.h"
#include "services/network/test/test_url_loader_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_ANDROID)
#include "components/signin/public/identity_manager/identity_test_utils.h"
#endif
#if defined(OS_CHROMEOS)
#include "chromeos/components/account_manager/account_manager.h"
#include "chromeos/components/account_manager/account_manager_factory.h"
#endif
#if defined(OS_IOS)
#include "components/signin/public/identity_manager/ios/fake_device_accounts_provider.h"
#endif
namespace signin {
class IdentityManagerBuilderTest : public testing::Test {
protected:
IdentityManagerBuilderTest()
: signin_client_(&pref_service_, &test_url_loader_factory_) {
IdentityManager::RegisterProfilePrefs(pref_service_.registry());
}
~IdentityManagerBuilderTest() override { signin_client_.Shutdown(); }
TestSigninClient* GetSigninClient() { return &signin_client_; }
sync_preferences::TestingPrefServiceSyncable* GetPrefService() {
return &pref_service_;
}
#if defined(OS_CHROMEOS)
chromeos::AccountManagerFactory* GetAccountManagerFactory() {
return &account_manager_factory_;
}
#endif
public:
IdentityManagerBuilderTest(const IdentityManagerBuilderTest&) = delete;
IdentityManagerBuilderTest& operator=(const IdentityManagerBuilderTest&) =
delete;
private:
base::test::TaskEnvironment task_environment_;
sync_preferences::TestingPrefServiceSyncable pref_service_;
network::TestURLLoaderFactory test_url_loader_factory_;
TestSigninClient signin_client_;
#if defined(OS_CHROMEOS)
chromeos::AccountManagerFactory account_manager_factory_;
#endif
};
// Test that IdentityManagerBuilder properly set all required parameters to the
// IdentityManager::InitParameters struct
TEST_F(IdentityManagerBuilderTest, BuildIdentityManagerInitParameters) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
base::FilePath dest_path = temp_dir.GetPath();
#if defined(OS_ANDROID)
DisableInteractionWithSystemAccounts();
#endif
IdentityManagerBuildParams params;
params.account_consistency = AccountConsistencyMethod::kDisabled;
params.image_decoder = std::make_unique<image_fetcher::FakeImageDecoder>();
params.local_state = GetPrefService();
params.network_connection_tracker =
network::TestNetworkConnectionTracker::GetInstance();
params.pref_service = GetPrefService();
params.profile_path = dest_path;
params.signin_client = GetSigninClient();
#if defined(OS_IOS)
params.device_accounts_provider =
std::make_unique<FakeDeviceAccountsProvider>();
#endif
#if defined(OS_CHROMEOS)
chromeos::AccountManager* account_manager =
GetAccountManagerFactory()->GetAccountManager(dest_path.value());
account_manager->Initialize(
dest_path, GetSigninClient()->GetURLLoaderFactory(),
base::BindRepeating(
[](base::OnceClosure closure) -> void { std::move(closure).Run(); }));
params.account_manager = account_manager;
params.is_regular_profile = true;
#endif
const IdentityManager::InitParameters init_params =
signin::BuildIdentityManagerInitParameters(&params);
EXPECT_NE(init_params.account_tracker_service, nullptr);
EXPECT_NE(init_params.token_service, nullptr);
EXPECT_NE(init_params.gaia_cookie_manager_service, nullptr);
EXPECT_NE(init_params.primary_account_manager, nullptr);
EXPECT_NE(init_params.account_fetcher_service, nullptr);
EXPECT_NE(init_params.primary_account_mutator, nullptr);
EXPECT_NE(init_params.accounts_cookie_mutator, nullptr);
EXPECT_NE(init_params.diagnostics_provider, nullptr);
#if defined(OS_IOS) || defined(OS_ANDROID)
EXPECT_NE(init_params.device_accounts_synchronizer, nullptr);
EXPECT_EQ(init_params.accounts_mutator, nullptr);
#else
EXPECT_EQ(init_params.device_accounts_synchronizer, nullptr);
EXPECT_NE(init_params.accounts_mutator, nullptr);
#endif
// Manually shut down AccountFetcherService to avoid DCHECK failure inside its
// destructor.
init_params.account_fetcher_service->Shutdown();
}
} // namespace signin

@ -21,10 +21,13 @@
#include "components/signin/internal/identity_manager/account_fetcher_service.h"
#include "components/signin/internal/identity_manager/account_tracker_service.h"
#include "components/signin/internal/identity_manager/accounts_cookie_mutator_impl.h"
#include "components/signin/internal/identity_manager/accounts_mutator_impl.h"
#include "components/signin/internal/identity_manager/device_accounts_synchronizer_impl.h"
#include "components/signin/internal/identity_manager/diagnostics_provider_impl.h"
#include "components/signin/internal/identity_manager/fake_profile_oauth2_token_service.h"
#include "components/signin/internal/identity_manager/gaia_cookie_manager_service.h"
#include "components/signin/internal/identity_manager/primary_account_manager.h"
#include "components/signin/internal/identity_manager/primary_account_mutator_impl.h"
#include "components/signin/internal/identity_manager/primary_account_policy_manager_impl.h"
#include "components/signin/internal/identity_manager/profile_oauth2_token_service_delegate.h"
#include "components/signin/public/base/account_consistency_method.h"
@ -33,11 +36,9 @@
#include "components/signin/public/base/test_signin_client.h"
#include "components/signin/public/identity_manager/access_token_info.h"
#include "components/signin/public/identity_manager/accounts_cookie_mutator.h"
#include "components/signin/public/identity_manager/accounts_mutator.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/device_accounts_synchronizer.h"
#include "components/signin/public/identity_manager/identity_test_utils.h"
#include "components/signin/public/identity_manager/primary_account_mutator.h"
#include "components/signin/public/identity_manager/scope_set.h"
#include "components/signin/public/identity_manager/set_accounts_in_cookie_result.h"
#include "components/signin/public/identity_manager/test_identity_manager_observer.h"
@ -352,19 +353,40 @@ class IdentityManagerTest : public testing::Test {
primary_account_manager->SignIn(kTestEmail);
}
auto accounts_cookie_mutator = std::make_unique<AccountsCookieMutatorImpl>(
&signin_client_, token_service.get(), gaia_cookie_manager_service.get(),
account_tracker_service.get());
IdentityManager::InitParameters init_params;
auto diagnostics_provider = std::make_unique<DiagnosticsProviderImpl>(
token_service.get(), gaia_cookie_manager_service.get());
init_params.accounts_cookie_mutator =
std::make_unique<AccountsCookieMutatorImpl>(
&signin_client_, token_service.get(),
gaia_cookie_manager_service.get(), account_tracker_service.get());
identity_manager_.reset(new IdentityManager(
std::move(account_tracker_service), std::move(token_service),
std::move(gaia_cookie_manager_service),
std::move(primary_account_manager), std::move(account_fetcher_service),
nullptr, nullptr, std::move(accounts_cookie_mutator),
std::move(diagnostics_provider), nullptr));
init_params.diagnostics_provider =
std::make_unique<DiagnosticsProviderImpl>(
token_service.get(), gaia_cookie_manager_service.get());
init_params.primary_account_mutator =
std::make_unique<PrimaryAccountMutatorImpl>(
account_tracker_service.get(), primary_account_manager.get(),
&pref_service_);
#if defined(OS_ANDROID) || defined(OS_IOS)
init_params.device_accounts_synchronizer =
std::make_unique<DeviceAccountsSynchronizerImpl>(
token_service->GetDelegate());
#else
init_params.accounts_mutator = std::make_unique<AccountsMutatorImpl>(
token_service.get(), account_tracker_service.get(),
primary_account_manager.get(), &pref_service_);
#endif
init_params.account_fetcher_service = std::move(account_fetcher_service);
init_params.account_tracker_service = std::move(account_tracker_service);
init_params.gaia_cookie_manager_service =
std::move(gaia_cookie_manager_service);
init_params.primary_account_manager = std::move(primary_account_manager);
init_params.token_service = std::move(token_service);
identity_manager_.reset(new IdentityManager(std::move(init_params)));
identity_manager_observer_.reset(
new TestIdentityManagerObserver(identity_manager_.get()));
identity_manager_diagnostics_observer_.reset(
@ -419,6 +441,25 @@ class IdentityManagerTest : public testing::Test {
DISALLOW_COPY_AND_ASSIGN(IdentityManagerTest);
};
// Test that IdentityManager's constructor properly sets all passed parameters.
TEST_F(IdentityManagerTest, Construct) {
EXPECT_NE(identity_manager()->GetAccountTrackerService(), nullptr);
EXPECT_NE(identity_manager()->GetTokenService(), nullptr);
EXPECT_NE(identity_manager()->GetGaiaCookieManagerService(), nullptr);
EXPECT_NE(identity_manager()->GetPrimaryAccountManager(), nullptr);
EXPECT_NE(identity_manager()->GetAccountFetcherService(), nullptr);
EXPECT_NE(identity_manager()->GetPrimaryAccountMutator(), nullptr);
EXPECT_NE(identity_manager()->GetAccountsCookieMutator(), nullptr);
EXPECT_NE(identity_manager()->GetDiagnosticsProvider(), nullptr);
#if defined(OS_ANDROID) || defined(OS_IOS)
EXPECT_EQ(identity_manager()->GetAccountsMutator(), nullptr);
EXPECT_NE(identity_manager()->GetDeviceAccountsSynchronizer(), nullptr);
#else
EXPECT_NE(identity_manager()->GetAccountsMutator(), nullptr);
EXPECT_EQ(identity_manager()->GetDeviceAccountsSynchronizer(), nullptr);
#endif
}
// Test that IdentityManager starts off with the information in
// PrimaryAccountManager.
TEST_F(IdentityManagerTest, PrimaryAccountInfoAtStartup) {

@ -198,39 +198,41 @@ IdentityTestEnvironment::BuildIdentityManagerForTests(
std::make_unique<GaiaCookieManagerService>(token_service.get(),
signin_client);
std::unique_ptr<PrimaryAccountMutator> primary_account_mutator =
IdentityManager::InitParameters init_params;
init_params.primary_account_mutator =
std::make_unique<PrimaryAccountMutatorImpl>(account_tracker_service.get(),
primary_account_manager.get(),
pref_service);
std::unique_ptr<AccountsMutator> accounts_mutator;
#if !defined(OS_ANDROID) && !defined(OS_IOS)
accounts_mutator = std::make_unique<AccountsMutatorImpl>(
init_params.accounts_mutator = std::make_unique<AccountsMutatorImpl>(
token_service.get(), account_tracker_service.get(),
primary_account_manager.get(), pref_service);
#endif
auto diagnostics_provider = std::make_unique<DiagnosticsProviderImpl>(
init_params.diagnostics_provider = std::make_unique<DiagnosticsProviderImpl>(
token_service.get(), gaia_cookie_manager_service.get());
auto accounts_cookie_mutator = std::make_unique<AccountsCookieMutatorImpl>(
signin_client, token_service.get(), gaia_cookie_manager_service.get(),
account_tracker_service.get());
init_params.accounts_cookie_mutator =
std::make_unique<AccountsCookieMutatorImpl>(
signin_client, token_service.get(), gaia_cookie_manager_service.get(),
account_tracker_service.get());
std::unique_ptr<DeviceAccountsSynchronizer> device_accounts_synchronizer;
#if defined(OS_IOS)
device_accounts_synchronizer =
init_params.device_accounts_synchronizer =
std::make_unique<DeviceAccountsSynchronizerImpl>(
token_service->GetDelegate());
#endif
return std::make_unique<IdentityManager>(
std::move(account_tracker_service), std::move(token_service),
std::move(gaia_cookie_manager_service),
std::move(primary_account_manager), std::move(account_fetcher_service),
std::move(primary_account_mutator), std::move(accounts_mutator),
std::move(accounts_cookie_mutator), std::move(diagnostics_provider),
std::move(device_accounts_synchronizer));
init_params.account_fetcher_service = std::move(account_fetcher_service);
init_params.account_tracker_service = std::move(account_tracker_service);
init_params.gaia_cookie_manager_service =
std::move(gaia_cookie_manager_service);
init_params.primary_account_manager = std::move(primary_account_manager);
init_params.token_service = std::move(token_service);
return std::make_unique<IdentityManager>(std::move(init_params));
}
IdentityTestEnvironment::~IdentityTestEnvironment() {