Skip to content

Commit

Permalink
[Password Manager] Add experiment controlling sync credential saving …
Browse files Browse the repository at this point in the history
…behavior

I also updated ProvisionalSaveFailure in histograms.xml, which was an oversight from a previous cl.

BUG=386402

Review URL: https://codereview.chromium.org/421563003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288162 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
gcasto@chromium.org committed Aug 7, 2014
1 parent 229d285 commit bdbf5cb
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 1 deletion.
6 changes: 6 additions & 0 deletions chrome/app/generated_resources.grd
Original file line number Diff line number Diff line change
Expand Up @@ -6192,6 +6192,12 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_FLAGS_DEBUG_PACKED_APP_DESCRIPTION" desc="Description of the flag to enable debugging context menu options for packed apps.">
Enables debugging context menu options such as Inspect Element for packed applications.
</message>
<message name="IDS_FLAGS_ENABLE_DROP_SYNC_CREDENTIAL_NAME" desc="Name of the flag to enable drop sync credential">
Drop sync credentials from password manager.
</message>
<message name="IDS_FLAGS_ENABLE_DROP_SYNC_CREDENTIAL_DESCRIPTION" desc="Description of the flag to enable drop sync credential">
If enabled, the password manager will not offer to save the credential used to sync.
</message>
<message name="IDS_FLAGS_ENABLE_PASSWORD_GENERATION_NAME" desc="Name of the flag to enable password generation.">
Enable password generation.
</message>
Expand Down
15 changes: 15 additions & 0 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,14 @@ const Experiment::Choice kRememberCertificateErrorDecisionsChoices[] = {
"7776000" },
};

const Experiment::Choice kEnableDropSyncCredentialChoices[] = {
{ IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT, "", ""},
{ IDS_GENERIC_EXPERIMENT_CHOICE_ENABLED,
password_manager::switches::kEnableDropSyncCredential, "" },
{ IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED,
password_manager::switches::kDisableDropSyncCredential, "" },
};

// RECORDING USER METRICS FOR FLAGS:
// -----------------------------------------------------------------------------
// The first line of the experiment is the internal name. If you'd like to
Expand Down Expand Up @@ -1924,6 +1932,13 @@ const Experiment kExperiments[] = {
kOsAll,
MULTI_VALUE_TYPE(kRememberCertificateErrorDecisionsChoices)
},
{
"enable-drop-sync-credential",
IDS_FLAGS_ENABLE_DROP_SYNC_CREDENTIAL_NAME,
IDS_FLAGS_ENABLE_DROP_SYNC_CREDENTIAL_DESCRIPTION,
kOsAll,
MULTI_VALUE_TYPE(kEnableDropSyncCredentialChoices)
}
};

const Experiment* experiments = kExperiments;
Expand Down
19 changes: 18 additions & 1 deletion components/password_manager/core/browser/password_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/password_manager/core/common/password_manager_pref_names.h"
#include "components/password_manager/core/common/password_manager_switches.h"
#include "components/pref_registry/pref_registry_syncable.h"

using autofill::PasswordForm;
Expand Down Expand Up @@ -53,6 +54,21 @@ void ReportMetrics(bool password_manager_enabled) {
UMA_HISTOGRAM_BOOLEAN("PasswordManager.Enabled", password_manager_enabled);
}

bool ShouldDropSyncCredential() {
std::string group_name =
base::FieldTrialList::FindFullName("PasswordManagerDropSyncCredential");

CommandLine* command_line = CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(switches::kEnableDropSyncCredential))
return true;

if (command_line->HasSwitch(switches::kDisableDropSyncCredential))
return false;

// Default to not saving.
return group_name != "Disabled";
}

} // namespace

const char PasswordManager::kOtherPossibleUsernamesExperiment[] =
Expand Down Expand Up @@ -218,7 +234,8 @@ void PasswordManager::ProvisionallySavePassword(const PasswordForm& form) {

// Don't save credentials for the syncing account. See crbug.com/365832 for
// background.
if (client_->IsSyncAccountCredential(
if (ShouldDropSyncCredential() &&
client_->IsSyncAccountCredential(
base::UTF16ToUTF8(form.username_value), form.signon_realm)) {
RecordFailure(SYNC_CREDENTIAL, form.origin.host(), logger.get());
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,18 @@ namespace password_manager {

namespace switches {

// Disable dropping the credential used to sync passwords.
const char kDisableDropSyncCredential[] =
"disable-drop-sync-credential";

// Disable both saving and filling for the sync signin form.
const char kDisableManagerForSyncSignin[] =
"disable-manager-for-sync-signin";

// Enable dropping the credential used to sync passwords.
const char kEnableDropSyncCredential[] =
"enable-drop-sync-credential";

// Enable saving and filling for the sync signin form. Currently the default
// behavior.
const char kEnableManagerForSyncSignin[] =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ 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 kDisableDropSyncCredential[];
extern const char kDisableManagerForSyncSignin[];
extern const char kEnableDropSyncCredential[];
extern const char kEnableManagerForSyncSignin[];
extern const char kEnableAutomaticPasswordSaving[];

Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46625,6 +46625,7 @@ Therefore, the affected-histogram name has to have at least one dot in it.
<int value="4" label="FORM_BLACKLISTED"/>
<int value="5" label="INVALID_FORM"/>
<int value="6" label="AUTOCOMPLETE_OFF"/>
<int value="7" label="SYNC_CREDENTIALS"/>
</enum>

<enum name="ProxyStatus" type="int">
Expand Down

0 comments on commit bdbf5cb

Please sign in to comment.