Add scaffold of SetUsesSplitStoresAndUPMForLocal() and invoke it
This is the function that will be called on startup to set the
value of UsesSplitStoresAndUPMForLocal(). Per the doc linked in
https://crbug.com/1495626, it needs to be invoked before any keyed
services are created. This is done in MigrateObsoleteProfilePrefs(),
which is the same approach taken by https://crrev.com/c/5033264.
Also add a call to MigrateObsoleteProfilePrefs() to TestingProfile,
so the latter can be used for testing later.
No behavior change (not even behind flag) because the function is not
implemented yet.
Bug: 1495626
Change-Id: Ib8878a97f493d289e7c26c8fe681e175091da773
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5088792
Auto-Submit: Victor Vianna <[email protected]>
Commit-Queue: Monica Basta <[email protected]>
Reviewed-by: Ioana Pandele <[email protected]>
Reviewed-by: Dominic Battre <[email protected]>
Reviewed-by: Monica Basta <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1233943}
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 4c396bb..1ba947d 100644
--- a/chrome/browser/password_manager/android/password_manager_android_util.cc
+++ b/chrome/browser/password_manager/android/password_manager_android_util.cc
@@ -5,6 +5,7 @@
#include "chrome/browser/password_manager/android/password_manager_android_util.h"
#include "base/feature_list.h"
+#include "base/notreached.h"
#include "chrome/browser/password_manager/android/password_manager_eviction_util.h"
#include "components/password_manager/core/browser/features/password_features.h"
@@ -33,4 +34,10 @@
return UsesSplitStoresAndUPMForLocal(pref_service);
}
+void SetUsesSplitStoresAndUPMForLocal(
+ PrefService* pref_service,
+ const base::FilePath& login_db_directory) {
+ NOTIMPLEMENTED();
+}
+
} // namespace password_manager_android_util
diff --git a/chrome/browser/password_manager/android/password_manager_android_util.h b/chrome/browser/password_manager/android/password_manager_android_util.h
index ff1a3a0b..c6c09ee 100644
--- a/chrome/browser/password_manager/android/password_manager_android_util.h
+++ b/chrome/browser/password_manager/android/password_manager_android_util.h
@@ -7,6 +7,10 @@
class PrefService;
+namespace base {
+class FilePath;
+} // namespace base
+
namespace password_manager_android_util {
// Checks whether the UPM for local users is activated for this client.
// This also means that the single password store has been split in
@@ -17,6 +21,11 @@
// or local passwords.
bool CanUseUPMBackend(bool is_pwd_sync_enabled, PrefService* pref_service);
+// Called on startup to update the value of UsesSplitStoresAndUPMForLocal(),
+// based on feature flags, minimum GmsCore version and other criteria.
+void SetUsesSplitStoresAndUPMForLocal(PrefService* pref_service,
+ const base::FilePath& login_db_directory);
+
} // namespace password_manager_android_util
#endif // CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_PASSWORD_MANAGER_ANDROID_UTIL_H_
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 2c9e95b..9f162ca2 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -266,6 +266,7 @@
#include "chrome/browser/lens/android/lens_prefs.h"
#include "chrome/browser/media/android/cdm/media_drm_origin_id_manager.h"
#include "chrome/browser/notifications/notification_channels_provider_android.h"
+#include "chrome/browser/password_manager/android/password_manager_android_util.h"
#include "chrome/browser/readaloud/android/prefs.h"
#include "chrome/browser/ssl/known_interception_disclosure_infobar_delegate.h"
#include "components/cdm/browser/media_drm_storage_impl.h" // nogncheck crbug.com/1125897
@@ -2303,7 +2304,8 @@
// This method should be periodically pruned of year+ old migrations.
// See chrome/browser/prefs/README.md for details.
-void MigrateObsoleteProfilePrefs(PrefService* profile_prefs) {
+void MigrateObsoleteProfilePrefs(PrefService* profile_prefs,
+ const base::FilePath& profile_path) {
// IMPORTANT NOTE: This code is *not* run on iOS Chrome. If a pref is migrated
// or cleared here, and that pref is also used in iOS Chrome, it may also need
// to be migrated or cleared specifically for iOS as well. This could be by
@@ -2628,6 +2630,18 @@
profile_prefs);
#endif // !BUILDFLAG(IS_ANDROID)
+#if BUILDFLAG(IS_ANDROID)
+ // Added 11/2023, but DO NOT REMOVE after the usual year!
+ // TODO(crbug.com/1445497): The pref kPasswordsUseUPMLocalAndSeparateStores
+ // and this call (to compute said pref) should be removed once
+ // kUnifiedPasswordManagerLocalPasswordsAndroidWithMigration is launched and
+ // enough clients have migrated. UsesSplitStoresAndUPMForLocal() should be
+ // updated to check the GmsCoreVersion directly instead of the pref, or might
+ // be removed entirely, depending how the outdated GmsCore case is handled.
+ password_manager_android_util::SetUsesSplitStoresAndUPMForLocal(profile_prefs,
+ profile_path);
+#endif
+
// Deprecated 11/2023.
profile_prefs->ClearPref(kPasswordChangeSuccessTrackerFlows);
profile_prefs->ClearPref(kPasswordChangeSuccessTrackerVersion);
diff --git a/chrome/browser/prefs/browser_prefs.h b/chrome/browser/prefs/browser_prefs.h
index e2566884..f2c2a128 100644
--- a/chrome/browser/prefs/browser_prefs.h
+++ b/chrome/browser/prefs/browser_prefs.h
@@ -14,6 +14,10 @@
class PrefRegistrySimple;
class PrefService;
+namespace base {
+class FilePath;
+}
+
namespace user_prefs {
class PrefRegistrySyncable;
}
@@ -57,6 +61,7 @@
// deprecated prefs should be removed as new ones are added, but this call
// should never go away (even if it becomes an empty call for some time) as it
// should remain *the* place to drop deprecated profile prefs at.
-void MigrateObsoleteProfilePrefs(PrefService* profile_prefs);
+void MigrateObsoleteProfilePrefs(PrefService* profile_prefs,
+ const base::FilePath& profile_path);
#endif // CHROME_BROWSER_PREFS_BROWSER_PREFS_H_
diff --git a/chrome/browser/prefs/browser_prefs_unittest.cc b/chrome/browser/prefs/browser_prefs_unittest.cc
index 83a767a..2a032e3 100644
--- a/chrome/browser/prefs/browser_prefs_unittest.cc
+++ b/chrome/browser/prefs/browser_prefs_unittest.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/prefs/browser_prefs.h"
+#include "base/files/file_path.h"
#include "build/build_config.h"
#include "components/sync/base/pref_names.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
@@ -21,7 +22,7 @@
};
TEST_F(BrowserPrefsTest, MigrateObsoleteProfilePrefSyncRequestedDefaultValue) {
- MigrateObsoleteProfilePrefs(&prefs_);
+ MigrateObsoleteProfilePrefs(&prefs_, /*profile_path=*/base::FilePath());
EXPECT_EQ(nullptr, prefs_.GetUserPrefValue(kSyncRequested));
#if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -32,7 +33,7 @@
TEST_F(BrowserPrefsTest, MigrateObsoleteProfilePrefSyncRequestedSetToTrue) {
prefs_.SetBoolean(kSyncRequested, true);
- MigrateObsoleteProfilePrefs(&prefs_);
+ MigrateObsoleteProfilePrefs(&prefs_, /*profile_path=*/base::FilePath());
EXPECT_EQ(nullptr, prefs_.GetUserPrefValue(kSyncRequested));
#if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -43,7 +44,7 @@
TEST_F(BrowserPrefsTest, MigrateObsoleteProfilePrefSyncRequestedSetToFalse) {
prefs_.SetBoolean(kSyncRequested, false);
- MigrateObsoleteProfilePrefs(&prefs_);
+ MigrateObsoleteProfilePrefs(&prefs_, /*profile_path=*/base::FilePath());
EXPECT_EQ(nullptr, prefs_.GetUserPrefValue(kSyncRequested));
#if BUILDFLAG(IS_CHROMEOS_ASH)
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 52a5f08..f9a48f5 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -1117,7 +1117,7 @@
TRACE_EVENT0("browser", "ProfileImpl::OnLocaleReady");
// Migrate obsolete prefs.
- MigrateObsoleteProfilePrefs(GetPrefs());
+ MigrateObsoleteProfilePrefs(GetPrefs(), GetPath());
#if BUILDFLAG(ENABLE_EXTENSIONS)
// Note: Extension preferences can be keyed off the extension ID, so need to
// be handled specially (rather than directly as part of
