Commit 6a6d719f authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Reland "[s13n] Migrate ContentAreaAccountsMigration away from GCMS::Observer"

This reverts commit 71359b31.

Reason for revert: message from the sheriff that reverted it [1]:

"
Well, theoretically there still could have been an interaction of
your CL with another one that would have caused the failing test to 
change from ActivityKeptInPref to SupervisedUserRemoved.

That said, the MSan build with my revert just finished and it's 
indeed still red. So your CL likely indeed wasn't the culprit.

Sorry for that. Please go ahead and reland.
"

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=926154#c10
Original change's description:
> Revert "[s13n] Migrate ContentAreaAccountsMigration away from GCMS::Observer"
> 
> This reverts commit 5f7f5569.
> 
> Reason for revert: Seems to break multiple Chrome OS trybots. See crbug.com/926154 for more details.
> 
> Original change's description:
> > [s13n] Migrate ContentAreaAccountsMigration away from GCMS::Observer
> > 
> > CL changes the inheritance ofi ContentAreaAccountsMigration from
> > GaiaCookieManagerService::Observer to IdentityManager::Observer instead.
> > 
> > No functionality change expected.
> > 
> > BUG=859882
> > 
> > Change-Id: I907b2624d23e93b6a0ecbcda3e39f0322f238612
> > Reviewed-on: https://chromium-review.googlesource.com/c/1436916
> > Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > Reviewed-by: Lowell Manners <lowell@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#626748}
> 
> TBR=xiyuan@chromium.org,tonikitoo@igalia.com,lowell@chromium.org
> 
> Change-Id: Ide04520a6a6f249e5220a6b9086c80b591bc923e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 859882,926154
> Reviewed-on: https://chromium-review.googlesource.com/c/1442197
> Reviewed-by: Martin Šrámek <msramek@chromium.org>
> Commit-Queue: Martin Šrámek <msramek@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#626972}

TBR=xiyuan@chromium.org,msramek@chromium.org,tonikitoo@igalia.com,lowell@chromium.org

