Settings Virtual Prefs: Lazily create generated preferences

Previously all generated preferences were created along with the
service. As the SettingsPrivateEventRouter is created along with the
profile, all generated preferences and all services they may depend
on are also created. Preferences are only actually required to exist
when they are observed, which may never occur during the lifetime of
the browser. e.g a user never opens settings.

This CL moves the creation of all generated preferences to only occur
when an attempt is made to interact with a generated preference.

Bug: 1112454
Change-Id: I82a565fd4181be018b9714d8c7a18b85e6bb988d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343076Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Cr-Commit-Position: refs/heads/master@{#800978}

chrome/browser/extensions/api/settings_private/generated_prefs.cc

@@ -22,33 +22,16 @@
 namespace extensions {
 namespace settings_private {
 
-GeneratedPrefs::GeneratedPrefs(Profile* profile) {
-#if defined(OS_CHROMEOS)
-  prefs_[kResolveTimezoneByGeolocationOnOff] =
-      CreateGeneratedResolveTimezoneByGeolocationOnOff(profile);
-  prefs_[kResolveTimezoneByGeolocationMethodShort] =
-      CreateGeneratedResolveTimezoneByGeolocationMethodShort(profile);
-#endif
-  prefs_[content_settings::kCookiePrimarySetting] =
-      std::make_unique<content_settings::GeneratedCookiePrimarySettingPref>(
-          profile);
-  prefs_[content_settings::kCookieSessionOnly] =
-      std::make_unique<content_settings::GeneratedCookieSessionOnlyPref>(
-          profile);
-  prefs_[kGeneratedPasswordLeakDetectionPref] =
-      std::make_unique<GeneratedPasswordLeakDetectionPref>(profile);
-  prefs_[safe_browsing::kGeneratedSafeBrowsingPref] =
-      std::make_unique<safe_browsing::GeneratedSafeBrowsingPref>(profile);
-}
+GeneratedPrefs::GeneratedPrefs(Profile* profile) : profile_(profile) {}
 
 GeneratedPrefs::~GeneratedPrefs() = default;
 
-bool GeneratedPrefs::HasPref(const std::string& pref_name) const {
+bool GeneratedPrefs::HasPref(const std::string& pref_name) {
   return FindPrefImpl(pref_name) != nullptr;
 }
 
 std::unique_ptr<api::settings_private::PrefObject> GeneratedPrefs::GetPref(
-    const std::string& pref_name) const {
+    const std::string& pref_name) {
   GeneratedPref* impl = FindPrefImpl(pref_name);
   if (!impl)
     return nullptr;
@@ -88,8 +71,10 @@ void GeneratedPrefs::Shutdown() {
   prefs_.clear();
 }
 
-GeneratedPref* GeneratedPrefs::FindPrefImpl(
-    const std::string& pref_name) const {
+GeneratedPref* GeneratedPrefs::FindPrefImpl(const std::string& pref_name) {
+  if (prefs_.empty())
+    CreatePrefs();
+
   const PrefsMap::const_iterator it = prefs_.find(pref_name);
   if (it == prefs_.end())
     return nullptr;
@@ -97,5 +82,24 @@ GeneratedPref* GeneratedPrefs::FindPrefImpl(
   return it->second.get();
 }
 
+void GeneratedPrefs::CreatePrefs() {
+#if defined(OS_CHROMEOS)
+  prefs_[kResolveTimezoneByGeolocationOnOff] =
+      CreateGeneratedResolveTimezoneByGeolocationOnOff(profile_);
+  prefs_[kResolveTimezoneByGeolocationMethodShort] =
+      CreateGeneratedResolveTimezoneByGeolocationMethodShort(profile_);
+#endif
+  prefs_[content_settings::kCookiePrimarySetting] =
+      std::make_unique<content_settings::GeneratedCookiePrimarySettingPref>(
+          profile_);
+  prefs_[content_settings::kCookieSessionOnly] =
+      std::make_unique<content_settings::GeneratedCookieSessionOnlyPref>(
+          profile_);
+  prefs_[kGeneratedPasswordLeakDetectionPref] =
+      std::make_unique<GeneratedPasswordLeakDetectionPref>(profile_);
+  prefs_[safe_browsing::kGeneratedSafeBrowsingPref] =
+      std::make_unique<safe_browsing::GeneratedSafeBrowsingPref>(profile_);
+}
+
 }  // namespace settings_private
 }  // namespace extensions

chrome/browser/extensions/api/settings_private/generated_prefs.h

@@ -42,11 +42,11 @@ class GeneratedPrefs : public KeyedService {
   ~GeneratedPrefs() override;
 
   // Returns true if preference is supported.
-  bool HasPref(const std::string& pref_name) const;
+  bool HasPref(const std::string& pref_name);
 
   // Returns fully populated PrefObject or nullptr if not supported.
   std::unique_ptr<api::settings_private::PrefObject> GetPref(
-      const std::string& pref_name) const;
+      const std::string& pref_name);
 
   // Updates preference value.
   SetPrefResult SetPref(const std::string& pref_name, const base::Value* value);
@@ -61,12 +61,18 @@ class GeneratedPrefs : public KeyedService {
   void Shutdown() override;
 
  private:
-  // Returns preference implementation or nullptr if not found.
-  GeneratedPref* FindPrefImpl(const std::string& pref_name) const;
+  // Returns preference implementation or nullptr if not found. Will create
+  // preferences if they haven't already been created.
+  GeneratedPref* FindPrefImpl(const std::string& pref_name);
 
-  // Known preference map.
+  // Creates all generated preferences and populates the preference map.
+  void CreatePrefs();
+
+  // Preference object map.
   PrefsMap prefs_;
 
+  Profile* profile_;
+
   DISALLOW_COPY_AND_ASSIGN(GeneratedPrefs);
 };
 

chrome/browser/extensions/api/settings_private/generated_prefs_factory.cc

@@ -29,10 +29,7 @@ GeneratedPrefsFactory* GeneratedPrefsFactory::GetInstance() {
 GeneratedPrefsFactory::GeneratedPrefsFactory()
     : BrowserContextKeyedServiceFactory(
           "GeneratedPrefs",
-          BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(ProfileSyncServiceFactory::GetInstance());
-  DependsOn(IdentityManagerFactory::GetInstance());
-}
+          BrowserContextDependencyManager::GetInstance()) {}
 
 GeneratedPrefsFactory::~GeneratedPrefsFactory() {}
 

chrome/browser/password_manager/generated_password_leak_detection_pref.cc

@@ -74,6 +74,9 @@ GeneratedPasswordLeakDetectionPref::GeneratedPasswordLeakDetectionPref(
   if (auto* identity_manager = IdentityManagerFactory::GetForProfile(profile))
     identity_manager_observer_.Add(identity_manager);
 
+  if (auto* identity_manager_factory = IdentityManagerFactory::GetInstance())
+    identity_manager_factory_observer_.Add(identity_manager_factory);
+
   if (auto* sync_service = ProfileSyncServiceFactory::GetForProfile(profile))
     sync_service_observer_.Add(sync_service);
 }
@@ -133,6 +136,11 @@ void GeneratedPasswordLeakDetectionPref::OnSourcePreferencesChanged() {
   NotifyObservers(kGeneratedPasswordLeakDetectionPref);
 }
 
+void GeneratedPasswordLeakDetectionPref::IdentityManagerShutdown(
+    signin::IdentityManager* identity_manager) {
+  identity_manager_observer_.RemoveAll();
+}
+
 void GeneratedPasswordLeakDetectionPref::OnPrimaryAccountSet(
     const CoreAccountInfo& primary_account_info) {
   NotifyObservers(kGeneratedPasswordLeakDetectionPref);
@@ -157,3 +165,8 @@ void GeneratedPasswordLeakDetectionPref::OnStateChanged(
     syncer::SyncService* sync) {
   NotifyObservers(kGeneratedPasswordLeakDetectionPref);
 }
+
+void GeneratedPasswordLeakDetectionPref::OnSyncShutdown(
+    syncer::SyncService* sync) {
+  sync_service_observer_.RemoveAll();
+}

chrome/browser/password_manager/generated_password_leak_detection_pref.h

@@ -8,6 +8,7 @@
 #include "base/scoped_observer.h"
 #include "chrome/browser/extensions/api/settings_private/generated_pref.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/identity_manager_factory.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/sync/driver/sync_service.h"
@@ -21,6 +22,7 @@ extern const char kGeneratedPasswordLeakDetectionPref[];
 // logic used to generate these behaviors.
 class GeneratedPasswordLeakDetectionPref
     : public extensions::settings_private::GeneratedPref,
+      public IdentityManagerFactory::Observer,
       public signin::IdentityManager::Observer,
       public syncer::SyncServiceObserver {
  public:
@@ -44,8 +46,13 @@ class GeneratedPasswordLeakDetectionPref
   void OnExtendedAccountInfoUpdated(const AccountInfo& info) override;
   void OnExtendedAccountInfoRemoved(const AccountInfo& info) override;
 
+  // IdentityManagerFactory::Observer implementation.
+  void IdentityManagerShutdown(
+      signin::IdentityManager* identity_manager) override;
+
   // syncer::SyncServiceObserver implementation.
   void OnStateChanged(syncer::SyncService* sync) override;
+  void OnSyncShutdown(syncer::SyncService* sync) override;
 
  private:
   // Non-owning pointer to the profile this preference is generated for.
@@ -53,6 +60,8 @@ class GeneratedPasswordLeakDetectionPref
 
   ScopedObserver<signin::IdentityManager, signin::IdentityManager::Observer>
       identity_manager_observer_{this};
+  ScopedObserver<IdentityManagerFactory, IdentityManagerFactory::Observer>
+      identity_manager_factory_observer_{this};
   ScopedObserver<syncer::SyncService, syncer::SyncServiceObserver>
       sync_service_observer_{this};
   PrefChangeRegistrar user_prefs_registrar_;