Separate storage for protected preferences into Protected Preferences file.

chrome/browser/prefs/tracked/segregated_pref_store.cc

+// Copyright 2014 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.
+
+#include "chrome/browser/prefs/tracked/segregated_pref_store.h"
+
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "base/values.h"
+
+SegregatedPrefStore::AggregatingObserver::AggregatingObserver(
+    SegregatedPrefStore* outer)
+    : outer_(outer),
+      failed_sub_initializations_(0),
+      successful_sub_initializations_(0) {}
+
+void SegregatedPrefStore::AggregatingObserver::OnPrefValueChanged(
+    const std::string& key) {
+  // There is no need to tell clients about changes if they have not yet been
+  // told about initialization.
+  if (failed_sub_initializations_ + successful_sub_initializations_ < 2)
+    return;

[gab, 2014/04/01 18:55:06]

Shouldn't we keep this notification around to send it after we send the
initialization ping?

i.e.
1) Store 1 is initialized
2) A value is changed in store 1
3) Store 2 is initialized
4) We notify observers of initialization

Shouldn't we then 5) Notify observers of the value changed in (2)?

Is it even possible for (2) to occur before (3) or should this return also be
NOTREACHED()?

[erikwright (departed), 2014/04/01 19:46:08]

Yes it is possible for (2) to occur before (3). For example, the PrefHashFilter
attached to Store 1 might change a value in Store 1.

Please see previous discussion with Robert about this.

There is no reason to inform clients of a change in this store when they never
heard of the original value. The value that is present when they first hear
about the store _is_ the original value, as far as they are concerned.

[gab, 2014/04/02 17:02:03]

