Commit 8e59e937 authored by Mario Sanchez Prada's avatar Mario Sanchez Prada Committed by Commit Bot

Move away from AccountTrackerService in AccountManagerUIHandler

Use the IdentityManager and observe events from IdentityManager::Observer
to replace all direct usage of AccountTrackerService's APIs, leaving for
now just a reference to AccountTrackerService, required to initialize the
chromeos::AccountMapperUtil private member from the constructor.

Bug: 922785, 922786
Change-Id: Ieec3c3818a6993739fa204b112add0e5cc2f7f7b
Reviewed-on: https://chromium-review.googlesource.com/c/1437194
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626257}
parent 555aec39
...@@ -59,14 +59,13 @@ AccountManagerUIHandler::AccountManagerUIHandler( ...@@ -59,14 +59,13 @@ AccountManagerUIHandler::AccountManagerUIHandler(
AccountTrackerService* account_tracker_service, AccountTrackerService* account_tracker_service,
identity::IdentityManager* identity_manager) identity::IdentityManager* identity_manager)
: account_manager_(account_manager), : account_manager_(account_manager),
account_tracker_service_(account_tracker_service),
identity_manager_(identity_manager), identity_manager_(identity_manager),
account_mapper_util_(account_tracker_service_), account_mapper_util_(account_tracker_service),
account_manager_observer_(this), account_manager_observer_(this),
account_tracker_service_observer_(this), identity_manager_observer_(this),
weak_factory_(this) { weak_factory_(this) {
DCHECK(account_manager_); DCHECK(account_manager_);
DCHECK(account_tracker_service_); DCHECK(identity_manager_);
} }
AccountManagerUIHandler::~AccountManagerUIHandler() = default; AccountManagerUIHandler::~AccountManagerUIHandler() = default;
...@@ -123,9 +122,6 @@ void AccountManagerUIHandler::GetAccountsCallbackHandler( ...@@ -123,9 +122,6 @@ void AccountManagerUIHandler::GetAccountsCallbackHandler(
account_manager::AccountType::ACCOUNT_TYPE_GAIA) { account_manager::AccountType::ACCOUNT_TYPE_GAIA) {
continue; continue;
} }
AccountInfo account_info =
account_tracker_service_->FindAccountInfoByGaiaId(account_key.id);
DCHECK(!account_info.IsEmpty());
base::DictionaryValue account; base::DictionaryValue account;
account.SetString("id", account_key.id); account.SetString("id", account_key.id);
...@@ -140,11 +136,18 @@ void AccountManagerUIHandler::GetAccountsCallbackHandler( ...@@ -140,11 +136,18 @@ void AccountManagerUIHandler::GetAccountsCallbackHandler(
!identity_manager_ !identity_manager_
->HasAccountWithRefreshTokenInPersistentErrorState( ->HasAccountWithRefreshTokenInPersistentErrorState(
oauth_account_id)); oauth_account_id));
account.SetString("fullName", account_info.full_name);
account.SetString("email", account_info.email); base::Optional<AccountInfo> maybe_account_info =
if (!account_info.account_image.IsEmpty()) { identity_manager_->FindAccountInfoForAccountWithRefreshTokenByGaiaId(
account.SetString("pic", webui::GetBitmapDataUrl( account_key.id);
account_info.account_image.AsBitmap())); DCHECK(maybe_account_info.has_value());
account.SetString("fullName", maybe_account_info->full_name);
account.SetString("email", maybe_account_info->email);
if (!maybe_account_info->account_image.IsEmpty()) {
account.SetString("pic",
webui::GetBitmapDataUrl(
maybe_account_info->account_image.AsBitmap()));
} else { } else {
gfx::ImageSkia default_icon = gfx::ImageSkia default_icon =
*ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed( *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
...@@ -214,19 +217,19 @@ void AccountManagerUIHandler::HandleShowWelcomeDialogIfRequired( ...@@ -214,19 +217,19 @@ void AccountManagerUIHandler::HandleShowWelcomeDialogIfRequired(
void AccountManagerUIHandler::OnJavascriptAllowed() { void AccountManagerUIHandler::OnJavascriptAllowed() {
account_manager_observer_.Add(account_manager_); account_manager_observer_.Add(account_manager_);
account_tracker_service_observer_.Add(account_tracker_service_); identity_manager_observer_.Add(identity_manager_);
} }
void AccountManagerUIHandler::OnJavascriptDisallowed() { void AccountManagerUIHandler::OnJavascriptDisallowed() {
account_manager_observer_.RemoveAll(); account_manager_observer_.RemoveAll();
account_tracker_service_observer_.RemoveAll(); identity_manager_observer_.RemoveAll();
} }
// |AccountManager::Observer| overrides. // |AccountManager::Observer| overrides. Note: We need to listen on
// Note: We need to listen on |AccountManager| in addition to // |AccountManager| in addition to |IdentityManager| because there is no
// |AccountTrackerService| because there is no guarantee that |AccountManager| // guarantee that |AccountManager| (our source of truth) will have a newly added
// (our source of truth) will have a newly added account by the time // account by the time the underlying |AccountTrackerService| managed by the
// |AccountTrackerService| has it. // |IdentityManager| has it.
void AccountManagerUIHandler::OnTokenUpserted( void AccountManagerUIHandler::OnTokenUpserted(
const AccountManager::AccountKey& account_key) { const AccountManager::AccountKey& account_key) {
RefreshUI(); RefreshUI();
...@@ -237,18 +240,18 @@ void AccountManagerUIHandler::OnAccountRemoved( ...@@ -237,18 +240,18 @@ void AccountManagerUIHandler::OnAccountRemoved(
RefreshUI(); RefreshUI();
} }
// |AccountTrackerService::Observer| overrides. // |identity::IdentityManager::Observer| overrides. For newly added accounts,
// For newly added accounts, |AccountTrackerService| may take some time to // |identity::IdentityManager| may take some time to fetch user's full name and
// fetch user's full name and account image. Whenever that is completed, we // account image. Whenever that is completed, we may need to update the UI with
// may need to update the UI with this new set of information. // this new set of information. Note that we may be listening to
// Note that we may be listening to |AccountTrackerService| but we still // |identity::IdentityManager| but we still consider |AccountManager| to be the
// consider |AccountManager| to be the source of truth for account list. // source of truth for account list.
void AccountManagerUIHandler::OnAccountUpdated(const AccountInfo& info) { void AccountManagerUIHandler::OnAccountUpdated(const AccountInfo& info) {
RefreshUI(); RefreshUI();
} }
void AccountManagerUIHandler::OnAccountRemoved(const AccountInfo& account_key) { void AccountManagerUIHandler::OnAccountRemovedWithInfo(
} const AccountInfo& account_key) {}
void AccountManagerUIHandler::RefreshUI() { void AccountManagerUIHandler::RefreshUI() {
FireWebUIListener("accounts-changed"); FireWebUIListener("accounts-changed");
......
...@@ -14,18 +14,19 @@ ...@@ -14,18 +14,19 @@
#include "chrome/browser/chromeos/account_mapper_util.h" #include "chrome/browser/chromeos/account_mapper_util.h"
#include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h" #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
#include "chromeos/account_manager/account_manager.h" #include "chromeos/account_manager/account_manager.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "services/identity/public/cpp/identity_manager.h" #include "services/identity/public/cpp/identity_manager.h"
class AccountTrackerService;
namespace chromeos { namespace chromeos {
namespace settings { namespace settings {
class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler, class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler,
public AccountManager::Observer, public AccountManager::Observer,
public AccountTrackerService::Observer { public identity::IdentityManager::Observer {
public: public:
// Accepts non-owning pointers to |AccountManager| and // Accepts non-owning pointers to |AccountManager|, |AccountTrackerService|
// |AccountTrackerService|. Both of these must outlive |this| instance. // and |IdentityManager|. Both of these must outlive |this| instance.
AccountManagerUIHandler(AccountManager* account_manager, AccountManagerUIHandler(AccountManager* account_manager,
AccountTrackerService* account_tracker_service, AccountTrackerService* account_tracker_service,
identity::IdentityManager* identity_manager); identity::IdentityManager* identity_manager);
...@@ -42,9 +43,9 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler, ...@@ -42,9 +43,9 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler,
void OnTokenUpserted(const AccountManager::AccountKey& account_key) override; void OnTokenUpserted(const AccountManager::AccountKey& account_key) override;
void OnAccountRemoved(const AccountManager::AccountKey& account_key) override; void OnAccountRemoved(const AccountManager::AccountKey& account_key) override;
// |AccountTrackerService::Observer| overrides. // |identity::IdentityManager::Observer| overrides.
void OnAccountUpdated(const AccountInfo& info) override; void OnAccountUpdated(const AccountInfo& info) override;
void OnAccountRemoved(const AccountInfo& account_key) override; void OnAccountRemovedWithInfo(const AccountInfo& account_key) override;
private: private:
// WebUI "getAccounts" message callback. // WebUI "getAccounts" message callback.
...@@ -73,9 +74,6 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler, ...@@ -73,9 +74,6 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler,
// A non-owning pointer to |AccountManager|. // A non-owning pointer to |AccountManager|.
AccountManager* const account_manager_; AccountManager* const account_manager_;
// A non-owning pointer to |AccountTrackerService|.
AccountTrackerService* const account_tracker_service_;
// A non-owning pointer to |IdentityManager|. // A non-owning pointer to |IdentityManager|.
identity::IdentityManager* const identity_manager_; identity::IdentityManager* const identity_manager_;
...@@ -86,10 +84,10 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler, ...@@ -86,10 +84,10 @@ class AccountManagerUIHandler : public ::settings::SettingsPageUIHandler,
ScopedObserver<AccountManager, AccountManager::Observer> ScopedObserver<AccountManager, AccountManager::Observer>
account_manager_observer_; account_manager_observer_;
// An observer for |AccountTrackerService|. Automatically deregisters when // An observer for |identity::IdentityManager|. Automatically deregisters when
// |this| is destructed. // |this| is destructed.
ScopedObserver<AccountTrackerService, AccountTrackerService::Observer> ScopedObserver<identity::IdentityManager, identity::IdentityManager::Observer>
account_tracker_service_observer_; identity_manager_observer_;
base::WeakPtrFactory<AccountManagerUIHandler> weak_factory_; base::WeakPtrFactory<AccountManagerUIHandler> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(AccountManagerUIHandler); DISALLOW_COPY_AND_ASSIGN(AccountManagerUIHandler);
......
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