Refactor TestingPrefStore to make it useful to some tests of load errors and asynchrony.

base/prefs/testing_pref_store.cc

 // Copyright (c) 2012 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 "base/prefs/testing_pref_store.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "base/values.h"
 
 TestingPrefStore::TestingPrefStore()
     : read_only_(true),
-      init_complete_(false) {
-}
+      read_success_(true),
+      read_error_(PersistentPrefStore::PREF_READ_ERROR_NONE),
+      block_async_read_(false),
+      pending_async_read_(false),
+      init_complete_(false),
+      committed_(true) {}
 
 bool TestingPrefStore::GetValue(const std::string& key,
                                 const base::Value** value) const {
   return prefs_.GetValue(key, value);
 }
 
 bool TestingPrefStore::GetMutableValue(const std::string& key,
                                        base::Value** value) {
   return prefs_.GetValue(key, value);
 }
 
 void TestingPrefStore::AddObserver(PrefStore::Observer* observer) {
   observers_.AddObserver(observer);
 }
 
 void TestingPrefStore::RemoveObserver(PrefStore::Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
 bool TestingPrefStore::HasObservers() const {
   return observers_.might_have_observers();
 }
 
 bool TestingPrefStore::IsInitializationComplete() const {
   return init_complete_;
 }
 
 void TestingPrefStore::SetValue(const std::string& key, base::Value* value) {
-  if (prefs_.SetValue(key, value))
+  if (prefs_.SetValue(key, value)) {
+    committed_ = false;
     NotifyPrefValueChanged(key);
+  }
 }
 
 void TestingPrefStore::SetValueSilently(const std::string& key,
                                         base::Value* value) {
-  prefs_.SetValue(key, value);
+  if (prefs_.SetValue(key, value))
+    committed_ = false;
 }
 
 void TestingPrefStore::RemoveValue(const std::string& key) {
-  if (prefs_.RemoveValue(key))
+  if (prefs_.RemoveValue(key)) {
+    committed_ = false;
     NotifyPrefValueChanged(key);
+  }
 }
 
 bool TestingPrefStore::ReadOnly() const {
   return read_only_;
 }
 
 PersistentPrefStore::PrefReadError TestingPrefStore::GetReadError() const {
-  return PersistentPrefStore::PREF_READ_ERROR_NONE;
+  return read_error_;
 }
 
 PersistentPrefStore::PrefReadError TestingPrefStore::ReadPrefs() {
   NotifyInitializationCompleted();
-  return PersistentPrefStore::PREF_READ_ERROR_NONE;
+  return read_error_;
 }
 
-void TestingPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate_raw) {
-  scoped_ptr<ReadErrorDelegate> error_delegate(error_delegate_raw);
+void TestingPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate) {
+  DCHECK(!pending_async_read_);
+  error_delegate_.reset(error_delegate);
+  if (block_async_read_)
+    pending_async_read_ = true;
+  else
+    NotifyInitializationCompleted();
+}
+
+void TestingPrefStore::CommitPendingWrite() { committed_ = true; }
+
+void TestingPrefStore::SetInitializationCompleted() {
   NotifyInitializationCompleted();
 }
 
-void TestingPrefStore::SetInitializationCompleted() {
-  init_complete_ = true;
-  NotifyInitializationCompleted();
-}
-
 void TestingPrefStore::NotifyPrefValueChanged(const std::string& key) {
   FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key));
 }
 
 void TestingPrefStore::NotifyInitializationCompleted() {
-  FOR_EACH_OBSERVER(Observer, observers_, OnInitializationCompleted(true));
+  DCHECK(!init_complete_);
+  init_complete_ = true;
+  if (read_success_ && read_error_ != PREF_READ_ERROR_NONE && error_delegate_)

[Mattias Nissler (ping if slow), 2014/03/27 20:28:05]

nit: I'd drop the read_error_ != PREF_READ_ERROR_NONE clause here, which would
leave tests fully in control of the returned error code.

[erikwright (departed), 2014/03/28 12:29:22]

This logic models the behavior of JsonPrefStore, which is the only actual
implementation of PersistentPrefStore.

To remove the logic would require me to add another bit of state, which is
whether or not to actually call the error delegate.

While the API of JsonPrefStore (and thus PersistentPrefStore) is a bit awkward,
I think for now it makes sense to have this mock object facilitate testing
something as similar as possible to it.

[Mattias Nissler (ping if slow), 2014/03/28 12:32:53]

Don't you already have that bit in |read_success_| ?

[erikwright (departed), 2014/03/28 12:49:55]

No, in JsonPrefStore, read_success_ can be true while read_error is not NONE.

Strangely, it's only not "success" if there is no directory. If there is a
directory, even if we can't for some reason access the file (which causes a
non-NONE read error) we report success.

As surprising to me as it is to you, but that's what I'm working with, for now.

[Mattias Nissler (ping if slow), 2014/03/28 12:57:41]

OK, makes sense.

+    error_delegate_->OnError(read_error_);
+  FOR_EACH_OBSERVER(
+      Observer, observers_, OnInitializationCompleted(read_success_));
 }
 
 void TestingPrefStore::ReportValueChanged(const std::string& key) {
   FOR_EACH_OBSERVER(Observer, observers_, OnPrefValueChanged(key));
 }
 
 void TestingPrefStore::SetString(const std::string& key,
                                  const std::string& value) {
   SetValue(key, new base::StringValue(value));
 }
@@ skipping 24 matching lines @@
 }
 
 bool TestingPrefStore::GetBoolean(const std::string& key, bool* value) const {
   const base::Value* stored_value;
   if (!prefs_.GetValue(key, &stored_value) || !stored_value)
     return false;
 
   return stored_value->GetAsBoolean(value);
 }
 
+void TestingPrefStore::SetBlockAsyncRead(bool block_async_read) {
+  DCHECK(!init_complete_);
+  block_async_read_ = block_async_read;
+  if (pending_async_read_ && !block_async_read_)
+    NotifyInitializationCompleted();
+}
+
 void TestingPrefStore::set_read_only(bool read_only) {
   read_only_ = read_only;
 }
 
+void TestingPrefStore::set_read_success(bool read_success) {
+  DCHECK(!init_complete_);
+  read_success_ = read_success;
+}
+
+void TestingPrefStore::set_read_error(
+    PersistentPrefStore::PrefReadError read_error) {
+  DCHECK(!init_complete_);
+  read_error_ = read_error;
+}
+
 TestingPrefStore::~TestingPrefStore() {}