Change-Id: I565d0335169be1e4fb61954f0d2f1a3dc8319cc7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 859882, 926154
Reviewed-on: https://chromium-review.googlesource.com/c/1443231Reviewed-by: default avatarAntonio Gomes <tonikitoo@igalia.com>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#627010}
parent b119c2f7
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/account_reconcilor_factory.h" #include "chrome/browser/signin/account_reconcilor_factory.h"
#include "chrome/browser/signin/account_tracker_service_factory.h" #include "chrome/browser/signin/account_tracker_service_factory.h"
#include "chrome/browser/signin/gaia_cookie_manager_service_factory.h"
#include "chrome/browser/signin/identity_manager_factory.h" #include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/web_data_service_factory.h" #include "chrome/browser/web_data_service_factory.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
...@@ -40,9 +39,9 @@ ...@@ -40,9 +39,9 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/signin/core/browser/account_reconcilor.h" #include "components/signin/core/browser/account_reconcilor.h"
#include "components/signin/core/browser/account_tracker_service.h" #include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/gaia_cookie_manager_service.h"
#include "components/signin/core/browser/webdata/token_web_data.h" #include "components/signin/core/browser/webdata/token_web_data.h"
#include "components/webdata/common/web_data_service_consumer.h" #include "components/webdata/common/web_data_service_consumer.h"
#include "services/identity/public/cpp/accounts_in_cookie_jar_info.h"
#include "services/identity/public/cpp/identity_manager.h" #include "services/identity/public/cpp/identity_manager.h"
namespace chromeos { namespace chromeos {
...@@ -273,39 +272,34 @@ class DeviceAccountMigration : public AccountMigrationBaseStep, ...@@ -273,39 +272,34 @@ class DeviceAccountMigration : public AccountMigrationBaseStep,
// to |AccountManager|. The objective is to migrate the account names only. We // to |AccountManager|. The objective is to migrate the account names only. We
// cannot migrate any credentials (cookies). // cannot migrate any credentials (cookies).
class ContentAreaAccountsMigration : public AccountMigrationBaseStep, class ContentAreaAccountsMigration : public AccountMigrationBaseStep,
GaiaCookieManagerService::Observer { identity::IdentityManager::Observer {
public: public:
ContentAreaAccountsMigration( ContentAreaAccountsMigration(AccountManager* account_manager,
AccountManager* account_manager, identity::IdentityManager* identity_manager)
identity::IdentityManager* identity_manager,
GaiaCookieManagerService* gaia_cookie_manager_service)
: AccountMigrationBaseStep(kContentAreaAccountsMigration, : AccountMigrationBaseStep(kContentAreaAccountsMigration,
account_manager, account_manager,
identity_manager), identity_manager),
gaia_cookie_manager_service_(gaia_cookie_manager_service) {} identity_manager_(identity_manager) {}
~ContentAreaAccountsMigration() override { ~ContentAreaAccountsMigration() override {
gaia_cookie_manager_service_->RemoveObserver(this); identity_manager_->RemoveObserver(this);
} }
private: private:
void StartMigration() override { void StartMigration() override {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::vector<gaia::ListedAccount> signed_in_content_area_accounts; identity_manager_->AddObserver(this);
std::vector<gaia::ListedAccount> signed_out_content_area_accounts; identity::AccountsInCookieJarInfo accounts_in_cookie_jar_info =
gaia_cookie_manager_service_->AddObserver(this); identity_manager_->GetAccountsInCookieJar();
if (gaia_cookie_manager_service_->ListAccounts( if (accounts_in_cookie_jar_info.accounts_are_fresh) {
&signed_in_content_area_accounts, OnAccountsInCookieUpdated(
&signed_out_content_area_accounts)) { accounts_in_cookie_jar_info,
OnGaiaAccountsInCookieUpdated(
signed_in_content_area_accounts, signed_out_content_area_accounts,
GoogleServiceAuthError(GoogleServiceAuthError::NONE)); GoogleServiceAuthError(GoogleServiceAuthError::NONE));
} }
} }
void OnGaiaAccountsInCookieUpdated( void OnAccountsInCookieUpdated(
const std::vector<gaia::ListedAccount>& signed_in_content_area_accounts, const identity::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const std::vector<gaia::ListedAccount>& signed_out_content_area_accounts,
const GoogleServiceAuthError& error) override { const GoogleServiceAuthError& error) override {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// We should not have reached here without |OnGetAccounts| having been // We should not have reached here without |OnGetAccounts| having been
...@@ -313,10 +307,10 @@ class ContentAreaAccountsMigration : public AccountMigrationBaseStep, ...@@ -313,10 +307,10 @@ class ContentAreaAccountsMigration : public AccountMigrationBaseStep,
// Furthermore, Account Manager must have been populated with the Device // Furthermore, Account Manager must have been populated with the Device
// Account before this |Step| is run. // Account before this |Step| is run.
DCHECK(!IsAccountManagerEmpty()); DCHECK(!IsAccountManagerEmpty());
gaia_cookie_manager_service_->RemoveObserver(this); identity_manager_->RemoveObserver(this);
MigrateAccounts(signed_in_content_area_accounts, MigrateAccounts(accounts_in_cookie_jar_info.signed_in_accounts,
signed_out_content_area_accounts); accounts_in_cookie_jar_info.signed_out_accounts);
FinishWithSuccess(); FinishWithSuccess();
} }
...@@ -334,8 +328,8 @@ class ContentAreaAccountsMigration : public AccountMigrationBaseStep, ...@@ -334,8 +328,8 @@ class ContentAreaAccountsMigration : public AccountMigrationBaseStep,
} }
} }
// A non-owning pointer to |GaiaCookieManagerService|. // A non-owning pointer to |IdentityManager|.
GaiaCookieManagerService* const gaia_cookie_manager_service_; identity::IdentityManager* const identity_manager_;
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
...@@ -524,9 +518,7 @@ void AccountManagerMigrator::AddMigrationSteps() { ...@@ -524,9 +518,7 @@ void AccountManagerMigrator::AddMigrationSteps() {
WebDataServiceFactory::GetTokenWebDataForProfile( WebDataServiceFactory::GetTokenWebDataForProfile(
profile_, ServiceAccessType::EXPLICIT_ACCESS) /* token_web_data */)); profile_, ServiceAccessType::EXPLICIT_ACCESS) /* token_web_data */));
migration_runner_.AddStep(std::make_unique<ContentAreaAccountsMigration>( migration_runner_.AddStep(std::make_unique<ContentAreaAccountsMigration>(
account_manager, identity_manager, account_manager, identity_manager));
GaiaCookieManagerServiceFactory::GetForProfile(
profile_) /* gaia_cookie_manager_service */));
if (arc::IsArcProvisioned(profile_)) { if (arc::IsArcProvisioned(profile_)) {
// Add a migration step for ARC only if ARC has been provisioned. If ARC has // Add a migration step for ARC only if ARC has been provisioned. If ARC has
...@@ -610,7 +602,7 @@ AccountManagerMigratorFactory::AccountManagerMigratorFactory() ...@@ -610,7 +602,7 @@ AccountManagerMigratorFactory::AccountManagerMigratorFactory()
// be re-enabled once migration is done. // be re-enabled once migration is done.
DependsOn(AccountReconcilorFactory::GetInstance()); DependsOn(AccountReconcilorFactory::GetInstance());
// For getting Chrome content area accounts. // For getting Chrome content area accounts.
DependsOn(GaiaCookieManagerServiceFactory::GetInstance()); DependsOn(IdentityManagerFactory::GetInstance());
} }
AccountManagerMigratorFactory::~AccountManagerMigratorFactory() = default; AccountManagerMigratorFactory::~AccountManagerMigratorFactory() = default;
......
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