Remove dependency of PrefSyncableService on PrefsTabHelper.

In preparation of the componentization of PrefSyncableService (with the
goal of sharing the code on iOS), remove the dependency on PrefsTabHelper
by passing a list of preferences to register as overlaid.

Add a helper method in chrome/browser/prefspref_service_syncable_util.cc
to pass the list of preferences previously registered in PrefsTabHelper.

Remove the method PrefsTabHelper::InitIncognitoUserPrefStore().

BUG=522536
TBR=jochen@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#348656}

9 files changed, 68 insertions, 40 deletions

diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc
index b91899b..7784408 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/prefs/pref_service_mock_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/common/chrome_constants.h"
 #include "components/crx_file/id_util.h"
 #include "components/pref_registry/pref_registry_syncable.h"
@@ -197,7 +198,8 @@ void TestExtensionPrefs::AddExtension(Extension* extension) {
 }
 
 PrefService* TestExtensionPrefs::CreateIncognitoPrefService() const {
-  return pref_service_->CreateIncognitoPrefService(
+  return CreateIncognitoPrefServiceSyncable(
+      pref_service_.get(),
       new ExtensionPrefStore(extension_pref_value_map_.get(), true));
 }
 
diff --git a/chrome/browser/prefs/pref_service_syncable.cc b/chrome/browser/prefs/pref_service_syncable.cc
index c6cf8ba..0ab3b89 100644
--- a/chrome/browser/prefs/pref_service_syncable.cc
+++ b/chrome/browser/prefs/pref_service_syncable.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/prefs/pref_service_syncable.h"
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/prefs/default_pref_store.h"
 #include "base/prefs/overlay_user_pref_store.h"
@@ -15,7 +16,6 @@
 #include "base/value_conversions.h"
 #include "chrome/browser/prefs/pref_model_associator.h"
 #include "chrome/browser/prefs/pref_service_syncable_observer.h"
-#include "chrome/browser/ui/prefs/prefs_tab_helper.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 
 PrefServiceSyncable::PrefServiceSyncable(
@@ -38,9 +38,8 @@ PrefServiceSyncable::PrefServiceSyncable(
   priority_pref_sync_associator_.SetPrefService(this);
 
   // Let PrefModelAssociators know about changes to preference values.
-  pref_value_store->set_callback(
-      base::Bind(&PrefServiceSyncable::ProcessPrefChange,
-                 base::Unretained(this)));
+  pref_value_store->set_callback(base::Bind(
+      &PrefServiceSyncable::ProcessPrefChange, base::Unretained(this)));
 
   // Add already-registered syncable preferences to PrefModelAssociator.
   for (PrefRegistry::const_iterator it = pref_registry->begin();
@@ -65,12 +64,14 @@ PrefServiceSyncable::~PrefServiceSyncable() {
 }
 
 PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
-    PrefStore* incognito_extension_prefs) {
+    PrefStore* incognito_extension_pref_store,
+    const std::vector<const char*>& overlay_pref_names) {
   pref_service_forked_ = true;
   PrefNotifierImpl* pref_notifier = new PrefNotifierImpl();
   OverlayUserPrefStore* incognito_pref_store =
       new OverlayUserPrefStore(user_pref_store_.get());
-  PrefsTabHelper::InitIncognitoUserPrefStore(incognito_pref_store);
+  for (const char* overlay_pref_name : overlay_pref_names)
+    incognito_pref_store->RegisterOverlayPref(overlay_pref_name);
 
   scoped_refptr<user_prefs::PrefRegistrySyncable> forked_registry =
       static_cast<user_prefs::PrefRegistrySyncable*>(
@@ -79,7 +80,7 @@ PrefServiceSyncable* PrefServiceSyncable::CreateIncognitoPrefService(
       pref_notifier,
       pref_value_store_->CloneAndSpecialize(NULL,  // managed
                                             NULL,  // supervised_user
-                                            incognito_extension_prefs,
+                                            incognito_extension_pref_store,
                                             NULL,  // command_line_prefs
                                             incognito_pref_store,
                                             NULL,  // recommended
@@ -134,9 +135,8 @@ void PrefServiceSyncable::UpdateCommandLinePrefStore(
   PrefService::UpdateCommandLinePrefStore(cmd_line_store);
 }
 
-void PrefServiceSyncable::AddSyncedPrefObserver(
-    const std::string& name,
-    SyncedPrefObserver* observer) {
+void PrefServiceSyncable::AddSyncedPrefObserver(const std::string& name,
+                                                SyncedPrefObserver* observer) {
   pref_sync_associator_.AddSyncedPrefObserver(name, observer);
   priority_pref_sync_associator_.AddSyncedPrefObserver(name, observer);
 }
@@ -154,8 +154,7 @@ void PrefServiceSyncable::AddRegisteredSyncablePreference(
   DCHECK(FindPreference(path));
   if (flags & user_prefs::PrefRegistrySyncable::SYNCABLE_PREF) {
     pref_sync_associator_.RegisterPref(path.c_str());
-  } else if (flags &
-             user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF) {
+  } else if (flags & user_prefs::PrefRegistrySyncable::SYNCABLE_PRIORITY_PREF) {
     priority_pref_sync_associator_.RegisterPref(path.c_str());
   }
 }
diff --git a/chrome/browser/prefs/pref_service_syncable.h b/chrome/browser/prefs/pref_service_syncable.h
index f94a302..d37b616 100644
--- a/chrome/browser/prefs/pref_service_syncable.h
+++ b/chrome/browser/prefs/pref_service_syncable.h
@@ -5,6 +5,9 @@
 #ifndef CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_H_
 #define CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_H_
 
+#include <vector>
+
+#include "base/callback_forward.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/prefs/pref_model_associator.h"
 #include "chrome/browser/prefs/synced_pref_observer.h"
@@ -23,22 +26,23 @@ class PrefServiceSyncable : public PrefService {
  public:
   // You may wish to use PrefServiceFactory or one of its subclasses
   // for simplified construction.
-  PrefServiceSyncable(
-      PrefNotifierImpl* pref_notifier,
-      PrefValueStore* pref_value_store,
-      PersistentPrefStore* user_prefs,
-      user_prefs::PrefRegistrySyncable* pref_registry,
-      base::Callback<void(PersistentPrefStore::PrefReadError)>
-          read_error_callback,
-      bool async);
+  PrefServiceSyncable(PrefNotifierImpl* pref_notifier,
+                      PrefValueStore* pref_value_store,
+                      PersistentPrefStore* user_prefs,
+                      user_prefs::PrefRegistrySyncable* pref_registry,
+                      base::Callback<void(PersistentPrefStore::PrefReadError)>
+                          read_error_callback,
+                      bool async);
   ~PrefServiceSyncable() override;
 
   // Creates an incognito copy of the pref service that shares most pref stores
   // but uses a fresh non-persistent overlay for the user pref store and an
   // individual extension pref store (to cache the effective extension prefs for
-  // incognito windows).
+  // incognito windows). |overlay_pref_names| is a list of preference names
+  // whose changes will not be persisted by the returned incognito pref service.
   PrefServiceSyncable* CreateIncognitoPrefService(
-      PrefStore* incognito_extension_prefs);
+      PrefStore* incognito_extension_pref_store,
+      const std::vector<const char*>& overlay_pref_names);
 
   // Returns true if preferences state has synchronized with the remote
   // preferences. If true is returned it can be assumed the local preferences
diff --git a/chrome/browser/prefs/pref_service_syncable_util.cc b/chrome/browser/prefs/pref_service_syncable_util.cc
index e3b099c..eac0df9 100644
--- a/chrome/browser/prefs/pref_service_syncable_util.cc
+++ b/chrome/browser/prefs/pref_service_syncable_util.cc
@@ -4,8 +4,17 @@
 
 #include "chrome/browser/prefs/pref_service_syncable_util.h"
 
+#include <vector>
+
+#include "base/logging.h"
+#include "build/build_config.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+
+#if defined(OS_ANDROID) || defined(OS_IOS)
+#include "components/proxy_config/proxy_config_pref_names.h"
+#endif
 
 PrefServiceSyncable* PrefServiceSyncableFromProfile(Profile* profile) {
   return static_cast<PrefServiceSyncable*>(profile->GetPrefs());
@@ -14,3 +23,19 @@ PrefServiceSyncable* PrefServiceSyncableFromProfile(Profile* profile) {
 PrefServiceSyncable* PrefServiceSyncableIncognitoFromProfile(Profile* profile) {
   return static_cast<PrefServiceSyncable*>(profile->GetOffTheRecordPrefs());
 }
+
+PrefServiceSyncable* CreateIncognitoPrefServiceSyncable(
+    PrefServiceSyncable* pref_service,
+    PrefStore* incognito_extension_pref_store) {
+  // List of keys that cannot be changed in the user prefs file by the incognito
+  // profile.  All preferences that store information about the browsing history
+  // or behavior of the user should have this property.
+  std::vector<const char*> overlay_pref_names;
+  overlay_pref_names.push_back(prefs::kBrowserWindowPlacement);
+  overlay_pref_names.push_back(prefs::kSaveFileDefaultDirectory);
+#if defined(OS_ANDROID) || defined(OS_IOS)
+  overlay_pref_names.push_back(proxy_config::prefs::kProxy);
+#endif
+  return pref_service->CreateIncognitoPrefService(
+      incognito_extension_pref_store, overlay_pref_names);
+}
diff --git a/chrome/browser/prefs/pref_service_syncable_util.h b/chrome/browser/prefs/pref_service_syncable_util.h
index 71dd077..dfa40965 100644
--- a/chrome/browser/prefs/pref_service_syncable_util.h
+++ b/chrome/browser/prefs/pref_service_syncable_util.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_UTIL_H_
 
 class PrefServiceSyncable;
+class PrefStore;
 class Profile;
 
 // PrefServiceSyncable is a PrefService with added integration for
@@ -20,4 +21,12 @@ class Profile;
 PrefServiceSyncable* PrefServiceSyncableFromProfile(Profile* profile);
 PrefServiceSyncable* PrefServiceSyncableIncognitoFromProfile(Profile* profile);
 
+// Creates an incognito copy of |pref_service| that shares most prefs but uses
+// a fresh non-persistent overlay for the user pref store and an individual
+// extension pref store (to cache the effective extension prefs for incognito
+// windows).
+PrefServiceSyncable* CreateIncognitoPrefServiceSyncable(
+    PrefServiceSyncable* pref_service,
+    PrefStore* incognito_extension_pref_store);
+
 #endif  // CHROME_BROWSER_PREFS_PREF_SERVICE_SYNCABLE_UTIL_H_
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 9ee9a80..296751fd 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -52,6 +52,7 @@
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/chrome_pref_service_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
 #include "chrome/browser/profiles/bookmark_model_loaded_observer.h"
 #include "chrome/browser/profiles/chrome_version_service.h"
@@ -905,8 +906,8 @@ PrefService* ProfileImpl::GetOffTheRecordPrefs() {
   if (!otr_prefs_) {
     // The new ExtensionPrefStore is ref_counted and the new PrefService
     // stores a reference so that we do not leak memory here.
-    otr_prefs_.reset(prefs_->CreateIncognitoPrefService(
-        CreateExtensionPrefStore(this, true)));
+    otr_prefs_.reset(CreateIncognitoPrefServiceSyncable(
+        prefs_.get(), CreateExtensionPrefStore(this, true)));
   }
   return otr_prefs_.get();
 }
diff --git a/chrome/browser/ui/prefs/prefs_tab_helper.cc b/chrome/browser/ui/prefs/prefs_tab_helper.cc
index 1cf523f..5cb2d6e 100644
--- a/chrome/browser/ui/prefs/prefs_tab_helper.cc
+++ b/chrome/browser/ui/prefs/prefs_tab_helper.cc
@@ -491,19 +491,6 @@ PrefsTabHelper::~PrefsTabHelper() {
 }
 
 // static
-void PrefsTabHelper::InitIncognitoUserPrefStore(
-    OverlayUserPrefStore* pref_store) {
-  // List of keys that cannot be changed in the user prefs file by the incognito
-  // profile.  All preferences that store information about the browsing history
-  // or behavior of the user should have this property.
-  pref_store->RegisterOverlayPref(prefs::kBrowserWindowPlacement);
-  pref_store->RegisterOverlayPref(prefs::kSaveFileDefaultDirectory);
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  pref_store->RegisterOverlayPref(proxy_config::prefs::kProxy);
-#endif  // defined(OS_ANDROID) || defined(OS_IOS)
-}
-
-// static
 void PrefsTabHelper::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
   WebPreferences pref_defaults;
diff --git a/chrome/browser/ui/prefs/prefs_tab_helper.h b/chrome/browser/ui/prefs/prefs_tab_helper.h
index 8ce832a..07e64cd 100644
--- a/chrome/browser/ui/prefs/prefs_tab_helper.h
+++ b/chrome/browser/ui/prefs/prefs_tab_helper.h
@@ -31,7 +31,6 @@ class PrefsTabHelper : public content::NotificationObserver,
  public:
   ~PrefsTabHelper() override;
 
-  static void InitIncognitoUserPrefStore(OverlayUserPrefStore* pref_store);
   static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
   static void GetServiceInstance();
 
diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
index 77e2165..a221ed8 100644
--- a/chrome/test/base/testing_profile.cc
+++ b/chrome/test/base/testing_profile.cc
@@ -33,6 +33,7 @@
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/prefs/pref_service_syncable_util.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -744,7 +745,8 @@ void TestingProfile::CreateIncognitoPrefService() {
   DCHECK(!testing_prefs_);
   // Simplified version of ProfileImpl::GetOffTheRecordPrefs(). Note this
   // leaves testing_prefs_ unset.
-  prefs_.reset(original_profile_->prefs_->CreateIncognitoPrefService(NULL));
+  prefs_.reset(CreateIncognitoPrefServiceSyncable(
+      original_profile_->prefs_.get(), NULL));
   user_prefs::UserPrefs::Set(this, prefs_.get());
 }