Commit 4eb4de41 authored by Marc Treib's avatar Marc Treib Committed by Chromium LUCI CQ

Make default password store configurable via a feature param

This adds a new feature param (and corresponding about:flags entry)
which, when set to true, makes the profile store the default one for
new users.

Bug: 1160655
Change-Id: I549b624fdc4cf3f01fdfb0425cc12299d58ed9aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599529
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842060}
parent 9f838913
......@@ -2497,6 +2497,16 @@ constexpr FeatureEntry::FeatureVariation
kPlatformProvidedTrustTokenIssuance,
base::size(kPlatformProvidedTrustTokenIssuance), nullptr}};
const FeatureEntry::FeatureParam kPasswordsAccountStorage_ProfileStore[] = {
{password_manager::features::kSaveToProfileStoreByDefault, "true"},
};
const FeatureEntry::FeatureVariation kPasswordsAccountStorageVariations[] = {
{"(save to profile store by default)",
kPasswordsAccountStorage_ProfileStore,
base::size(kPasswordsAccountStorage_ProfileStore), nullptr},
};
// RECORDING USER METRICS FOR FLAGS:
// -----------------------------------------------------------------------------
// The first line of the entry is the internal name.
......@@ -5996,8 +6006,10 @@ const FeatureEntry kFeatureEntries[] = {
flag_descriptions::kEnablePasswordsAccountStorageName,
flag_descriptions::kEnablePasswordsAccountStorageDescription,
kOsWin | kOsMac | kOsLinux,
FEATURE_VALUE_TYPE(
password_manager::features::kEnablePasswordsAccountStorage)},
FEATURE_WITH_PARAMS_VALUE_TYPE(
password_manager::features::kEnablePasswordsAccountStorage,
kPasswordsAccountStorageVariations,
"ButterForPasswords")},
#if defined(OS_WIN) || defined(OS_MAC) || defined(OS_LINUX) || \
defined(OS_CHROMEOS)
......
......@@ -327,9 +327,17 @@ PasswordForm::Store GetDefaultPasswordStore(
.GetDefaultStore();
// If none of the early-outs above triggered, then we *can* save to the
// account store in principle (though the user might not have opted in to that
// yet). In this case, default to the account store.
if (default_store == PasswordForm::Store::kNotSet)
return PasswordForm::Store::kAccountStore;
// yet).
if (default_store == PasswordForm::Store::kNotSet) {
// If the user hasn't made a choice about the default store yet, retrieve it
// from a feature param.
bool save_to_profile_store = base::GetFieldTrialParamByFeatureAsBool(
features::kEnablePasswordsAccountStorage,
features::kSaveToProfileStoreByDefault,
features::kSaveToProfileStoreByDefaultDefaultValue);
return save_to_profile_store ? PasswordForm::Store::kProfileStore
: PasswordForm::Store::kAccountStore;
}
return default_store;
}
......
......@@ -258,6 +258,39 @@ TEST(PasswordFeatureManagerUtil, AccountStoragePerAccountSettings) {
PasswordForm::Store::kProfileStore);
}
TEST(PasswordFeatureManagerUtil, SaveToProfileStoreByDefaultParam) {
base::test::ScopedFeatureList features;
features.InitAndEnableFeatureWithParameters(
features::kEnablePasswordsAccountStorage,
{{features::kSaveToProfileStoreByDefault, "true"}});
TestingPrefServiceSimple pref_service;
pref_service.registry()->RegisterDictionaryPref(
prefs::kAccountStoragePerAccountSettings);
CoreAccountInfo account;
account.email = "name@account.com";
account.gaia = "name";
account.account_id = CoreAccountId::FromGaiaId(account.gaia);
// SyncService is running in transport mode.
syncer::TestSyncService sync_service;
sync_service.SetAuthenticatedAccountInfo(account);
sync_service.SetIsAuthenticatedAccountPrimary(false);
sync_service.SetDisableReasons({});
sync_service.SetTransportState(syncer::SyncService::TransportState::ACTIVE);
ASSERT_FALSE(sync_service.IsSyncFeatureEnabled());
// By default, the user is not opted in. Since the
// |kSaveToProfileStoreByDefault| parameter is set, the default store should
// be the *profile* one. The opt-in for the account store should still show
// up, though.
ASSERT_FALSE(IsOptedInForAccountStorage(&pref_service, &sync_service));
EXPECT_TRUE(ShouldShowAccountStorageOptIn(&pref_service, &sync_service));
EXPECT_EQ(GetDefaultPasswordStore(&pref_service, &sync_service),
PasswordForm::Store::kProfileStore);
}
TEST(PasswordFeatureManagerUtil, AccountStorageKeepSettingsOnlyForUsers) {
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(features::kEnablePasswordsAccountStorage);
......
......@@ -156,6 +156,12 @@ const char kMaxMoveToAccountOffersForNonOptedInUser[] =
const int kMaxMoveToAccountOffersForNonOptedInUserDefaultValue = 5;
// If set to true, Chrome will default to saving to the profile store for users
// who haven't made an explicit choice yet.
const char kSaveToProfileStoreByDefault[] = "save_to_profile_store_by_default";
const bool kSaveToProfileStoreByDefaultDefaultValue = false;
} // namespace features
} // namespace password_manager
......@@ -57,9 +57,11 @@ extern const char
kPasswordChangeWithForcedDialogAfterEverySuccessfulSubmission[];
extern const char kPasswordChangeInSettingsWithForcedWarningForEverySite[];
// |kEnablePasswordAccountStorage| variations.
// |kEnablePasswordsAccountStorage| variations.
extern const char kMaxMoveToAccountOffersForNonOptedInUser[];
extern const int kMaxMoveToAccountOffersForNonOptedInUserDefaultValue;
extern const char kSaveToProfileStoreByDefault[];
extern const bool kSaveToProfileStoreByDefaultDefaultValue;
} // namespace features
......
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