I was talking about (2) being a "real" change. PrefHashFilter's changes aren't
"real" (i.e. they don't generate OnValueChanged notifications).

I don't think that's possible (since no external user is made aware of the store
before they are both initialized) -- and thus I think this should be a
NOTREACHED().

+
+  FOR_EACH_OBSERVER(
+      PrefStore::Observer, outer_->observers_, OnPrefValueChanged(key));
+}
+
+void SegregatedPrefStore::AggregatingObserver::OnInitializationCompleted(
+    bool succeeded) {
+  if (succeeded)
+    ++successful_sub_initializations_;
+  else
+    ++failed_sub_initializations_;
+
+  DCHECK_LE(failed_sub_initializations_ + successful_sub_initializations_, 2);
+
+  if (failed_sub_initializations_ + successful_sub_initializations_ == 2) {
+

[gab, 2014/04/01 18:55:06]

rm empty line

+    if (!outer_->on_initialization_.is_null())
+      outer_->on_initialization_.Run();
+
+    if (successful_sub_initializations_ == 2 && outer_->read_error_delegate_) {
+      PersistentPrefStore::PrefReadError read_error = outer_->GetReadError();
+      if (read_error != PersistentPrefStore::PREF_READ_ERROR_NONE)
+        outer_->read_error_delegate_->OnError(read_error);
+    }
+
+    FOR_EACH_OBSERVER(
+        PrefStore::Observer,
+        outer_->observers_,
+        OnInitializationCompleted(successful_sub_initializations_ == 2));
+  }
+}
+
+SegregatedPrefStore::SegregatedPrefStore(
+    const scoped_refptr<PersistentPrefStore>& default_pref_store,
+    const scoped_refptr<PersistentPrefStore>& selected_pref_store,
+    const std::set<std::string>& selected_pref_names,
+    const base::Closure& on_initialization)
+    : default_pref_store_(default_pref_store),
+      selected_pref_store_(selected_pref_store),
+      selected_preference_names_(selected_pref_names),
+      on_initialization_(on_initialization),
+      aggregating_observer_(this) {
+
+  default_pref_store_->AddObserver(&aggregating_observer_);
+  selected_pref_store_->AddObserver(&aggregating_observer_);
+}
+
+void SegregatedPrefStore::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void SegregatedPrefStore::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+bool SegregatedPrefStore::HasObservers() const {
+  return observers_.might_have_observers();
+}
+
+bool SegregatedPrefStore::IsInitializationComplete() const {
+  return default_pref_store_->IsInitializationComplete() &&
+         selected_pref_store_->IsInitializationComplete();
+}
+
+bool SegregatedPrefStore::GetValue(const std::string& key,
+                                   const base::Value** result) const {
+  return StoreForKey(key)->GetValue(key, result);
+}
+
+void SegregatedPrefStore::SetValue(const std::string& key, base::Value* value) {
+  StoreForKey(key)->SetValue(key, value);
+}
+
+void SegregatedPrefStore::RemoveValue(const std::string& key) {
+  StoreForKey(key)->RemoveValue(key);
+}
+
+bool SegregatedPrefStore::GetMutableValue(const std::string& key,
+                                          base::Value** result) {
+  return StoreForKey(key)->GetMutableValue(key, result);
+}
+
+void SegregatedPrefStore::ReportValueChanged(const std::string& key) {
+  StoreForKey(key)->ReportValueChanged(key);
+}
+
+void SegregatedPrefStore::SetValueSilently(const std::string& key,
+                                           base::Value* value) {
+  StoreForKey(key)->SetValueSilently(key, value);
+}
+
+bool SegregatedPrefStore::ReadOnly() const {
+  return selected_pref_store_->ReadOnly() ||
+         default_pref_store_->ReadOnly();
+}
+
+PersistentPrefStore::PrefReadError SegregatedPrefStore::GetReadError() const {
+  PersistentPrefStore::PrefReadError read_error =
+      default_pref_store_->GetReadError();
+  return read_error != PersistentPrefStore::PREF_READ_ERROR_NONE
+             ? read_error
+             : selected_pref_store_->GetReadError();
+}
+
+PersistentPrefStore::PrefReadError SegregatedPrefStore::ReadPrefs() {
+  PersistentPrefStore::PrefReadError unselected_read_error =
+      default_pref_store_->ReadPrefs();
+  PersistentPrefStore::PrefReadError selected_read_error =
+      selected_pref_store_->ReadPrefs();
+
+  return unselected_read_error != PersistentPrefStore::PREF_READ_ERROR_NONE
+             ? unselected_read_error
+             : selected_read_error;
+}
+
+void SegregatedPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate) {
+  read_error_delegate_.reset(error_delegate);
+  default_pref_store_->ReadPrefsAsync(NULL);
+  selected_pref_store_->ReadPrefsAsync(NULL);
+}
+
+void SegregatedPrefStore::CommitPendingWrite() {
+  default_pref_store_->CommitPendingWrite();
+  selected_pref_store_->CommitPendingWrite();
+}
+
+SegregatedPrefStore::~SegregatedPrefStore() {
+  default_pref_store_->RemoveObserver(&aggregating_observer_);
+  selected_pref_store_->RemoveObserver(&aggregating_observer_);
+}
+
+const PersistentPrefStore*
+SegregatedPrefStore::StoreForKey(const std::string& key) const {

[gab, 2014/04/01 18:55:06]

Method name fits on previous line, wrap params instead of method name.

+  if (ContainsKey(selected_preference_names_, key) ||
+      selected_pref_store_->GetValue(key, NULL)) {
+    return selected_pref_store_.get();
+  }
+  return default_pref_store_.get();
+}
+
+PersistentPrefStore* SegregatedPrefStore::StoreForKey(const std::string& key) {
+  if (ContainsKey(selected_preference_names_, key))
+    return selected_pref_store_.get();
+
+  // Check if this unselected value was previously selected. If so, migrate it
+  // back to the unselected store.
+  // It's hard to do this in a single pass at startup because PrefStore does not
+  // permit us to enumerate its contents.
+  const base::Value* value = NULL;
+  if (selected_pref_store_->GetValue(key, &value)) {
+    scoped_ptr<base::Value> migrated_value(value->DeepCopy());
+    value = NULL;
+    default_pref_store_->SetValue(key, migrated_value.release());

[gab, 2014/04/01 18:55:06]

Why not just inline value->DeepCopy() here?

+    default_pref_store_->CommitPendingWrite();
+    selected_pref_store_->RemoveValue(key);
+    selected_pref_store_->CommitPendingWrite();

[gab, 2014/04/01 18:55:06]

This commit isn't strictly speaking necessary right?

i.e., the only important thing is that the |default_pref_store| be flushed first
(not that the |selected_pref_store| be flushed immediately after).

Should we remove it?

[erikwright (departed), 2014/04/01 19:46:08]

I suppose it's OK. If someone calls CommitPendingWrite on the
SegregatedPrefStore we will commit each of the underlying stores, which will
have the affect that the caller expected.

+  }
+
+  return default_pref_store_.get();
+}