[passwords] Clean up UPM migration warning flag in activation algorithm
kUnifiedPasswordManagerLocalPasswordsMigrationWarning was *disabled* by
default in M132 (https://crrev.com/c/5928711, http://cl/686529557). It
can be cleaned up now.
Bug: 413696987
Change-Id: Ibbdbf67a959b2061f1490ea79e986909de4fba45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6490857
Commit-Queue: Victor Vianna <[email protected]>
Reviewed-by: Maria Kazinova <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1452469}
diff --git a/chrome/browser/password_manager/android/password_manager_android_util.cc b/chrome/browser/password_manager/android/password_manager_android_util.cc
index b72aaa60..123ca0c7 100644
--- a/chrome/browser/password_manager/android/password_manager_android_util.cc
+++ b/chrome/browser/password_manager/android/password_manager_android_util.cc
@@ -59,8 +59,8 @@
// (Deprecated) kLoginDbFileMoveFailed = 3,
kOutdatedGmsCore = 4,
// (Deprecated) kFlagDisabled = 5,
- kMigrationWarningUnacknowledged = 6,
- kMaxValue = kMigrationWarningUnacknowledged,
+ // (Deprecated) kMigrationWarningUnacknowledged = 6,
+ kMaxValue = kOutdatedGmsCore,
};
// Set on startup before the local passwords migration starts.
@@ -95,34 +95,6 @@
}
}
-bool ShouldDelayMigrationUntillMigrationWarningIsAcknowledged(
- PrefService* pref_service) {
- // The migration warning is only relevant for non-stable channels.
- version_info::Channel channel = version_info::android::GetChannel();
- if (channel == version_info::Channel::STABLE) {
- return false;
- }
- // If there are no passwords to migrate and migration is still needed for
- // settings, there is no need to acknowledge the password migration warning.
- if (pref_service->GetBoolean(
- password_manager::prefs::kEmptyProfileStoreLoginDatabase)) {
- return false;
- }
-
- // There is no warning shown on automotive.
- if (base::android::BuildInfo::GetInstance()->is_automotive()) {
- return false;
- }
-
- if (!base::FeatureList::IsEnabled(
- password_manager::features::
- kUnifiedPasswordManagerLocalPasswordsMigrationWarning)) {
- return false;
- }
- return !pref_service->GetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning);
-}
-
bool HasCustomPasswordSettings(PrefService* pref_service) {
bool has_custom_enable_service_setting =
!pref_service
@@ -200,16 +172,10 @@
}
UseUpmLocalAndSeparateStoresState state_to_set_on_success = kOn;
- ActivationError error = ActivationError::kNone;
switch (user_type) {
case UserType::kNonSyncingAndNoMigrationNeeded:
break;
case UserType::kNonSyncingAndMigrationNeeded:
- if (ShouldDelayMigrationUntillMigrationWarningIsAcknowledged(
- pref_service)) {
- error = ActivationError::kMigrationWarningUnacknowledged;
- break;
- }
state_to_set_on_success = kOffAndMigrationPending;
break;
case UserType::kSyncing: {
@@ -225,12 +191,9 @@
break;
}
}
- RecordActivationError(user_type, error);
-
- if (error == ActivationError::kNone) {
- pref_service->SetInteger(kPasswordsUseUPMLocalAndSeparateStores,
- static_cast<int>(state_to_set_on_success));
- }
+ RecordActivationError(user_type, ActivationError::kNone);
+ pref_service->SetInteger(kPasswordsUseUPMLocalAndSeparateStores,
+ static_cast<int>(state_to_set_on_success));
}
#if !BUILDFLAG(USE_LOGIN_DATABASE_AS_BACKEND)
diff --git a/chrome/browser/password_manager/android/password_manager_android_util_unittest.cc b/chrome/browser/password_manager/android/password_manager_android_util_unittest.cc
index a026dd9..a37a37b7 100644
--- a/chrome/browser/password_manager/android/password_manager_android_util_unittest.cc
+++ b/chrome/browser/password_manager/android/password_manager_android_util_unittest.cc
@@ -96,15 +96,9 @@
// Duplicated from password_manager_android_util.cc, which is fine since the
// enum values should never change.
enum class ActivationError {
- // The user was activated or the local passwords/settings migration was
- // scheduled if one is needed.
kNone = 0,
- kUnenrolled = 1,
- kInitialUpmMigrationMissing = 2,
- kLoginDbFileMoveFailed = 3,
kOutdatedGmsCore = 4,
- kMigrationWarningUnacknowledged = 6,
- kMax = kMigrationWarningUnacknowledged,
+ kMax = kOutdatedGmsCore,
};
password_manager::PasswordForm MakeExampleForm() {
@@ -197,10 +191,6 @@
".", syncer::DataTypeToStableLowerCaseString(syncer::PASSWORDS)}),
false);
pref_service_.registry()->RegisterBooleanPref(
- password_manager::prefs::
- kUserAcknowledgedLocalPasswordsMigrationWarning,
- false);
- pref_service_.registry()->RegisterBooleanPref(
password_manager::prefs::kSettingsMigratedToUPMLocal, false);
pref_service_.registry()->RegisterBooleanPref(
password_manager::prefs::kUpmUnmigratedPasswordsExported, false);
@@ -852,9 +842,6 @@
password_manager::prefs::kEmptyProfileStoreLoginDatabase, true);
pref_service()->SetBoolean(password_manager::prefs::kCredentialsEnableService,
false);
- pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- false);
ASSERT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
static_cast<int>(kOff));
@@ -862,9 +849,8 @@
GetMockBridgeWithBackendPresent());
// The migration is pending.
- // Even though the migration warning was not acknowledged, the migration
- // should happen because there are no local passwords. Only settings should be
- // migrated.
+ // The migration should happen because there are no local passwords. Only
+ // settings should be migrated.
EXPECT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
static_cast<int>(kOffAndMigrationPending));
histogram_tester->ExpectUniqueSample(
@@ -909,10 +895,6 @@
TEST_F(PasswordManagerUpmActivationTest,
SetUsesSplitStoresAndUPMForLocal_SignedOutWithPasswords) {
- base::test::ScopedFeatureList scoped_feature_state;
- scoped_feature_state.InitAndEnableFeature(
- password_manager::features::
- kUnifiedPasswordManagerLocalPasswordsMigrationWarning);
auto histogram_tester = std::make_unique<base::HistogramTester>();
pref_service()->SetBoolean(
password_manager::prefs::kEmptyProfileStoreLoginDatabase, false);
@@ -922,33 +904,6 @@
SetUsesSplitStoresAndUPMForLocal(pref_service(), login_db_directory(),
GetMockBridgeWithBackendPresent());
- if (!base::android::BuildInfo::GetInstance()->is_automotive()) {
- // The migration warning was not acknowledged so the migration attempt
- // fails.
- EXPECT_EQ(
- pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
- static_cast<int>(kOff));
- histogram_tester->ExpectUniqueSample(
- "PasswordManager.LocalUpmActivationError.NonSyncingWithMigration",
- ActivationError::kMigrationWarningUnacknowledged, 1);
- histogram_tester->ExpectUniqueSample("PasswordManager.LocalUpmActivated",
- false, 1);
- histogram_tester->ExpectUniqueSample(
- "PasswordManager.LocalUpmActivationStatus", kOff, 1);
- histogram_tester = std::make_unique<base::HistogramTester>();
- pref_service()->SetBoolean(
- password_manager::prefs::
- kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
-
- // Try again.
- SetUsesSplitStoresAndUPMForLocal(pref_service(), login_db_directory(),
- GetMockBridgeWithBackendPresent());
- } else {
- // On Android Auto, the migration warning is not shown, so acknowledging is
- // not required.
- }
-
// The migration got marked as pending (but the user is not considered
// activated).
EXPECT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
@@ -995,46 +950,12 @@
"PasswordManager.LocalUpmActivationStatus", kOn, 1);
}
-// Tests that acknowledging the migration warning is no longer required for
-// migration.
-TEST_F(PasswordManagerUpmActivationTest,
- SetUsesSplitStoresAndUPMForLocal_SkipMigrationWarningAcknowledgement) {
- base::test::ScopedFeatureList scoped_feature_list;
- scoped_feature_list.InitAndDisableFeature(
- password_manager::features::
- kUnifiedPasswordManagerLocalPasswordsMigrationWarning);
- auto histogram_tester = std::make_unique<base::HistogramTester>();
- pref_service()->SetBoolean(
- password_manager::prefs::kEmptyProfileStoreLoginDatabase, false);
- ASSERT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
- static_cast<int>(kOff));
-
- SetUsesSplitStoresAndUPMForLocal(pref_service(), login_db_directory(),
- GetMockBridgeWithBackendPresent());
-
- // The migration got marked as pending (but the user is not considered
- // activated).
- EXPECT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
- static_cast<int>(kOffAndMigrationPending));
- histogram_tester->ExpectUniqueSample(
- "PasswordManager.LocalUpmActivationError.NonSyncingWithMigration",
- ActivationError::kNone, 1);
- histogram_tester->ExpectUniqueSample("PasswordManager.LocalUpmActivated",
- false, 1);
- histogram_tester->ExpectUniqueSample(
- "PasswordManager.LocalUpmActivationStatus", kOffAndMigrationPending, 1);
- histogram_tester = std::make_unique<base::HistogramTester>();
-}
-
TEST_F(
PasswordManagerUpmActivationTest,
SetUsesSplitStoresAndUPMForLocal_SignedOutWithCustomEnableServiceSetting) {
auto histogram_tester = std::make_unique<base::HistogramTester>();
pref_service()->SetBoolean(password_manager::prefs::kCredentialsEnableService,
false);
- pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
ASSERT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
static_cast<int>(kOff));
@@ -1059,9 +980,6 @@
auto histogram_tester = std::make_unique<base::HistogramTester>();
pref_service()->SetBoolean(
password_manager::prefs::kCredentialsEnableAutosignin, false);
- pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
ASSERT_EQ(pref_service()->GetInteger(kPasswordsUseUPMLocalAndSeparateStores),
static_cast<int>(kOff));
@@ -1243,9 +1161,6 @@
base::NumberToString(GetLocalUpmMinGmsVersion() - 1));
pref_service()->SetBoolean(
password_manager::prefs::kEmptyProfileStoreLoginDatabase, false);
- pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
SetUsesSplitStoresAndUPMForLocal(pref_service(), login_db_directory(),
GetMockBridgeWithBackendPresent());
@@ -1315,9 +1230,6 @@
static_cast<int>(kOffAndMigrationPending));
pref_service()->SetBoolean(
password_manager::prefs::kEmptyProfileStoreLoginDatabase, false);
- pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
SetUsesSplitStoresAndUPMForLocal(pref_service(), login_db_directory(),
GetMockBridgeWithBackendPresent());
@@ -1339,9 +1251,6 @@
pref_service()->SetInteger(kPasswordsUseUPMLocalAndSeparateStores,
static_cast<int>(kOn));
pref_service()->SetBoolean(
- password_manager::prefs::kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
- pref_service()->SetBoolean(
password_manager::prefs::kEmptyProfileStoreLoginDatabase, false);
// Creating the login data files for testing.
@@ -1652,9 +1561,8 @@
TEST_F(UsesSplitStoresAndUPMForLocalTest, SignedOutWithPasswords) {
{
- // Set up a signed-out user, with saved passwords, who already acknowledged
- // the migration warning. Prevent activation before the passwords are added,
- // by faking an outdated GmsCore.
+ // Set up a signed-out user, with saved passwords. Prevent activation before
+ // the passwords are added, by faking an outdated GmsCore.
base::android::BuildInfo::GetInstance()->set_gms_version_code_for_test(
base::NumberToString(GetLocalUpmMinGmsVersion() - 1));
CreateProfile();
@@ -1662,10 +1570,6 @@
CreateSyncService();
profile_password_store()->AddLogin(MakeExampleForm());
ASSERT_FALSE(UsesSplitStoresAndUPMForLocal(pref_service()));
- pref_service()->SetBoolean(
- password_manager::prefs::
- kUserAcknowledgedLocalPasswordsMigrationWarning,
- true);
DestroyProfile();
}
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index c409f160..412675b2 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -1091,6 +1091,12 @@
inline constexpr char kManagedAccessToGetAllScreensMediaAllowedForUrls[] =
"profile.managed_access_to_get_all_screens_media_allowed_for_urls";
+#if BUILDFLAG(IS_ANDROID)
+// Deprecated 04/2025.
+constexpr char kObsoleteUserAcknowledgedLocalPasswordsMigrationWarning[] =
+ "user_acknowledged_local_passwords_migration_warning";
+#endif
+
// Register local state used only for migration (clearing or moving to a new
// key).
void RegisterLocalStatePrefsForMigration(PrefRegistrySimple* registry) {
@@ -1529,6 +1535,12 @@
// Deprecated 04/2025.
registry->RegisterListPref(kManagedAccessToGetAllScreensMediaAllowedForUrls);
+
+#if BUILDFLAG(IS_ANDROID)
+ // Deprecated 04/2025.
+ registry->RegisterBooleanPref(
+ kObsoleteUserAcknowledgedLocalPasswordsMigrationWarning, false);
+#endif
}
} // namespace
@@ -2813,6 +2825,12 @@
// Added 04/2025
profile_prefs->ClearPref(kManagedAccessToGetAllScreensMediaAllowedForUrls);
+#if BUILDFLAG(IS_ANDROID)
+ // Added 04/2025
+ profile_prefs->ClearPref(
+ kObsoleteUserAcknowledgedLocalPasswordsMigrationWarning);
+#endif
+
// Please don't delete the following line. It is used by PRESUBMIT.py.
// END_MIGRATE_OBSOLETE_PROFILE_PREFS
