From 465933b5e78eca669273df5f1ef5351e2cd3cbf5 Mon Sep 17 00:00:00 2001 From: vabr Date: Wed, 20 May 2015 14:24:59 -0700 Subject: [PATCH] Prepare password_bubble_experiment and password_manager_util for componentisation This CL removes some unneeded //chrome dependencies in password manager utilities. The goal is to make those files movable to the password manager component. The move will happen in a separate CL. BUG=486739 Review URL: https://codereview.chromium.org/1141413002 Cr-Commit-Position: refs/heads/master@{#330805} --- chrome/browser/android/password_ui_view_android.cc | 8 ++++++-- .../browser/password_manager/password_manager_util.cc | 4 ++-- chrome/browser/password_manager/password_manager_util.h | 6 ++++-- .../password_manager/save_password_infobar_delegate.cc | 6 +++++- .../password_generation_popup_controller_impl.cc | 8 ++++++-- .../ui/passwords/manage_passwords_bubble_model.cc | 17 +++++++++++++---- .../browser/ui/passwords/password_bubble_experiment.cc | 7 ++----- .../browser/ui/passwords/password_bubble_experiment.h | 8 ++++++-- .../browser/ui/webui/options/browser_options_handler.cc | 3 ++- .../ui/webui/options/password_manager_handler.cc | 6 +++++- 10 files changed, 51 insertions(+), 22 deletions(-) diff --git a/chrome/browser/android/password_ui_view_android.cc b/chrome/browser/android/password_ui_view_android.cc index 5ad0c7c0e27f..7f1609c8fe91 100644 --- a/chrome/browser/android/password_ui_view_android.cc +++ b/chrome/browser/android/password_ui_view_android.cc @@ -10,6 +10,8 @@ #include "base/metrics/field_trial.h" #include "base/prefs/pref_service.h" #include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/passwords/password_bubble_experiment.h" #include "chrome/common/pref_names.h" #include "chrome/common/url_constants.h" @@ -117,8 +119,10 @@ static jboolean ShouldDisplayManageAccountLink( static jboolean ShouldUseSmartLockBranding( JNIEnv* env, jclass) { - return password_bubble_experiment::IsSmartLockBrandingEnabled( - ProfileManager::GetLastUsedProfile()); + const ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile( + ProfileManager::GetLastUsedProfile()); + return password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service); } // static diff --git a/chrome/browser/password_manager/password_manager_util.cc b/chrome/browser/password_manager/password_manager_util.cc index 32d04865a0bb..e1c1c6adafb3 100644 --- a/chrome/browser/password_manager/password_manager_util.cc +++ b/chrome/browser/password_manager/password_manager_util.cc @@ -4,12 +4,12 @@ #include "chrome/browser/password_manager/password_manager_util.h" -#include "chrome/browser/sync/profile_sync_service.h" +#include "components/sync_driver/sync_service.h" namespace password_manager_util { password_manager::PasswordSyncState GetPasswordSyncState( - const ProfileSyncService* sync_service) { + const sync_driver::SyncService* sync_service) { if (sync_service && sync_service->HasSyncSetupCompleted() && sync_service->SyncActive() && sync_service->GetActiveDataTypes().Has(syncer::PASSWORDS)) { diff --git a/chrome/browser/password_manager/password_manager_util.h b/chrome/browser/password_manager/password_manager_util.h index e01ac228f2cb..69c3754ab8f2 100644 --- a/chrome/browser/password_manager/password_manager_util.h +++ b/chrome/browser/password_manager/password_manager_util.h @@ -10,7 +10,9 @@ #include "components/password_manager/core/browser/password_manager_client.h" #include "ui/gfx/native_widget_types.h" -class ProfileSyncService; +namespace sync_driver { +class SyncService; +} namespace password_manager_util { @@ -40,7 +42,7 @@ void GetOsPasswordStatus(const base::Callback& reply); // Reports whether and how passwords are currently synced. In particular, for a // null |sync_service| returns NOT_SYNCING_PASSWORDS. password_manager::PasswordSyncState GetPasswordSyncState( - const ProfileSyncService* sync_service); + const sync_driver::SyncService* sync_service); } // namespace password_manager_util diff --git a/chrome/browser/password_manager/save_password_infobar_delegate.cc b/chrome/browser/password_manager/save_password_infobar_delegate.cc index 299b246f8289..9e020a1289d4 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate.cc @@ -7,6 +7,8 @@ #include "base/metrics/histogram.h" #include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/passwords/password_bubble_experiment.h" #include "chrome/grit/chromium_strings.h" #include "chrome/grit/generated_resources.h" @@ -41,10 +43,12 @@ void SavePasswordInfoBarDelegate::Create( InfoBarService::FromWebContents(web_contents); Profile* profile = Profile::FromBrowserContext(web_contents->GetBrowserContext()); + const ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile(profile); SavePasswordInfoBarDelegate* infobar_delegate = new SavePasswordInfoBarDelegate( form_to_save.Pass(), uma_histogram_suffix, source_type, - password_bubble_experiment::IsSmartLockBrandingEnabled(profile)); + password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service)); #if defined(OS_ANDROID) // For Android in case of smart lock we need different appearance of infobar. scoped_ptr infobar = diff --git a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc index 5ee343e6c654..9691aeaf4501 100644 --- a/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc +++ b/chrome/browser/ui/autofill/password_generation_popup_controller_impl.cc @@ -12,6 +12,8 @@ #include "base/strings/utf_string_conversion_utils.h" #include "base/strings/utf_string_conversions.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/autofill/password_generation_popup_observer.h" #include "chrome/browser/ui/autofill/password_generation_popup_view.h" #include "chrome/browser/ui/autofill/popup_constants.h" @@ -91,8 +93,10 @@ PasswordGenerationPopupControllerImpl::PasswordGenerationPopupControllerImpl( int link_id = IDS_MANAGE_PASSWORDS_LINK; int help_text_id = IDS_PASSWORD_GENERATION_PROMPT; - if (password_bubble_experiment::IsSmartLockBrandingEnabled( - Profile::FromBrowserContext(web_contents->GetBrowserContext()))) { + const ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile( + Profile::FromBrowserContext(web_contents->GetBrowserContext())); + if (password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service)) { help_text_id = IDS_PASSWORD_GENERATION_SMART_LOCK_PROMPT; link_id = IDS_PASSWORD_MANAGER_SMART_LOCK_FOR_PASSWORDS; } diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc index 2b832256440a..1495ffce4a5a 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc @@ -9,6 +9,8 @@ #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/signin_manager_factory.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h" @@ -72,6 +74,14 @@ ScopedVector DeepCopyForms( return result.Pass(); } +// A wrapper around password_bubble_experiment::IsSmartLockBrandingEnabled +// extracting the sync_service from the profile. +bool IsSmartLockBrandingEnabled(Profile* profile) { + const ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile(profile); + return password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service); +} + } // namespace ManagePasswordsBubbleModel::ManagePasswordsBubbleModel( @@ -121,7 +131,7 @@ ManagePasswordsBubbleModel::ManagePasswordsBubbleModel( base::string16 save_confirmation_link = l10n_util::GetStringUTF16(IDS_MANAGE_PASSWORDS_LINK); int confirmation_text_id = IDS_MANAGE_PASSWORDS_CONFIRM_GENERATED_TEXT; - if (password_bubble_experiment::IsSmartLockBrandingEnabled(GetProfile())) { + if (IsSmartLockBrandingEnabled(GetProfile())) { std::string management_hostname = GURL(chrome::kPasswordManagerAccountDashboardURL).host(); save_confirmation_link = base::UTF8ToUTF16(management_hostname); @@ -249,7 +259,7 @@ void ManagePasswordsBubbleModel::OnOKClicked() { void ManagePasswordsBubbleModel::OnManageLinkClicked() { dismissal_reason_ = metrics_util::CLICKED_MANAGE; - if (password_bubble_experiment::IsSmartLockBrandingEnabled(GetProfile())) { + if (IsSmartLockBrandingEnabled(GetProfile())) { ManagePasswordsUIController::FromWebContents(web_contents()) ->NavigateToExternalPasswordManager(); } else { @@ -328,8 +338,7 @@ void ManagePasswordsBubbleModel::UpdatePendingStateTitle() { if (never_save_passwords_) { title_ = l10n_util::GetStringUTF16( IDS_MANAGE_PASSWORDS_BLACKLIST_CONFIRMATION_TITLE); - } else if (password_bubble_experiment::IsSmartLockBrandingEnabled( - GetProfile())) { + } else if (IsSmartLockBrandingEnabled(GetProfile())) { // "Google Smart Lock" should be a hyperlink. base::string16 brand_link = l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SMART_LOCK); diff --git a/chrome/browser/ui/passwords/password_bubble_experiment.cc b/chrome/browser/ui/passwords/password_bubble_experiment.cc index aca8198c6e8c..50ec27f2c10c 100644 --- a/chrome/browser/ui/passwords/password_bubble_experiment.cc +++ b/chrome/browser/ui/passwords/password_bubble_experiment.cc @@ -5,9 +5,8 @@ #include "chrome/browser/ui/passwords/password_bubble_experiment.h" #include "base/metrics/field_trial.h" +#include "base/prefs/pref_service.h" #include "chrome/browser/password_manager/password_manager_util.h" -#include "chrome/browser/signin/signin_manager_factory.h" -#include "chrome/browser/sync/profile_sync_service_factory.h" namespace password_bubble_experiment { namespace { @@ -23,9 +22,7 @@ void RecordBubbleClosed( // TODO(vasilii): store the statistics. } -bool IsSmartLockBrandingEnabled(Profile* profile) { - const ProfileSyncService* sync_service = - ProfileSyncServiceFactory::GetForProfile(profile); +bool IsSmartLockBrandingEnabled(const sync_driver::SyncService* sync_service) { return password_manager_util::GetPasswordSyncState(sync_service) == password_manager::SYNCING_NORMAL_ENCRYPTION && base::FieldTrialList::FindFullName(kBrandingExperimentName) == diff --git a/chrome/browser/ui/passwords/password_bubble_experiment.h b/chrome/browser/ui/passwords/password_bubble_experiment.h index f20ac7f5e32c..5479c6ac290d 100644 --- a/chrome/browser/ui/passwords/password_bubble_experiment.h +++ b/chrome/browser/ui/passwords/password_bubble_experiment.h @@ -8,7 +8,11 @@ #include "base/macros.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" -class Profile; +class PrefService; + +namespace sync_driver { +class SyncService; +} namespace password_bubble_experiment { @@ -20,7 +24,7 @@ void RecordBubbleClosed( // Returns true if the password manager should be referred to as Smart Lock. // This is only true for signed-in users. -bool IsSmartLockBrandingEnabled(Profile* profile); +bool IsSmartLockBrandingEnabled(const sync_driver::SyncService* sync_service); } // namespace password_bubble_experiment diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc index ce012680538b..e83872e1a913 100644 --- a/chrome/browser/ui/webui/options/browser_options_handler.cc +++ b/chrome/browser/ui/webui/options/browser_options_handler.cc @@ -310,7 +310,8 @@ void BrowserOptionsHandler::GetLocalizedValues(base::DictionaryValue* values) { IDS_NETWORK_PREDICTION_ENABLED_DESCRIPTION }, { "passwordManagerEnabled", password_bubble_experiment::IsSmartLockBrandingEnabled( - Profile::FromWebUI(web_ui())) ? + ProfileSyncServiceFactory::GetForProfile( + Profile::FromWebUI(web_ui()))) ? IDS_OPTIONS_PASSWORD_MANAGER_SMART_LOCK_ENABLE : IDS_OPTIONS_PASSWORD_MANAGER_ENABLE }, { "passwordsAndAutofillGroupName", diff --git a/chrome/browser/ui/webui/options/password_manager_handler.cc b/chrome/browser/ui/webui/options/password_manager_handler.cc index de7041a6219e..24d2e4479e82 100644 --- a/chrome/browser/ui/webui/options/password_manager_handler.cc +++ b/chrome/browser/ui/webui/options/password_manager_handler.cc @@ -13,6 +13,8 @@ #include "base/values.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/profiles/profile.h" +#include "chrome/browser/sync/profile_sync_service.h" +#include "chrome/browser/sync/profile_sync_service_factory.h" #if defined(OS_WIN) && defined(USE_ASH) #include "chrome/browser/ui/ash/ash_util.h" #endif @@ -76,8 +78,10 @@ void PasswordManagerHandler::GetLocalizedValues( RegisterStrings(localized_strings, resources, arraysize(resources)); + const ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile(GetProfile()); int title_id = - password_bubble_experiment::IsSmartLockBrandingEnabled(GetProfile()) ? + password_bubble_experiment::IsSmartLockBrandingEnabled(sync_service) ? IDS_PASSWORDS_EXCEPTIONS_SMART_LOCK_WINDOW_TITLE : IDS_PASSWORDS_EXCEPTIONS_WINDOW_TITLE; RegisterTitle(localized_strings, "passwordsPage", title_id); -- 2.11.4.GIT