Commit 5c868eac authored by Daniel Classon's avatar Daniel Classon Committed by Commit Bot

[OsPeoplePage] Fix "Remove Account" search tag when no second account

Fixes a bug where the "Remove Account" search tag showed up in search
when there was no second account to remove.

Bug: 1116553
Change-Id: I800ebc0d667d0417303f0d11458ea152e1849589
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2422457
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809525}
parent f8d6dbcd
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "chrome/browser/browser_process_platform_part.h" #include "chrome/browser/browser_process_platform_part.h"
#include "chrome/browser/chromeos/account_manager/account_manager_util.h" #include "chrome/browser/chromeos/account_manager/account_manager_util.h"
#include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_utils.h" #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_utils.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/ui/ui_features.h" #include "chrome/browser/ui/ui_features.h"
...@@ -44,6 +45,7 @@ ...@@ -44,6 +45,7 @@
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/sync/driver/sync_service.h" #include "components/sync/driver/sync_service.h"
#include "components/sync/driver/sync_user_settings.h" #include "components/sync/driver/sync_user_settings.h"
#include "components/user_manager/user.h"
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "content/public/browser/web_ui.h" #include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h" #include "content/public/browser/web_ui_data_source.h"
...@@ -113,12 +115,6 @@ const std::vector<SearchConcept>& GetPeopleSearchConcepts() { ...@@ -113,12 +115,6 @@ const std::vector<SearchConcept>& GetPeopleSearchConcepts() {
{.setting = mojom::Setting::kLockScreen}, {.setting = mojom::Setting::kLockScreen},
{IDS_OS_SETTINGS_TAG_LOCK_SCREEN_WHEN_WAKING_ALT1, {IDS_OS_SETTINGS_TAG_LOCK_SCREEN_WHEN_WAKING_ALT1,
SearchConcept::kAltTagEnd}}, SearchConcept::kAltTagEnd}},
{IDS_OS_SETTINGS_TAG_PEOPLE_ACCOUNTS_REMOVE,
mojom::kMyAccountsSubpagePath,
mojom::SearchResultIcon::kAvatar,
mojom::SearchResultDefaultRank::kMedium,
mojom::SearchResultType::kSetting,
{.setting = mojom::Setting::kRemoveAccount}},
{IDS_OS_SETTINGS_TAG_RESTRICT_SIGN_IN, {IDS_OS_SETTINGS_TAG_RESTRICT_SIGN_IN,
mojom::kManageOtherPeopleSubpagePath, mojom::kManageOtherPeopleSubpagePath,
mojom::SearchResultIcon::kAvatar, mojom::SearchResultIcon::kAvatar,
...@@ -142,6 +138,18 @@ const std::vector<SearchConcept>& GetPeopleSearchConcepts() { ...@@ -142,6 +138,18 @@ const std::vector<SearchConcept>& GetPeopleSearchConcepts() {
return *tags; return *tags;
} }
const std::vector<SearchConcept>& GetRemoveAccountSearchConcepts() {
static const base::NoDestructor<std::vector<SearchConcept>> tags({
{IDS_OS_SETTINGS_TAG_PEOPLE_ACCOUNTS_REMOVE,
mojom::kMyAccountsSubpagePath,
mojom::SearchResultIcon::kAvatar,
mojom::SearchResultDefaultRank::kMedium,
mojom::SearchResultType::kSetting,
{.setting = mojom::Setting::kRemoveAccount}},
});
return *tags;
}
const std::vector<SearchConcept>& GetNonSplitSyncSearchConcepts() { const std::vector<SearchConcept>& GetNonSplitSyncSearchConcepts() {
static const base::NoDestructor<std::vector<SearchConcept>> tags({ static const base::NoDestructor<std::vector<SearchConcept>> tags({
{IDS_OS_SETTINGS_TAG_SYNC_AND_GOOGLE_SERVICES, {IDS_OS_SETTINGS_TAG_SYNC_AND_GOOGLE_SERVICES,
...@@ -672,6 +680,20 @@ void AddParentalControlStrings(content::WebUIDataSource* html_source, ...@@ -672,6 +680,20 @@ void AddParentalControlStrings(content::WebUIDataSource* html_source,
tooltip); tooltip);
} }
bool IsSameAccount(const AccountManager::AccountKey& account_key,
const AccountId& account_id) {
switch (account_key.account_type) {
case account_manager::AccountType::ACCOUNT_TYPE_GAIA:
return account_id.GetAccountType() == AccountType::GOOGLE &&
account_id.GetGaiaId() == account_key.id;
case account_manager::AccountType::ACCOUNT_TYPE_ACTIVE_DIRECTORY:
return account_id.GetAccountType() == AccountType::ACTIVE_DIRECTORY &&
account_id.GetObjGuid() == account_key.id;
case account_manager::AccountType::ACCOUNT_TYPE_UNSPECIFIED:
return false;
}
}
} // namespace } // namespace
PeopleSection::PeopleSection( PeopleSection::PeopleSection(
...@@ -695,6 +717,19 @@ PeopleSection::PeopleSection( ...@@ -695,6 +717,19 @@ PeopleSection::PeopleSection(
SearchTagRegistry::ScopedTagUpdater updater = registry()->StartUpdate(); SearchTagRegistry::ScopedTagUpdater updater = registry()->StartUpdate();
updater.AddSearchTags(GetPeopleSearchConcepts()); updater.AddSearchTags(GetPeopleSearchConcepts());
// TODO(jamescook): Sort out how account management is split between Chrome
// OS and browser settings.
if (IsAccountManagerAvailable(profile)) {
// Some Account Manager search tags are added/removed dynamically.
AccountManagerFactory* factory =
g_browser_process->platform_part()->GetAccountManagerFactory();
account_manager_ = factory->GetAccountManager(profile->GetPath().value());
DCHECK(account_manager_);
account_manager_->AddObserver(this);
FetchAccounts();
}
if (kerberos_credentials_manager_) { if (kerberos_credentials_manager_) {
// Kerberos search tags are added/removed dynamically. // Kerberos search tags are added/removed dynamically.
kerberos_credentials_manager_->AddObserver(this); kerberos_credentials_manager_->AddObserver(this);
...@@ -730,6 +765,9 @@ PeopleSection::~PeopleSection() { ...@@ -730,6 +765,9 @@ PeopleSection::~PeopleSection() {
if (chromeos::features::IsSplitSettingsSyncEnabled() && sync_service_) if (chromeos::features::IsSplitSettingsSyncEnabled() && sync_service_)
sync_service_->RemoveObserver(this); sync_service_->RemoveObserver(this);
if (account_manager_)
account_manager_->RemoveObserver(this);
} }
void PeopleSection::AddLoadTimeData(content::WebUIDataSource* html_source) { void PeopleSection::AddLoadTimeData(content::WebUIDataSource* html_source) {
...@@ -754,7 +792,7 @@ void PeopleSection::AddLoadTimeData(content::WebUIDataSource* html_source) { ...@@ -754,7 +792,7 @@ void PeopleSection::AddLoadTimeData(content::WebUIDataSource* html_source) {
// Toggles the Chrome OS Account Manager submenu in the People section. // Toggles the Chrome OS Account Manager submenu in the People section.
html_source->AddBoolean("isAccountManagerEnabled", html_source->AddBoolean("isAccountManagerEnabled",
chromeos::IsAccountManagerAvailable(profile())); account_manager_ != nullptr);
if (chromeos::features::ShouldUseBrowserSyncConsent()) { if (chromeos::features::ShouldUseBrowserSyncConsent()) {
static constexpr webui::LocalizedString kTurnOffStrings[] = { static constexpr webui::LocalizedString kTurnOffStrings[] = {
...@@ -822,18 +860,10 @@ void PeopleSection::AddHandlers(content::WebUI* web_ui) { ...@@ -822,18 +860,10 @@ void PeopleSection::AddHandlers(content::WebUI* web_ui) {
IDS_OS_SETTINGS_PROFILE_LABEL); IDS_OS_SETTINGS_PROFILE_LABEL);
web_ui->AddMessageHandler(std::move(plural_string_handler)); web_ui->AddMessageHandler(std::move(plural_string_handler));
// TODO(jamescook): Sort out how account management is split between Chrome OS if (account_manager_) {
// and browser settings.
if (chromeos::IsAccountManagerAvailable(profile())) {
chromeos::AccountManagerFactory* factory =
g_browser_process->platform_part()->GetAccountManagerFactory();
chromeos::AccountManager* account_manager =
factory->GetAccountManager(profile()->GetPath().value());
DCHECK(account_manager);
web_ui->AddMessageHandler( web_ui->AddMessageHandler(
std::make_unique<chromeos::settings::AccountManagerUIHandler>( std::make_unique<chromeos::settings::AccountManagerUIHandler>(
account_manager, identity_manager_)); account_manager_, identity_manager_));
} }
if (chromeos::features::IsSplitSettingsSyncEnabled()) if (chromeos::features::IsSplitSettingsSyncEnabled())
...@@ -976,6 +1006,41 @@ void PeopleSection::RegisterHierarchy(HierarchyGenerator* generator) const { ...@@ -976,6 +1006,41 @@ void PeopleSection::RegisterHierarchy(HierarchyGenerator* generator) const {
generator); generator);
} }
void PeopleSection::FetchAccounts() {
account_manager_->GetAccounts(
base::BindOnce(&PeopleSection::UpdateAccountManagerSearchTags,
weak_factory_.GetWeakPtr()));
}
void PeopleSection::OnTokenUpserted(const AccountManager::Account& account) {
FetchAccounts();
}
void PeopleSection::OnAccountRemoved(const AccountManager::Account& account) {
FetchAccounts();
}
void PeopleSection::UpdateAccountManagerSearchTags(
const std::vector<AccountManager::Account>& accounts) {
DCHECK(IsAccountManagerAvailable(profile()));
// Start with no Account Manager search tags.
SearchTagRegistry::ScopedTagUpdater updater = registry()->StartUpdate();
updater.RemoveSearchTags(GetRemoveAccountSearchConcepts());
user_manager::User* user = ProfileHelper::Get()->GetUserByProfile(profile());
DCHECK(user);
for (const AccountManager::Account& account : accounts) {
if (IsSameAccount(account.key, user->GetAccountId()))
continue;
// If a non-device account exists, add the "Remove Account" search tag.
updater.AddSearchTags(GetRemoveAccountSearchConcepts());
return;
}
}
void PeopleSection::OnStateChanged(syncer::SyncService* sync_service) { void PeopleSection::OnStateChanged(syncer::SyncService* sync_service) {
DCHECK(chromeos::features::IsSplitSettingsSyncEnabled()); DCHECK(chromeos::features::IsSplitSettingsSyncEnabled());
DCHECK_EQ(sync_service, sync_service_); DCHECK_EQ(sync_service, sync_service_);
......
...@@ -5,8 +5,10 @@ ...@@ -5,8 +5,10 @@
#ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_PEOPLE_SECTION_H_ #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_PEOPLE_SECTION_H_
#define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_PEOPLE_SECTION_H_ #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_PEOPLE_SECTION_H_
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h" #include "chrome/browser/chromeos/kerberos/kerberos_credentials_manager.h"
#include "chrome/browser/ui/webui/settings/chromeos/os_settings_section.h" #include "chrome/browser/ui/webui/settings/chromeos/os_settings_section.h"
#include "chromeos/components/account_manager/account_manager.h"
#include "components/sync/driver/sync_service_observer.h" #include "components/sync/driver/sync_service_observer.h"
class PrefService; class PrefService;
...@@ -38,6 +40,7 @@ class SearchTagRegistry; ...@@ -38,6 +40,7 @@ class SearchTagRegistry;
// they are allowed by policy/flags. Different sets of Sync tags are shown // they are allowed by policy/flags. Different sets of Sync tags are shown
// depending on whether the feature is enabed or disabled. // depending on whether the feature is enabed or disabled.
class PeopleSection : public OsSettingsSection, class PeopleSection : public OsSettingsSection,
public AccountManager::Observer,
public syncer::SyncServiceObserver, public syncer::SyncServiceObserver,
public KerberosCredentialsManager::Observer { public KerberosCredentialsManager::Observer {
public: public:
...@@ -60,6 +63,10 @@ class PeopleSection : public OsSettingsSection, ...@@ -60,6 +63,10 @@ class PeopleSection : public OsSettingsSection,
std::string GetSectionPath() const override; std::string GetSectionPath() const override;
void RegisterHierarchy(HierarchyGenerator* generator) const override; void RegisterHierarchy(HierarchyGenerator* generator) const override;
// AccountManager::Observer:
void OnTokenUpserted(const AccountManager::Account& account) override;
void OnAccountRemoved(const AccountManager::Account& account) override;
// syncer::SyncServiceObserver: // syncer::SyncServiceObserver:
void OnStateChanged(syncer::SyncService* sync_service) override; void OnStateChanged(syncer::SyncService* sync_service) override;
...@@ -69,12 +76,17 @@ class PeopleSection : public OsSettingsSection, ...@@ -69,12 +76,17 @@ class PeopleSection : public OsSettingsSection,
void AddKerberosAccountsPageStrings( void AddKerberosAccountsPageStrings(
content::WebUIDataSource* html_source) const; content::WebUIDataSource* html_source) const;
bool AreFingerprintSettingsAllowed(); bool AreFingerprintSettingsAllowed();
void FetchAccounts();
void UpdateAccountManagerSearchTags(
const std::vector<AccountManager::Account>& accounts);
AccountManager* account_manager_ = nullptr;
syncer::SyncService* sync_service_; syncer::SyncService* sync_service_;
SupervisedUserService* supervised_user_service_; SupervisedUserService* supervised_user_service_;
KerberosCredentialsManager* kerberos_credentials_manager_; KerberosCredentialsManager* kerberos_credentials_manager_;
signin::IdentityManager* identity_manager_; signin::IdentityManager* identity_manager_;
PrefService* pref_service_; PrefService* pref_service_;
base::WeakPtrFactory<PeopleSection> weak_factory_{this};
}; };
} // namespace settings } // namespace settings
......
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