Change incognito preference store default to in-memory.

The default storage for OverlayUserPrefStore which is used for
incognito preferences is the persistent storage in the regular profile,
and preference types that should not be stored there must be enumerated
in CreateIncognitoPrefServiceSyncable.

This is changed so that the default storage is in memory, and
preferences that need a persistent storage would need enumeration.

Bug: 861722
Change-Id: Ia4c8f655752a07f81fb6d35685876cd252118999
Reviewed-on: https://chromium-review.googlesource.com/1128753
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575613}

chrome/browser/BUILD.gn

@@ -1151,6 +1151,8 @@ jumbo_split_static_library("browser") {
     "prefs/origin_trial_prefs.h",
     "prefs/pref_metrics_service.cc",
     "prefs/pref_metrics_service.h",
+    "prefs/pref_service_incognito_whitelist.cc",
+    "prefs/pref_service_incognito_whitelist.h",
     "prefs/pref_service_syncable_util.cc",
     "prefs/pref_service_syncable_util.h",
     "prefs/profile_pref_store_manager.cc",

chrome/browser/prefs/pref_service_incognito_whitelist.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_PREFS_PREF_SERVICE_INCOGNITO_WHITELIST_H_
+#define CHROME_BROWSER_PREFS_PREF_SERVICE_INCOGNITO_WHITELIST_H_
+
+#include <vector>
+
+namespace prefs {
+
+// Populate a whitelist of all preferences that are stored on disk in incognito
+// mode. Please refere to the comments in .cc file.
+void GetIncognitoWhitelist(std::vector<const char*>* whitelist);
+
+}  // namespace prefs
+
+#endif  // CHROME_BROWSER_PREFS_PREF_SERVICE_INCOGNITO_WHITELIST_H_

chrome/browser/prefs/pref_service_syncable_util.cc

@@ -6,16 +6,10 @@
 
 #include <vector>
 
-#include "base/logging.h"
-#include "build/build_config.h"
+#include "chrome/browser/prefs/pref_service_incognito_whitelist.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/common/pref_names.h"
 #include "components/sync_preferences/pref_service_syncable.h"
-#include "components/translate/core/browser/translate_prefs.h"
 
-#if defined(OS_ANDROID)
-#include "components/proxy_config/proxy_config_pref_names.h"
-#endif
 
 sync_preferences::PrefServiceSyncable* PrefServiceSyncableFromProfile(
     Profile* profile) {
@@ -34,22 +28,20 @@ CreateIncognitoPrefServiceSyncable(
     sync_preferences::PrefServiceSyncable* pref_service,
     PrefStore* incognito_extension_pref_store,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
-  // 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::kMediaRouterTabMirroringSources);
-  overlay_pref_names.push_back(prefs::kSaveFileDefaultDirectory);
-  overlay_pref_names.push_back(
-      translate::TranslatePrefs::kPrefTranslateSiteBlacklist);
-  overlay_pref_names.push_back(
-      translate::TranslatePrefs::kPrefTranslateBlockedLanguages);
-  overlay_pref_names.push_back(
-      translate::TranslatePrefs::kPrefTranslateWhitelists);
-#if defined(OS_ANDROID)
-  overlay_pref_names.push_back(proxy_config::prefs::kProxy);
-#endif
+  // List of keys that can be changed in the user prefs file by the incognito
+  // profile.
+  std::vector<const char*> persistent_pref_names;
+
+  // TODO(https://crbug.com/861722): Remove |GetIncognitoWhitelist| and its
+  // file. This list is ONLY added for transition of code from blacklist to
+  // whitelist. All whitelisted prefs can be added here to
+  // |persistent_pref_names|.
+  prefs::GetIncognitoWhitelist(&persistent_pref_names);
+
+  // TODO(https://crbug.com/861722): Current implementation does not cover
+  // preferences from iOS. The code should be refactored to cover it.
+
   return pref_service->CreateIncognitoPrefService(
-      incognito_extension_pref_store, overlay_pref_names, std::move(delegate));
+      incognito_extension_pref_store, persistent_pref_names,
+      std::move(delegate));
 }

components/prefs/overlay_user_pref_store.h

@@ -20,18 +20,19 @@
 // PersistentPrefStore that directs all write operations into an in-memory
 // PrefValueMap. Read operations are first answered by the PrefValueMap.
 // If the PrefValueMap does not contain a value for the requested key,
-// the look-up is passed on to an underlying PersistentPrefStore |underlay_|.
+// the look-up is passed on to an underlying PersistentPrefStore
+// |persistent_user_pref_store_|.
 class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore
     : public PersistentPrefStore {
  public:
-  explicit OverlayUserPrefStore(PersistentPrefStore* underlay);
-  // The |overlay| must already be initialized.
-  OverlayUserPrefStore(PersistentPrefStore* overlay,
-                       PersistentPrefStore* underlay);
+  explicit OverlayUserPrefStore(PersistentPrefStore* persistent);
+  // The |ephemeral| store must already be initialized.
+  OverlayUserPrefStore(PersistentPrefStore* ephemeral,
+                       PersistentPrefStore* persistent);
 
   // Returns true if a value has been set for the |key| in this
   // OverlayUserPrefStore, i.e. if it potentially overrides a value
-  // from the |underlay_|.
+  // from the |persistent_user_pref_store_|.
   virtual bool IsSetInOverlay(const std::string& key) const;
 
   // Methods of PrefStore.
@@ -60,7 +61,9 @@ class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore
   void SchedulePendingLossyWrites() override;
   void ReportValueChanged(const std::string& key, uint32_t flags) override;
 
-  void RegisterOverlayPref(const std::string& key);
+  // Registers preferences that should be stored in the persistent preferences
+  // (|persistent_user_pref_store_|).
+  void RegisterPersistentPref(const std::string& key);
 
   void ClearMutableValues() override;
   void OnStoreDeletionFromDisk() override;
@@ -72,20 +75,20 @@ class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore
   typedef std::set<std::string> NamesSet;
   class ObserverAdapter;
 
-  void OnPrefValueChanged(bool overlay, const std::string& key);
-  void OnInitializationCompleted(bool overlay, bool succeeded);
+  void OnPrefValueChanged(bool ephemeral, const std::string& key);
+  void OnInitializationCompleted(bool ephemeral, bool succeeded);
 
   // Returns true if |key| corresponds to a preference that shall be stored in
-  // an in-memory PrefStore that is not persisted to disk.
-  bool ShallBeStoredInOverlay(const std::string& key) const;
+  // persistent PrefStore.
+  bool ShallBeStoredInPersistent(const std::string& key) const;
 
   base::ObserverList<PrefStore::Observer, true> observers_;
-  std::unique_ptr<ObserverAdapter> overlay_observer_;
-  std::unique_ptr<ObserverAdapter> underlay_observer_;
-  scoped_refptr<PersistentPrefStore> overlay_;
-  scoped_refptr<PersistentPrefStore> underlay_;
-  NamesSet overlay_names_set_;
-  NamesSet written_overlay_names_;
+  std::unique_ptr<ObserverAdapter> ephemeral_pref_store_observer_;
+  std::unique_ptr<ObserverAdapter> persistent_pref_store_observer_;
+  scoped_refptr<PersistentPrefStore> ephemeral_user_pref_store_;
+  scoped_refptr<PersistentPrefStore> persistent_user_pref_store_;
+  NamesSet persistent_names_set_;
+  NamesSet written_ephemeral_names_;
 
   DISALLOW_COPY_AND_ASSIGN(OverlayUserPrefStore);
 };

components/sync_preferences/pref_service_syncable.cc

@@ -85,7 +85,7 @@ PrefServiceSyncable::~PrefServiceSyncable() {
 std::unique_ptr<PrefServiceSyncable>
 PrefServiceSyncable::CreateIncognitoPrefService(
     PrefStore* incognito_extension_pref_store,
-    const std::vector<const char*>& overlay_pref_names,
+    const std::vector<const char*>& persistent_pref_names,
     std::unique_ptr<PrefValueStore::Delegate> delegate) {
   pref_service_forked_ = true;
   auto pref_notifier = std::make_unique<PrefNotifierImpl>();
@@ -96,14 +96,14 @@ PrefServiceSyncable::CreateIncognitoPrefService(
   auto overlay = base::MakeRefCounted<InMemoryPrefStore>();
   if (delegate) {
     delegate->InitIncognitoUserPrefs(overlay, user_pref_store_,
-                                     overlay_pref_names);
+                                     persistent_pref_names);
     delegate->InitPrefRegistry(forked_registry.get());
   }
   auto incognito_pref_store = base::MakeRefCounted<OverlayUserPrefStore>(
       overlay.get(), user_pref_store_.get());
 
-  for (const char* overlay_pref_name : overlay_pref_names)
-    incognito_pref_store->RegisterOverlayPref(overlay_pref_name);
+  for (const char* persistent_pref_name : persistent_pref_names)
+    incognito_pref_store->RegisterPersistentPref(persistent_pref_name);
 
   auto pref_value_store = pref_value_store_->CloneAndSpecialize(
       nullptr,  // managed

components/sync_preferences/pref_service_syncable.h

@@ -50,11 +50,11 @@ class PrefServiceSyncable : public PrefService {
   // 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). |overlay_pref_names| is a list of preference names
-  // whose changes will not be persisted by the returned incognito pref service.
+  // incognito windows). |persistent_pref_names| is a list of preference names
+  // whose changes will be persisted by the returned incognito pref service.
   std::unique_ptr<PrefServiceSyncable> CreateIncognitoPrefService(
       PrefStore* incognito_extension_pref_store,
-      const std::vector<const char*>& overlay_pref_names,
+      const std::vector<const char*>& persistent_pref_names,
       std::unique_ptr<PrefValueStore::Delegate> delegate);
 
   // Returns true if preferences state has synchronized with the remote

services/preferences/pref_service_factory_unittest.cc

@@ -613,16 +613,16 @@ class IncognitoPrefServiceFactoryTest
         base::MakeRefCounted<InMemoryPrefStore>();
     scoped_refptr<PersistentPrefStore> underlay =
         base::MakeRefCounted<InMemoryPrefStore>();
-    const auto overlay_pref_names = GetOverlayPrefNames();
-    delegate->InitIncognitoUserPrefs(overlay, underlay, overlay_pref_names);
+    const auto persistent_pref_names = GetPersistentPrefNames();
+    delegate->InitIncognitoUserPrefs(overlay, underlay, persistent_pref_names);
     auto overlay_pref_store = base::MakeRefCounted<OverlayUserPrefStore>(
         overlay.get(), underlay.get());
-    for (auto* overlay_pref_name : overlay_pref_names)
-      overlay_pref_store->RegisterOverlayPref(overlay_pref_name);
+    for (auto* persistent_pref_name : persistent_pref_names)
+      overlay_pref_store->RegisterPersistentPref(persistent_pref_name);
     factory->set_user_prefs(std::move(overlay_pref_store));
   }
 
-  std::vector<const char*> GetOverlayPrefNames() {
+  std::vector<const char*> GetPersistentPrefNames() {
     if (GetParam())
       return {kInitialKey, kOtherInitialKey, kKey};
     return {};

services/preferences/pref_store_manager_impl.cc

@@ -44,7 +44,7 @@ class PrefStoreManagerImpl::ConnectorConnection
       if (owner_->incognito_persistent_pref_store_underlay_->initialized()) {
         connection->ProvideIncognitoPersistentPrefStoreUnderlay(
             owner_->incognito_persistent_pref_store_underlay_.get(),
-            owner_->overlay_pref_names_);
+            owner_->persistent_perf_names_);
       } else {
         owner_->pending_persistent_incognito_connections_.push_back(connection);
       }
@@ -66,7 +66,7 @@ PrefStoreManagerImpl::PrefStoreManagerImpl(
     PersistentPrefStore* incognito_user_prefs_underlay,
     PrefStore* recommended_prefs,
     PrefRegistry* pref_registry,
-    std::vector<const char*> overlay_pref_names)
+    std::vector<const char*> persistent_perf_names)
     : shared_pref_registry_(std::make_unique<SharedPrefRegistry>(
           base::WrapRefCounted(pref_registry))),
       weak_factory_(this) {
@@ -85,9 +85,9 @@ PrefStoreManagerImpl::PrefStoreManagerImpl(
             base::BindOnce(
                 &PrefStoreManagerImpl::OnIncognitoPersistentPrefStoreReady,
                 base::Unretained(this)));
-    overlay_pref_names_ = std::move(overlay_pref_names);
+    persistent_perf_names_ = std::move(persistent_perf_names);
   } else {
-    DCHECK(overlay_pref_names.empty());
+    DCHECK(persistent_perf_names.empty());
   }
   RegisterPrefStore(PrefValueStore::MANAGED_STORE, managed_prefs);
   RegisterPrefStore(PrefValueStore::SUPERVISED_USER_STORE,
@@ -133,7 +133,8 @@ void PrefStoreManagerImpl::OnIncognitoPersistentPrefStoreReady() {
   DVLOG(1) << "Incognito PersistentPrefStore ready";
   for (const auto& connection : pending_persistent_connections_)
     connection->ProvideIncognitoPersistentPrefStoreUnderlay(
-        incognito_persistent_pref_store_underlay_.get(), overlay_pref_names_);
+        incognito_persistent_pref_store_underlay_.get(),
+        persistent_perf_names_);
   pending_persistent_incognito_connections_.clear();
 }
 

services/preferences/pref_store_manager_impl.h

@@ -46,7 +46,7 @@ class PrefStoreManagerImpl : public service_manager::Service {
                        PersistentPrefStore* incognito_user_prefs_underlay,
                        PrefStore* recommended_prefs,
                        PrefRegistry* pref_registry,
-                       std::vector<const char*> overlay_pref_names);
+                       std::vector<const char*> persistent_perf_names);
   ~PrefStoreManagerImpl() override;
 
   base::OnceClosure ShutDownClosure();
@@ -79,7 +79,7 @@ class PrefStoreManagerImpl : public service_manager::Service {
   std::unique_ptr<PersistentPrefStoreImpl> persistent_pref_store_;
   std::unique_ptr<PersistentPrefStoreImpl>
       incognito_persistent_pref_store_underlay_;
-  std::vector<const char*> overlay_pref_names_;
+  std::vector<const char*> persistent_perf_names_;
 
   const std::unique_ptr<SharedPrefRegistry> shared_pref_registry_;
 

services/preferences/public/cpp/in_process_service_factory.cc

@@ -57,11 +57,11 @@ class InProcessPrefServiceFactory::RegisteringDelegate
   void InitIncognitoUserPrefs(
       scoped_refptr<PersistentPrefStore> incognito_user_prefs_overlay,
       scoped_refptr<PersistentPrefStore> incognito_user_prefs_underlay,
-      const std::vector<const char*>& overlay_pref_names) override {
+      const std::vector<const char*>& persistent_perf_names) override {
     factory_->user_prefs_ = std::move(incognito_user_prefs_overlay);
     factory_->incognito_user_prefs_underlay_ =
         std::move(incognito_user_prefs_underlay);
-    factory_->overlay_pref_names_ = overlay_pref_names;
+    factory_->persistent_perf_names_ = persistent_perf_names;
   }
 
   void InitPrefRegistry(PrefRegistry* pref_registry) override {
@@ -104,7 +104,7 @@ InProcessPrefServiceFactory::CreatePrefService() {
       managed_prefs_.get(), supervised_user_prefs_.get(),
       extension_prefs_.get(), command_line_prefs_.get(), user_prefs_.get(),
       incognito_user_prefs_underlay_.get(), recommended_prefs_.get(),
-      pref_registry_.get(), std::move(overlay_pref_names_));
+      pref_registry_.get(), std::move(persistent_perf_names_));
   quit_closure_ = std::move(result.second);
   return std::move(result.first);
 }

services/preferences/public/cpp/in_process_service_factory.h

@@ -43,7 +43,7 @@ class InProcessPrefServiceFactory : public KeyedService {
   scoped_refptr<PersistentPrefStore> incognito_user_prefs_underlay_;
   scoped_refptr<PrefStore> recommended_prefs_;
   scoped_refptr<PrefRegistry> pref_registry_;
-  std::vector<const char*> overlay_pref_names_;
+  std::vector<const char*> persistent_perf_names_;
 
   base::OnceClosure quit_closure_;
 

services/preferences/public/cpp/pref_service_factory.cc

@@ -104,9 +104,9 @@ void OnConnect(
         persistent_pref_store.get(),
         new PersistentPrefStoreClient(
             std::move(incognito_connection->pref_store_connection)));
-    for (const auto& overlay_pref_name :
-         incognito_connection->overlay_pref_names) {
-      overlay_pref_store->RegisterOverlayPref(overlay_pref_name);
+    for (const auto& persistent_pref_name :
+         incognito_connection->persistent_pref_names) {
+      overlay_pref_store->RegisterPersistentPref(persistent_pref_name);
     }
     persistent_pref_store = overlay_pref_store;
   }

services/preferences/public/cpp/pref_service_main.cc

@@ -18,11 +18,11 @@ CreatePrefService(PrefStore* managed_prefs,
                   PersistentPrefStore* incognito_user_prefs_underlay,
                   PrefStore* recommended_prefs,
                   PrefRegistry* pref_registry,
-                  std::vector<const char*> overlay_pref_names) {
+                  std::vector<const char*> persistent_perf_names) {
   auto service = std::make_unique<PrefStoreManagerImpl>(
       managed_prefs, supervised_user_prefs, extension_prefs, command_line_prefs,
       user_prefs, incognito_user_prefs_underlay, recommended_prefs,
-      pref_registry, std::move(overlay_pref_names));
+      pref_registry, std::move(persistent_perf_names));
   auto quit_closure = service->ShutDownClosure();
   return std::make_pair(std::move(service), std::move(quit_closure));
 }

services/preferences/public/mojom/preferences.mojom

@@ -79,7 +79,7 @@ struct PersistentPrefStoreConnection {
 
 struct IncognitoPersistentPrefStoreConnection {
   PersistentPrefStoreConnection pref_store_connection;
-  array<string> overlay_pref_names;
+  array<string> persistent_pref_names;
 };
 
 // Allows connections to pref stores registered with |PrefStoreRegistry|.

services/preferences/scoped_pref_connection_builder.cc

@@ -42,17 +42,17 @@ void ScopedPrefConnectionBuilder::ProvidePersistentPrefStore(
 
 void ScopedPrefConnectionBuilder::ProvideIncognitoPersistentPrefStoreUnderlay(
     PersistentPrefStoreImpl* persistent_pref_store,
-    const std::vector<const char*>& overlay_pref_names) {
+    const std::vector<const char*>& persistent_perf_names) {
   PersistentPrefStoreImpl::ObservedPrefs observed_prefs(observed_prefs_.begin(),
                                                         observed_prefs_.end());
-  std::vector<std::string> filtered_overlay_pref_names;
-  for (const char* overlay_pref_name : overlay_pref_names) {
-    if (base::ContainsKey(observed_prefs, overlay_pref_name))
-      filtered_overlay_pref_names.emplace_back(overlay_pref_name);
+  std::vector<std::string> filtered_persistent_perf_names;
+  for (const char* persistent_perf_name : persistent_perf_names) {
+    if (base::ContainsKey(observed_prefs, persistent_perf_name))
+      filtered_persistent_perf_names.emplace_back(persistent_perf_name);
   }
   incognito_connection_ = mojom::IncognitoPersistentPrefStoreConnection::New(
       persistent_pref_store->CreateConnection(std::move(observed_prefs)),
-      std::move(filtered_overlay_pref_names));
+      std::move(filtered_persistent_perf_names));
 }
 
 void ScopedPrefConnectionBuilder::ProvideDefaults(