diff options
author | gcasto@chromium.org <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-15 22:26:05 +0000 |
---|---|---|
committer | gcasto@chromium.org <gcasto@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-15 22:27:23 +0000 |
commit | e0808f7db517578fb4781d9a594ac82642e90b25 (patch) | |
tree | 97cb93198ba934a20e16619f5dee838d5059c430 | |
parent | b2bdc1038cb427cccc546716aac8731b41b0765d (diff) | |
download | chromium_src-e0808f7db517578fb4781d9a594ac82642e90b25.zip chromium_src-e0808f7db517578fb4781d9a594ac82642e90b25.tar.gz chromium_src-e0808f7db517578fb4781d9a594ac82642e90b25.tar.bz2 |
[Password Manager] Setup experiment to restrict autofilling of sync credential
By default there is no change in behavior, but autofilling can now be disabled
for the sync credential entirely or just disabled for reauth pages that support
transactional reauth.
Note that this also changes GetSyncUsername() to not return the username if
password sync is disabled if it is possible to determine. This makes GetSyncUsername() a little inconsistent depending on the state of sync setup, but it's important to be as specific as possible when disabling autofilling, since it's a
usability hit.
BUG=386692
R=isherman@chromium.org
Review URL: https://codereview.chromium.org/451853003
Cr-Commit-Position: refs/heads/master@{#290030}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290030 0039d316-1c4b-4281-b951-d872f2087c98
16 files changed, 297 insertions, 11 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 0730e6d..2a4b8ee 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -6121,6 +6121,12 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_FLAGS_PASSWORD_MANAGER_ANDROID_LINK_DESCRIPTION" desc="Description of the flag to enable showing a link to account central on Android password settings page"> Show a link in the password manager settings page to manage your synced passwords online. </message> + <message name="IDS_FLAGS_AUTOFILL_SYNC_CREDENTIAL_NAME" desc="Name of the flag specifying how the browser will handle autofilling the users sync credential."> + Autofill sync credential + </message> + <message name="IDS_FLAGS_AUTOFILL_SYNC_CREDENTIAL_DESCRIPTION" desc="Description of the flag specifying how the browser will handle autofilling the users sync credential."> + How the password manager handles autofill for the sync credential. + </message> <message name="IDS_FLAGS_PERFORMANCE_MONITOR_GATHERING_NAME" desc="Name for the flag to enable Performance Monitor."> Enable performance monitoring </message> @@ -12930,6 +12936,16 @@ Some features may be unavailable. Please check that the profile exists and you Undo </message> + <message name="IDS_ALLOW_AUTOFILL_SYNC_CREDENTIAL" desc="The text for the choice to allow autofilling in about:flags"> + Allow + </message> + <message name="IDS_DISALLOW_AUTOFILL_SYNC_CREDENTIAL" desc="The text for the choice to not allow autofilling in about:flags"> + Disallow + </message> + <message name="IDS_DISALLOW_AUTOFILL_SYNC_CREDENTIAL_FOR_REAUTH" desc="The text for the choice to not allow autofilling for reauth pages in about:flags"> + Disallow for reauth + </message> + <!-- Update Recommended dialog --> <if expr="not chromeos"> <message name="IDS_UPDATE_RECOMMENDED" desc="The main text of the Update Recommended dialog."> diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index fa08f1d2..2a87d86 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -503,6 +503,16 @@ const Experiment::Choice kEnableAVFoundationChoices[] = { }; #endif +const Experiment::Choice kAutofillSyncCredentialChoices[] = { + { IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT, "", ""}, + { IDS_ALLOW_AUTOFILL_SYNC_CREDENTIAL, + password_manager::switches::kAllowAutofillSyncCredential, ""}, + { IDS_DISALLOW_AUTOFILL_SYNC_CREDENTIAL_FOR_REAUTH, + password_manager::switches::kDisallowAutofillSyncCredentialForReauth, ""}, + { IDS_DISALLOW_AUTOFILL_SYNC_CREDENTIAL, + password_manager::switches::kDisallowAutofillSyncCredential, ""}, +}; + // RECORDING USER METRICS FOR FLAGS: // ----------------------------------------------------------------------------- // The first line of the experiment is the internal name. If you'd like to @@ -1939,6 +1949,13 @@ const Experiment kExperiments[] = { kOsWin | kOsLinux | kOsCrOS, SINGLE_VALUE_TYPE(extensions::switches::kEnableExtensionActionRedesign) }, + { + "autofill-sync-credential", + IDS_FLAGS_AUTOFILL_SYNC_CREDENTIAL_NAME, + IDS_FLAGS_AUTOFILL_SYNC_CREDENTIAL_DESCRIPTION, + kOsAll, + MULTI_VALUE_TYPE(kAutofillSyncCredentialChoices) + }, // NOTE: Adding new command-line switches requires adding corresponding // entries to enum "LoginCustomFlags" in histograms.xml. See note in // histograms.xml and don't forget to run AboutFlagsHistogramTest unit test. diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index df029ae..c5c50bf 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc @@ -8,6 +8,7 @@ #include "base/command_line.h" #include "base/memory/singleton.h" #include "base/metrics/histogram.h" +#include "base/strings/utf_string_conversions.h" #include "chrome/browser/password_manager/password_manager_util.h" #include "chrome/browser/password_manager/password_store_factory.h" #include "chrome/browser/password_manager/save_password_infobar_delegate.h" @@ -33,6 +34,8 @@ #include "content/public/browser/navigation_entry.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" +#include "google_apis/gaia/gaia_urls.h" +#include "net/base/url_util.h" #if defined(OS_ANDROID) #include "chrome/browser/android/password_authentication_manager.h" @@ -63,11 +66,14 @@ ChromePasswordManagerClient::ChromePasswordManagerClient( driver_(web_contents, this, autofill_client), observer_(NULL), weak_factory_(this), - can_use_log_router_(false) { + can_use_log_router_(false), + autofill_sync_state_(ALLOW_SYNC_CREDENTIALS), + sync_credential_was_filtered_(false) { PasswordManagerInternalsService* service = PasswordManagerInternalsServiceFactory::GetForBrowserContext(profile_); if (service) can_use_log_router_ = service->RegisterClient(this); + SetUpAutofillSyncState(); } ChromePasswordManagerClient::~ChromePasswordManagerClient() { @@ -79,7 +85,7 @@ ChromePasswordManagerClient::~ChromePasswordManagerClient() { bool ChromePasswordManagerClient::IsAutomaticPasswordSavingEnabled() const { return CommandLine::ForCurrentProcess()->HasSwitch( - password_manager::switches::kEnableAutomaticPasswordSaving) && + password_manager::switches::kEnableAutomaticPasswordSaving) && chrome::VersionInfo::GetChannel() == chrome::VersionInfo::CHANNEL_UNKNOWN; } @@ -102,12 +108,38 @@ bool ChromePasswordManagerClient::IsPasswordManagerEnabledForCurrentPage() return entry->GetURL().host() != chrome::kChromeUIChromeSigninHost; } +bool ChromePasswordManagerClient::ShouldFilterAutofillResult( + const autofill::PasswordForm& form) { + if (!IsSyncAccountCredential(base::UTF16ToUTF8(form.username_value), + form.signon_realm)) + return false; + + if (autofill_sync_state_ == DISALLOW_SYNC_CREDENTIALS) { + sync_credential_was_filtered_ = true; + return true; + } + + if (autofill_sync_state_ == DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH && + LastLoadWasTransactionalReauthPage()) { + sync_credential_was_filtered_ = true; + return true; + } + + return false; +} + bool ChromePasswordManagerClient::IsSyncAccountCredential( const std::string& username, const std::string& origin) const { return password_manager_sync_metrics::IsSyncAccountCredential( profile_, username, origin); } +void ChromePasswordManagerClient::AutofillResultsComputed() { + UMA_HISTOGRAM_BOOLEAN("PasswordManager.SyncCredentialFiltered", + sync_credential_was_filtered_); + sync_credential_was_filtered_ = false; +} + void ChromePasswordManagerClient::PromptUserToSavePassword( scoped_ptr<password_manager::PasswordFormManager> form_to_save) { if (IsTheHotNewBubbleUIEnabled()) { @@ -353,6 +385,24 @@ void ChromePasswordManagerClient::CommitFillPasswordForm( driver_.FillPasswordForm(*data); } +bool ChromePasswordManagerClient::LastLoadWasTransactionalReauthPage() const { + DCHECK(web_contents()); + content::NavigationEntry* entry = + web_contents()->GetController().GetLastCommittedEntry(); + if (!entry) + return false; + + if (entry->GetURL().GetOrigin() != + GaiaUrls::GetInstance()->gaia_url().GetOrigin()) + return false; + + // "rart" is the transactional reauth paramter. + std::string ignored_value; + return net::GetValueForKeyInQuery(entry->GetURL(), + "rart", + &ignored_value); +} + bool ChromePasswordManagerClient::IsTheHotNewBubbleUIEnabled() { #if !defined(USE_AURA) return false; @@ -386,3 +436,35 @@ bool ChromePasswordManagerClient::EnabledForSyncSignin() { base::FieldTrialList::FindFullName("PasswordManagerStateForSyncSignin"); return group_name != "Disabled"; } + +void ChromePasswordManagerClient::SetUpAutofillSyncState() { + std::string group_name = + base::FieldTrialList::FindFullName("AutofillSyncCredential"); + + CommandLine* command_line = CommandLine::ForCurrentProcess(); + if (command_line->HasSwitch( + password_manager::switches::kAllowAutofillSyncCredential)) { + autofill_sync_state_ = ALLOW_SYNC_CREDENTIALS; + return; + } + if (command_line->HasSwitch( + password_manager::switches:: + kDisallowAutofillSyncCredentialForReauth)) { + autofill_sync_state_ = DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH; + return; + } + if (command_line->HasSwitch( + password_manager::switches::kDisallowAutofillSyncCredential)) { + autofill_sync_state_ = DISALLOW_SYNC_CREDENTIALS; + return; + } + + if (group_name == "DisallowSyncCredentialsForReauth") { + autofill_sync_state_ = DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH; + } else if (group_name == "DisallowSyncCredentials") { + autofill_sync_state_ = DISALLOW_SYNC_CREDENTIALS; + } else { + // Allow by default. + autofill_sync_state_ = ALLOW_SYNC_CREDENTIALS; + } +} diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index 203aa42..4fbed64 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h @@ -40,8 +40,11 @@ class ChromePasswordManagerClient // PasswordManagerClient implementation. virtual bool IsAutomaticPasswordSavingEnabled() const OVERRIDE; virtual bool IsPasswordManagerEnabledForCurrentPage() const OVERRIDE; + virtual bool ShouldFilterAutofillResult( + const autofill::PasswordForm& form) OVERRIDE; virtual bool IsSyncAccountCredential( const std::string& username, const std::string& origin) const OVERRIDE; + virtual void AutofillResultsComputed() OVERRIDE; virtual void PromptUserToSavePassword( scoped_ptr<password_manager::PasswordFormManager> form_to_save) OVERRIDE; virtual void AutomaticPasswordSave( @@ -90,9 +93,18 @@ class ChromePasswordManagerClient // Returns true if the password manager should be enabled during sync signin. static bool EnabledForSyncSignin(); - private: + protected: + // Callable for tests. ChromePasswordManagerClient(content::WebContents* web_contents, autofill::AutofillClient* autofill_client); + + private: + enum AutofillForSyncCredentialsState { + ALLOW_SYNC_CREDENTIALS, + DISALLOW_SYNC_CREDENTIALS_FOR_REAUTH, + DISALLOW_SYNC_CREDENTIALS, + }; + friend class content::WebContentsUserData<ChromePasswordManagerClient>; // content::WebContentsObserver overrides. @@ -122,6 +134,13 @@ class ChromePasswordManagerClient // |can_use_log_router_|. void NotifyRendererOfLoggingAvailability(); + // Returns true if the last loaded page was for transactional re-auth on a + // Google property. + bool LastLoadWasTransactionalReauthPage() const; + + // Sets |autofill_state_| based on experiment and flag values. + void SetUpAutofillSyncState(); + Profile* const profile_; password_manager::ContentPasswordManagerDriver driver_; @@ -139,6 +158,13 @@ class ChromePasswordManagerClient // True if |this| is registered with some LogRouter which can accept logs. bool can_use_log_router_; + // How to handle the sync credential in ShouldFilterAutofillResult(). + AutofillForSyncCredentialsState autofill_sync_state_; + + // If the sync credential was filtered during autofill. Used for statistics + // reporting. + bool sync_credential_was_filtered_; + DISALLOW_COPY_AND_ASSIGN(ChromePasswordManagerClient); }; diff --git a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc index 100a3e6..802bdce 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc @@ -32,6 +32,29 @@ class MockLogReceiver : public password_manager::LogReceiver { MOCK_METHOD1(LogSavePasswordProgress, void(const std::string&)); }; +class TestChromePasswordManagerClient : public ChromePasswordManagerClient { + public: + explicit TestChromePasswordManagerClient(content::WebContents* web_contents) + : ChromePasswordManagerClient(web_contents, NULL), + is_sync_account_credential_(false) {} + virtual ~TestChromePasswordManagerClient() {} + + virtual bool IsSyncAccountCredential( + const std::string& username, + const std::string& origin) const OVERRIDE { + return is_sync_account_credential_; + } + + void set_is_sync_account_credential(bool is_sync_account_credential) { + is_sync_account_credential_ = is_sync_account_credential; + } + + private: + bool is_sync_account_credential_; + + DISALLOW_COPY_AND_ASSIGN(TestChromePasswordManagerClient); +}; + } // namespace class ChromePasswordManagerClientTest : public ChromeRenderViewHostTestHarness { @@ -192,3 +215,54 @@ TEST_F(ChromePasswordManagerClientTest, LogToAReceiver) { service_->UnregisterReceiver(&receiver_); EXPECT_FALSE(client->IsLoggingActive()); } + +TEST_F(ChromePasswordManagerClientTest, ShouldFilterAutofillResult_Reauth) { + // Make client disallow only reauth requests. + CommandLine* command_line = CommandLine::ForCurrentProcess(); + command_line->AppendSwitch( + password_manager::switches::kDisallowAutofillSyncCredentialForReauth); + scoped_ptr<TestChromePasswordManagerClient> client( + new TestChromePasswordManagerClient(web_contents())); + autofill::PasswordForm form; + + client->set_is_sync_account_credential(false); + NavigateAndCommit( + GURL("https://accounts.google.com/login?rart=123&continue=blah")); + EXPECT_FALSE(client->ShouldFilterAutofillResult(form)); + + client->set_is_sync_account_credential(true); + NavigateAndCommit( + GURL("https://accounts.google.com/login?rart=123&continue=blah")); + EXPECT_TRUE(client->ShouldFilterAutofillResult(form)); + + // This counts as a reauth url, though a valid URL should have a value for + // "rart" + NavigateAndCommit(GURL("https://accounts.google.com/addlogin?rart")); + EXPECT_TRUE(client->ShouldFilterAutofillResult(form)); + + NavigateAndCommit(GURL("https://accounts.google.com/login?param=123")); + EXPECT_FALSE(client->ShouldFilterAutofillResult(form)); + + NavigateAndCommit(GURL("https://site.com/login?rart=678")); + EXPECT_FALSE(client->ShouldFilterAutofillResult(form)); +} + +TEST_F(ChromePasswordManagerClientTest, ShouldFilterAutofillResult) { + // Normally the client should allow any credentials through, even if they + // are the sync credential. + scoped_ptr<TestChromePasswordManagerClient> client( + new TestChromePasswordManagerClient(web_contents())); + autofill::PasswordForm form; + client->set_is_sync_account_credential(true); + NavigateAndCommit(GURL("https://accounts.google.com/Login")); + EXPECT_FALSE(client->ShouldFilterAutofillResult(form)); + + // Adding disallow switch should cause sync credential to be filtered. + CommandLine* command_line = CommandLine::ForCurrentProcess(); + command_line->AppendSwitch( + password_manager::switches::kDisallowAutofillSyncCredential); + client.reset(new TestChromePasswordManagerClient(web_contents())); + client->set_is_sync_account_credential(true); + NavigateAndCommit(GURL("https://accounts.google.com/Login")); + EXPECT_TRUE(client->ShouldFilterAutofillResult(form)); +} diff --git a/chrome/browser/password_manager/sync_metrics.cc b/chrome/browser/password_manager/sync_metrics.cc index 4eddf42..304bb6d 100644 --- a/chrome/browser/password_manager/sync_metrics.cc +++ b/chrome/browser/password_manager/sync_metrics.cc @@ -14,10 +14,19 @@ namespace password_manager_sync_metrics { std::string GetSyncUsername(Profile* profile) { + // If sync is set up, return early if we aren't syncing passwords. + if (ProfileSyncServiceFactory::HasProfileSyncService(profile)) { + ProfileSyncService* sync_service = + ProfileSyncServiceFactory::GetForProfile(profile); + if (!sync_service->GetPreferredDataTypes().Has(syncer::PASSWORDS)) + return std::string(); + } + SigninManagerBase* signin_manager = SigninManagerFactory::GetForProfile(profile); + if (!signin_manager) - return ""; + return std::string(); return signin_manager->GetAuthenticatedUsername(); } diff --git a/chrome/browser/password_manager/sync_metrics.h b/chrome/browser/password_manager/sync_metrics.h index 2a22b07..74daa01 100644 --- a/chrome/browser/password_manager/sync_metrics.h +++ b/chrome/browser/password_manager/sync_metrics.h @@ -12,9 +12,9 @@ class Profile; namespace password_manager_sync_metrics { // Returns the sync username for |profile|. Returns an empty string if the -// |profile| isn't syncing. It would be preferable to only return the username -// if the user is syncing passwords, but that is not currently possible since -// this function can be called during sync setup (http://crbug.com/393626). +// |profile| isn't syncing. This function tries to return an empty string if +// the user isn't syncing passwords, but it is not always possibly to determine +// since this code can be called during sync setup (http://crbug.com/393626). std::string GetSyncUsername(Profile* profile); // Returns true if |username| and |origin| correspond to the account which is diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index 1248b11..6033da6 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc @@ -379,7 +379,7 @@ void PasswordFormManager::OnRequestDone( // These credentials will be in the final result regardless of score. std::vector<PasswordForm> credentials_to_keep; for (size_t i = 0; i < logins_result.size(); i++) { - if (IgnoreResult(*logins_result[i])) { + if (ShouldIgnoreResult(*logins_result[i])) { delete logins_result[i]; continue; } @@ -439,6 +439,8 @@ void PasswordFormManager::OnRequestDone( // We're done matching now. state_ = POST_MATCHING_PHASE; + client_->AutofillResultsComputed(); + if (best_score <= 0) { return; } @@ -507,7 +509,7 @@ void PasswordFormManager::OnGetPasswordStoreResults( OnRequestDone(results); } -bool PasswordFormManager::IgnoreResult(const PasswordForm& form) const { +bool PasswordFormManager::ShouldIgnoreResult(const PasswordForm& form) const { // Do not autofill on sign-up or change password forms (until we have some // working change password functionality). if (!observed_form_.new_password_element.empty()) @@ -515,6 +517,10 @@ bool PasswordFormManager::IgnoreResult(const PasswordForm& form) const { // Don't match an invalid SSL form with one saved under secure circumstances. if (form.ssl_valid && !observed_form_.ssl_valid) return true; + + if (client_->ShouldFilterAutofillResult(form)) + return true; + return false; } diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index 89c6933..0b69c6c 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h @@ -219,7 +219,7 @@ class PasswordFormManager : public PasswordStoreConsumer { // Helper for OnGetPasswordStoreResults to determine whether or not // the given result form is worth scoring. - bool IgnoreResult(const autofill::PasswordForm& form) const; + bool ShouldIgnoreResult(const autofill::PasswordForm& form) const; // Helper for Save in the case that best_matches.size() == 0, meaning // we have no prior record of this form/username/password and the user diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc index a3399cd..77b7d4c 100644 --- a/components/password_manager/core/browser/password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/password_form_manager_unittest.cc @@ -60,6 +60,13 @@ class TestPasswordManagerClient : public StubPasswordManagerClient { true); } + virtual bool ShouldFilterAutofillResult( + const autofill::PasswordForm& form) OVERRIDE { + if (form == form_to_filter_) + return true; + return false; + } + virtual void PromptUserToSavePassword( scoped_ptr<PasswordFormManager> form_to_save) OVERRIDE {} virtual PrefService* GetPrefs() OVERRIDE { return &prefs_; } @@ -70,9 +77,15 @@ class TestPasswordManagerClient : public StubPasswordManagerClient { driver_.FillPasswordForm(*fill_data.get()); } + void SetFormToFilter(const autofill::PasswordForm& form) { + form_to_filter_ = form; + } + MockPasswordManagerDriver* GetMockDriver() { return &driver_; } private: + autofill::PasswordForm form_to_filter_; + TestingPrefServiceSimple prefs_; PasswordStore* password_store_; MockPasswordManagerDriver driver_; @@ -177,7 +190,7 @@ class PasswordFormManagerTest : public testing::Test { } bool IgnoredResult(PasswordFormManager* p, PasswordForm* form) { - return p->IgnoreResult(*form); + return p->ShouldIgnoreResult(*form); } PasswordForm* observed_form() { return &observed_form_; } @@ -427,6 +440,10 @@ TEST_F(PasswordFormManagerTest, TestIgnoreResult) { saved_match()->action = GURL("http://www.google.com/b/Login"); saved_match()->origin = GURL("http://www.google.com/foo"); EXPECT_FALSE(IgnoredResult(&manager, saved_match())); + + // Results should be ignored if the client requests it. + client()->SetFormToFilter(*saved_match()); + EXPECT_TRUE(IgnoredResult(&manager, saved_match())); } TEST_F(PasswordFormManagerTest, TestEmptyAction) { diff --git a/components/password_manager/core/browser/password_manager_client.h b/components/password_manager/core/browser/password_manager_client.h index 4607800..99f1282 100644 --- a/components/password_manager/core/browser/password_manager_client.h +++ b/components/password_manager/core/browser/password_manager_client.h @@ -35,11 +35,19 @@ class PasswordManagerClient { // always returns true. virtual bool IsPasswordManagerEnabledForCurrentPage() const; + // Return true if |form| should not be available for autofill. + virtual bool ShouldFilterAutofillResult( + const autofill::PasswordForm& form) = 0; + // Returns true if |username| and |origin| correspond to the account which is // syncing. virtual bool IsSyncAccountCredential( const std::string& username, const std::string& origin) const = 0; + // Called when all autofill results have been computed. Client can use + // this signal to report statistics. Default implementation is a noop. + virtual void AutofillResultsComputed() {} + // Informs the embedder of a password form that can be saved if the user // allows it. The embedder is not required to prompt the user if it decides // that this form doesn't need to be saved. diff --git a/components/password_manager/core/browser/stub_password_manager_client.cc b/components/password_manager/core/browser/stub_password_manager_client.cc index ecaf219..ca4fa29 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.cc +++ b/components/password_manager/core/browser/stub_password_manager_client.cc @@ -17,6 +17,11 @@ bool StubPasswordManagerClient::IsSyncAccountCredential( return false; } +bool StubPasswordManagerClient::ShouldFilterAutofillResult( + const autofill::PasswordForm& form) { + return false; +} + void StubPasswordManagerClient::PromptUserToSavePassword( scoped_ptr<PasswordFormManager> form_to_save) {} diff --git a/components/password_manager/core/browser/stub_password_manager_client.h b/components/password_manager/core/browser/stub_password_manager_client.h index deac6ce..df98851 100644 --- a/components/password_manager/core/browser/stub_password_manager_client.h +++ b/components/password_manager/core/browser/stub_password_manager_client.h @@ -20,6 +20,8 @@ class StubPasswordManagerClient : public PasswordManagerClient { // PasswordManagerClient: virtual bool IsSyncAccountCredential( const std::string& username, const std::string& origin) const OVERRIDE; + virtual bool ShouldFilterAutofillResult( + const autofill::PasswordForm& form) OVERRIDE; virtual void PromptUserToSavePassword( scoped_ptr<PasswordFormManager> form_to_save) OVERRIDE; virtual void AutomaticPasswordSave( diff --git a/components/password_manager/core/common/password_manager_switches.cc b/components/password_manager/core/common/password_manager_switches.cc index 8ceab0f2..846d599 100644 --- a/components/password_manager/core/common/password_manager_switches.cc +++ b/components/password_manager/core/common/password_manager_switches.cc @@ -8,6 +8,10 @@ namespace password_manager { namespace switches { +// Force the password manager to allow sync credentials to be autofilled. +const char kAllowAutofillSyncCredential[] = + "allow-autofill-sync-credential"; + // Disable the link in the password manager settings page that points to account // central. const char kDisableAndroidPasswordLink[] = "disable-android-password-link"; @@ -20,6 +24,15 @@ const char kDisableDropSyncCredential[] = const char kDisableManagerForSyncSignin[] = "disable-manager-for-sync-signin"; +// Disallow autofilling of the sync credential. +const char kDisallowAutofillSyncCredential[] = + "disallow-autofill-sync-credential"; + +// Disallow autofilling of the sync credential only for transactional reauth +// pages. +const char kDisallowAutofillSyncCredentialForReauth[] = + "disallow-autofill-sync-credential-for-reauth"; + // Enable the link in the password manager settings page that points to account // central. const char kEnableAndroidPasswordLink[] = "enable-android-password-link"; diff --git a/components/password_manager/core/common/password_manager_switches.h b/components/password_manager/core/common/password_manager_switches.h index 995c5cc..9377375 100644 --- a/components/password_manager/core/common/password_manager_switches.h +++ b/components/password_manager/core/common/password_manager_switches.h @@ -12,9 +12,12 @@ namespace switches { // All switches in alphabetical order. The switches should be documented // alongside the definition of their values in the .cc file. +extern const char kAllowAutofillSyncCredential[]; extern const char kDisableAndroidPasswordLink[]; extern const char kDisableDropSyncCredential[]; extern const char kDisableManagerForSyncSignin[]; +extern const char kDisallowAutofillSyncCredential[]; +extern const char kDisallowAutofillSyncCredentialForReauth[]; extern const char kEnableAndroidPasswordLink[]; extern const char kEnableDropSyncCredential[]; extern const char kEnableManagerForSyncSignin[]; diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 85fcfe1..57bc5c8 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -20592,6 +20592,14 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="PasswordManager.SyncCredentialFiltered" enum="Boolean"> + <owner>gcasto@chromium.org</owner> + <owner>vabr@chromium.org</owner> + <summary> + If the sync credential was removed from autofill consideration. + </summary> +</histogram> + <histogram name="PasswordManager.SyncingAccountState" enum="PasswordManagerSyncingAccountState"> <owner>gcasto@chromium.org</owner> |