Commit 13f3d2ee authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

[s13n] Replace AccountReconcilor inheritance of GCMS::Observer

... in favor the IdentityManager::Observer counterparts.

This is another step forward on removing the dependency of
GaiaCookieManagerService from AccountReconcilor.

In a follow up step, the ctor argument that still takes an
GaiaCookieManagerService instance will be removed.

BUG=926890,926886

Change-Id: Ibd3dab957ad5b23eefe06c5dc3963d7e566c9bcf
Reviewed-on: https://chromium-review.googlesource.com/c/1454300
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629157}
parent 1767217b
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include "google_apis/gaia/gaia_auth_util.h" #include "google_apis/gaia/gaia_auth_util.h"
#include "google_apis/gaia/gaia_urls.h" #include "google_apis/gaia/gaia_urls.h"
#include "google_apis/gaia/google_service_auth_error.h" #include "google_apis/gaia/google_service_auth_error.h"
#include "services/identity/public/cpp/accounts_in_cookie_jar_info.h"
#include "services/identity/public/cpp/accounts_mutator.h" #include "services/identity/public/cpp/accounts_mutator.h"
using signin::AccountReconcilorDelegate; using signin::AccountReconcilorDelegate;
...@@ -194,7 +195,6 @@ AccountReconcilor::AccountReconcilor( ...@@ -194,7 +195,6 @@ AccountReconcilor::AccountReconcilor(
client_(client), client_(client),
cookie_manager_service_(cookie_manager_service), cookie_manager_service_(cookie_manager_service),
registered_with_identity_manager_(false), registered_with_identity_manager_(false),
registered_with_cookie_manager_service_(false),
registered_with_content_settings_(false), registered_with_content_settings_(false),
is_reconcile_started_(false), is_reconcile_started_(false),
first_execution_(true), first_execution_(true),
...@@ -216,7 +216,6 @@ AccountReconcilor::~AccountReconcilor() { ...@@ -216,7 +216,6 @@ AccountReconcilor::~AccountReconcilor() {
VLOG(1) << "AccountReconcilor::~AccountReconcilor"; VLOG(1) << "AccountReconcilor::~AccountReconcilor";
// Make sure shutdown was called first. // Make sure shutdown was called first.
DCHECK(!registered_with_identity_manager_); DCHECK(!registered_with_identity_manager_);
DCHECK(!registered_with_cookie_manager_service_);
} }
void AccountReconcilor::Initialize(bool start_reconcile_if_tokens_available) { void AccountReconcilor::Initialize(bool start_reconcile_if_tokens_available) {
...@@ -238,14 +237,12 @@ void AccountReconcilor::SetIsWKHTTPSystemCookieStoreEnabled(bool is_enabled) { ...@@ -238,14 +237,12 @@ void AccountReconcilor::SetIsWKHTTPSystemCookieStoreEnabled(bool is_enabled) {
void AccountReconcilor::EnableReconcile() { void AccountReconcilor::EnableReconcile() {
DCHECK(delegate_->IsReconcileEnabled()); DCHECK(delegate_->IsReconcileEnabled());
RegisterWithCookieManagerService();
RegisterWithContentSettings(); RegisterWithContentSettings();
RegisterWithIdentityManager(); RegisterWithIdentityManager();
} }
void AccountReconcilor::DisableReconcile(bool logout_all_accounts) { void AccountReconcilor::DisableReconcile(bool logout_all_accounts) {
AbortReconcile(); AbortReconcile();
UnregisterWithCookieManagerService();
UnregisterWithIdentityManager(); UnregisterWithIdentityManager();
UnregisterWithContentSettings(); UnregisterWithContentSettings();
...@@ -301,27 +298,6 @@ void AccountReconcilor::UnregisterWithIdentityManager() { ...@@ -301,27 +298,6 @@ void AccountReconcilor::UnregisterWithIdentityManager() {
registered_with_identity_manager_ = false; registered_with_identity_manager_ = false;
} }
void AccountReconcilor::RegisterWithCookieManagerService() {
VLOG(1) << "AccountReconcilor::RegisterWithCookieManagerService";
// During re-auth, the reconcilor will get a callback about successful signin
// even when the profile is already connected. Avoid re-registering
// with the helper since this will DCHECK.
if (registered_with_cookie_manager_service_)
return;
cookie_manager_service_->AddObserver(this);
registered_with_cookie_manager_service_ = true;
}
void AccountReconcilor::UnregisterWithCookieManagerService() {
VLOG(1) << "AccountReconcilor::UnregisterWithCookieManagerService";
if (!registered_with_cookie_manager_service_)
return;
cookie_manager_service_->RemoveObserver(this);
registered_with_cookie_manager_service_ = false;
}
signin_metrics::AccountReconcilorState AccountReconcilor::GetState() { signin_metrics::AccountReconcilorState AccountReconcilor::GetState() {
if (!is_reconcile_started_) { if (!is_reconcile_started_) {
return (error_during_last_reconcile_.state() != return (error_during_last_reconcile_.state() !=
...@@ -472,11 +448,13 @@ void AccountReconcilor::StartReconcile() { ...@@ -472,11 +448,13 @@ void AccountReconcilor::StartReconcile() {
return; return;
} }
// Rely on the GCMS to manage calls to and responses from ListAccounts. // Rely on the IdentityManager to manage calls to and responses from
std::vector<gaia::ListedAccount> gaia_accounts; // ListAccounts.
if (cookie_manager_service_->ListAccounts(&gaia_accounts, nullptr)) { identity::AccountsInCookieJarInfo accounts_in_cookie_jar =
OnGaiaAccountsInCookieUpdated( identity_manager_->GetAccountsInCookieJar();
gaia_accounts, std::vector<gaia::ListedAccount>(), if (accounts_in_cookie_jar.accounts_are_fresh) {
OnAccountsInCookieUpdated(
accounts_in_cookie_jar,
GoogleServiceAuthError(GoogleServiceAuthError::NONE)); GoogleServiceAuthError(GoogleServiceAuthError::NONE));
} }
} }
...@@ -529,11 +507,12 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint( ...@@ -529,11 +507,12 @@ void AccountReconcilor::FinishReconcileWithMultiloginEndpoint(
first_execution_ = false; first_execution_ = false;
} }
void AccountReconcilor::OnGaiaAccountsInCookieUpdated( void AccountReconcilor::OnAccountsInCookieUpdated(
const std::vector<gaia::ListedAccount>& accounts, const identity::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const std::vector<gaia::ListedAccount>& signed_out_accounts, const GoogleServiceAuthError& error) {
const GoogleServiceAuthError& error) { const std::vector<gaia::ListedAccount>& accounts(
VLOG(1) << "AccountReconcilor::OnGaiaAccountsInCookieUpdated: " accounts_in_cookie_jar_info.signed_in_accounts);
VLOG(1) << "AccountReconcilor::OnAccountsInCookieUpdated: "
<< "CookieJar " << accounts.size() << " accounts, " << "CookieJar " << accounts.size() << " accounts, "
<< "Reconcilor's state is " << is_reconcile_started_ << ", " << "Reconcilor's state is " << is_reconcile_started_ << ", "
<< "Error was " << error.ToString(); << "Error was " << error.ToString();
...@@ -605,7 +584,7 @@ void AccountReconcilor::OnGaiaAccountsInCookieUpdated( ...@@ -605,7 +584,7 @@ void AccountReconcilor::OnGaiaAccountsInCookieUpdated(
} }
} }
void AccountReconcilor::OnGaiaCookieDeletedByUserAction() { void AccountReconcilor::OnAccountsCookieDeletedByUserAction() {
if (!delegate_->ShouldRevokeTokensOnCookieDeleted()) if (!delegate_->ShouldRevokeTokensOnCookieDeleted())
return; return;
......
...@@ -41,7 +41,6 @@ class SigninClient; ...@@ -41,7 +41,6 @@ class SigninClient;
class AccountReconcilor : public KeyedService, class AccountReconcilor : public KeyedService,
public content_settings::Observer, public content_settings::Observer,
public GaiaCookieManagerService::Observer,
public identity::IdentityManager::Observer { public identity::IdentityManager::Observer {
public: public:
// When an instance of this class exists, the account reconcilor is suspended. // When an instance of this class exists, the account reconcilor is suspended.
...@@ -273,17 +272,6 @@ class AccountReconcilor : public KeyedService, ...@@ -273,17 +272,6 @@ class AccountReconcilor : public KeyedService,
ContentSettingsType content_type, ContentSettingsType content_type,
const std::string& resource_identifier) override; const std::string& resource_identifier) override;
// Overridden from GaiaGookieManagerService::Observer.
void OnAddAccountToCookieCompleted(
const std::string& account_id,
const GoogleServiceAuthError& error) override;
void OnSetAccountsInCookieCompleted(
const GoogleServiceAuthError& error) override;
void OnGaiaAccountsInCookieUpdated(
const std::vector<gaia::ListedAccount>& accounts,
const std::vector<gaia::ListedAccount>& signed_out_accounts,
const GoogleServiceAuthError& error) override;
void OnGaiaCookieDeletedByUserAction() override;
// Overridden from identity::IdentityManager::Observer. // Overridden from identity::IdentityManager::Observer.
void OnEndBatchOfRefreshTokenStateChanges() override; void OnEndBatchOfRefreshTokenStateChanges() override;
...@@ -291,6 +279,15 @@ class AccountReconcilor : public KeyedService, ...@@ -291,6 +279,15 @@ class AccountReconcilor : public KeyedService,
void OnErrorStateOfRefreshTokenUpdatedForAccount( void OnErrorStateOfRefreshTokenUpdatedForAccount(
const AccountInfo& account_info, const AccountInfo& account_info,
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
void OnAddAccountToCookieCompleted(
const std::string& account_id,
const GoogleServiceAuthError& error) override;
void OnSetAccountsInCookieCompleted(
const GoogleServiceAuthError& error) override;
void OnAccountsInCookieUpdated(
const identity::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const GoogleServiceAuthError& error) override;
void OnAccountsCookieDeletedByUserAction() override;
void FinishReconcileWithMultiloginEndpoint( void FinishReconcileWithMultiloginEndpoint(
const std::string& primary_account, const std::string& primary_account,
...@@ -321,7 +318,6 @@ class AccountReconcilor : public KeyedService, ...@@ -321,7 +318,6 @@ class AccountReconcilor : public KeyedService,
GaiaCookieManagerService* cookie_manager_service_; GaiaCookieManagerService* cookie_manager_service_;
bool registered_with_identity_manager_; bool registered_with_identity_manager_;
bool registered_with_cookie_manager_service_;
bool registered_with_content_settings_; bool registered_with_content_settings_;
// True while the reconcilor is busy checking or managing the accounts in // True while the reconcilor is busy checking or managing the accounts in
......
...@@ -280,7 +280,7 @@ class AccountReconcilorTest : public ::testing::Test { ...@@ -280,7 +280,7 @@ class AccountReconcilorTest : public ::testing::Test {
const std::string& username); const std::string& username);
void SimulateAddAccountToCookieCompleted( void SimulateAddAccountToCookieCompleted(
GaiaCookieManagerService::Observer* observer, identity::IdentityManager::Observer* observer,
const std::string& account_id, const std::string& account_id,
const GoogleServiceAuthError& error); const GoogleServiceAuthError& error);
...@@ -289,7 +289,7 @@ class AccountReconcilorTest : public ::testing::Test { ...@@ -289,7 +289,7 @@ class AccountReconcilorTest : public ::testing::Test {
const ContentSettingsPattern& primary_pattern); const ContentSettingsPattern& primary_pattern);
void SimulateSetAccountsInCookieCompleted( void SimulateSetAccountsInCookieCompleted(
GaiaCookieManagerService::Observer* observer, identity::IdentityManager::Observer* observer,
const GoogleServiceAuthError& error); const GoogleServiceAuthError& error);
void SetAccountConsistency(signin::AccountConsistencyMethod method); void SetAccountConsistency(signin::AccountConsistencyMethod method);
...@@ -452,14 +452,14 @@ std::string AccountReconcilorTest::SeedAccountInfo( ...@@ -452,14 +452,14 @@ std::string AccountReconcilorTest::SeedAccountInfo(
} }
void AccountReconcilorTest::SimulateAddAccountToCookieCompleted( void AccountReconcilorTest::SimulateAddAccountToCookieCompleted(
GaiaCookieManagerService::Observer* observer, identity::IdentityManager::Observer* observer,
const std::string& account_id, const std::string& account_id,
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
observer->OnAddAccountToCookieCompleted(account_id, error); observer->OnAddAccountToCookieCompleted(account_id, error);
} }
void AccountReconcilorTest::SimulateSetAccountsInCookieCompleted( void AccountReconcilorTest::SimulateSetAccountsInCookieCompleted(
GaiaCookieManagerService::Observer* observer, identity::IdentityManager::Observer* observer,
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
observer->OnSetAccountsInCookieCompleted(error); observer->OnSetAccountsInCookieCompleted(error);
} }
...@@ -1652,7 +1652,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) { ...@@ -1652,7 +1652,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) {
{ {
std::unique_ptr<AccountReconcilor::ScopedSyncedDataDeletion> deletion = std::unique_ptr<AccountReconcilor::ScopedSyncedDataDeletion> deletion =
reconcilor->GetScopedSyncDataDeletion(); reconcilor->GetScopedSyncDataDeletion();
reconcilor->OnGaiaCookieDeletedByUserAction(); reconcilor->OnAccountsCookieDeletedByUserAction();
EXPECT_TRUE( EXPECT_TRUE(
identity_manager->HasAccountWithRefreshToken(primary_account_id)); identity_manager->HasAccountWithRefreshToken(primary_account_id));
EXPECT_FALSE( EXPECT_FALSE(
...@@ -1663,7 +1663,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) { ...@@ -1663,7 +1663,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) {
} }
identity_test_env()->SetRefreshTokenForAccount(secondary_account_id); identity_test_env()->SetRefreshTokenForAccount(secondary_account_id);
reconcilor->OnGaiaCookieDeletedByUserAction(); reconcilor->OnAccountsCookieDeletedByUserAction();
// Without scoped deletion, the primary token is also invalidated. // Without scoped deletion, the primary token is also invalidated.
EXPECT_TRUE(identity_manager->HasAccountWithRefreshToken(primary_account_id)); EXPECT_TRUE(identity_manager->HasAccountWithRefreshToken(primary_account_id));
...@@ -1691,7 +1691,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) { ...@@ -1691,7 +1691,7 @@ TEST_F(AccountReconcilorTest, DiceDeleteCookie) {
{ {
std::unique_ptr<AccountReconcilor::ScopedSyncedDataDeletion> deletion = std::unique_ptr<AccountReconcilor::ScopedSyncedDataDeletion> deletion =
reconcilor->GetScopedSyncDataDeletion(); reconcilor->GetScopedSyncDataDeletion();
reconcilor->OnGaiaCookieDeletedByUserAction(); reconcilor->OnAccountsCookieDeletedByUserAction();
EXPECT_EQ(GoogleServiceAuthError::InvalidGaiaCredentialsReason:: EXPECT_EQ(GoogleServiceAuthError::InvalidGaiaCredentialsReason::
CREDENTIALS_REJECTED_BY_CLIENT, CREDENTIALS_REJECTED_BY_CLIENT,
identity_manager identity_manager
...@@ -1983,8 +1983,10 @@ TEST_P(AccountReconcilorMirrorEndpointParamTest, ...@@ -1983,8 +1983,10 @@ TEST_P(AccountReconcilorMirrorEndpointParamTest,
// Add extra cookie change notification. Reconcilor should ignore it. // Add extra cookie change notification. Reconcilor should ignore it.
gaia::ListedAccount listed_account = gaia::ListedAccount listed_account =
ListedAccountFromCookieParams(cookie_params, account_id); ListedAccountFromCookieParams(cookie_params, account_id);
reconcilor->OnGaiaAccountsInCookieUpdated( identity::AccountsInCookieJarInfo accounts_in_cookie_jar_info = {
{listed_account}, {}, GoogleServiceAuthError::AuthErrorNone()); /*accounts_are_fresh=*/true, {listed_account}, {}};
reconcilor->OnAccountsInCookieUpdated(
accounts_in_cookie_jar_info, GoogleServiceAuthError::AuthErrorNone());
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment