Take out and generalize user prefs overriding from IncognitoUserPrefStore.

chrome/browser/prefs/overlay_user_pref_store_unittest.cc

Index: chrome/browser/prefs/overlay_user_pref_store_unittest.cc
diff --git a/chrome/browser/prefs/overlay_user_pref_store_unittest.cc b/chrome/browser/prefs/overlay_user_pref_store_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9644c0d735f63c9f0d6e8554c28499f900490863
--- /dev/null
+++ b/chrome/browser/prefs/overlay_user_pref_store_unittest.cc
@@ -0,0 +1,324 @@
+// Copyright (c) 2011 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/values.h"
+#include "chrome/browser/prefs/overlay_user_pref_store.h"
+#include "chrome/browser/prefs/testing_pref_store.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/pref_store_observer_mock.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ::testing::Mock;
+using ::testing::StrEq;
+
+namespace {
+
+const char* overlay_key = prefs::kBrowserWindowPlacement;
+const char* regular_key = prefs::kShowBookmarkBar;
+// There is no real-life sense in mapping these two properties, it for test
+// purposes only.
+const char* mapped_overlay_key = prefs::kWebKitJavascriptEnabled;
+const char* mapped_underlay_key = prefs::kWebKitJavaEnabled;
+
+class OverlayUserPrefStoreImpl : public OverlayUserPrefStore {

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

You might get away with just giving the test access to
RegisterOverlayProperty(). If you prefer to keep this class, I suggest you
rename it to TestOverlayUserPrefStore to make it clear that it's only useful for
testing.

[mnaganov (inactive), 2011/11/10 14:49:40]

A good idea! Made test class a friend.

+ public:
+  explicit OverlayUserPrefStoreImpl(PersistentPrefStore* underlay)
+      : OverlayUserPrefStore(underlay) {
+    RegisterOverlayProperty(overlay_key);
+    RegisterOverlayProperty(mapped_overlay_key, mapped_underlay_key);
+  }
+
+ private:
+

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

remove newline

[mnaganov (inactive), 2011/11/10 14:49:40]

Removed class.

+  DISALLOW_COPY_AND_ASSIGN(OverlayUserPrefStoreImpl);
+};
+
+}  // namespace
+
+class OverlayUserPrefStoreTest : public testing::Test {
+ public:

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

this can be protected

[mnaganov (inactive), 2011/11/10 14:49:40]

Done.

+  OverlayUserPrefStoreTest()
+      : underlay_(new TestingPrefStore()),
+        overlay_(new OverlayUserPrefStoreImpl(underlay_.get())) {
+  }
+
+  virtual ~OverlayUserPrefStoreTest() {}
+
+  scoped_refptr<TestingPrefStore> underlay_;
+  scoped_refptr<OverlayUserPrefStoreImpl> overlay_;
+};
+
+TEST_F(OverlayUserPrefStoreTest, Observer) {
+  PrefStoreObserverMock obs;
+  overlay_->AddObserver(&obs);
+
+  // Check that underlay first value is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(1);
+  underlay_->SetValue(overlay_key, Value::CreateIntegerValue(42));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that underlay overwriting is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(1);
+  underlay_->SetValue(overlay_key, Value::CreateIntegerValue(43));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that overwriting change in overlay is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(1);
+  overlay_->SetValue(overlay_key, Value::CreateIntegerValue(44));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that hidden underlay change is not reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(0);
+  underlay_->SetValue(overlay_key, Value::CreateIntegerValue(45));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that overlay remove is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(1);
+  overlay_->RemoveValue(overlay_key);
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that underlay remove is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(1);
+  underlay_->RemoveValue(overlay_key);
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check respecting of silence.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(0);
+  overlay_->SetValueSilently(overlay_key, Value::CreateIntegerValue(46));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  overlay_->RemoveObserver(&obs);
+
+  // Check successful unsubscription.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(overlay_key))).Times(0);
+  underlay_->SetValue(overlay_key, Value::CreateIntegerValue(47));
+  overlay_->SetValue(overlay_key, Value::CreateIntegerValue(48));
+  Mock::VerifyAndClearExpectations(&obs);
+}
+
+TEST_F(OverlayUserPrefStoreTest, GetAndSet) {
+  const Value* value = NULL;
+  int i = -1;
+  EXPECT_EQ(PrefStore::READ_NO_VALUE,
+            overlay_->GetValue(overlay_key, &value));
+  EXPECT_EQ(PrefStore::READ_NO_VALUE,
+            underlay_->GetValue(overlay_key, &value));
+
+  underlay_->SetValue(overlay_key, Value::CreateIntegerValue(42));
+
+  // Value shines through:
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(42, i);

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

you could rewrite these three-line blocks as one line like this:

EXPECT_TRUE(FundamentalValue(42).Equals(value));

[mnaganov (inactive), 2011/11/10 14:49:40]

Neat! That significantly reduces the number of lines :)

+
+  EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(42, i);
+
+  overlay_->SetValue(overlay_key, Value::CreateIntegerValue(43));
+
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+
+  EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(42, i);
+
+  overlay_->RemoveValue(overlay_key);
+
+  // Value shines through:
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(42, i);
+
+  EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(42, i);
+}
+
+// Check that GetMutableValue does not return the dictionary of the underlay.
+TEST_F(OverlayUserPrefStoreTest, ModifyDictionaries) {
+  underlay_->SetValue(overlay_key, new DictionaryValue);
+
+  Value* modify = NULL;
+  EXPECT_EQ(PrefStore::READ_OK,
+            overlay_->GetMutableValue(overlay_key, &modify));
+  ASSERT_TRUE(modify);
+  ASSERT_TRUE(modify->GetType() == Value::TYPE_DICTIONARY);

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

type checks are better written using Value::IsType().

[mnaganov (inactive), 2011/11/10 14:49:40]

Done.

+  static_cast<DictionaryValue*>(modify)->SetInteger(overlay_key, 42);
+
+  Value* original_in_underlay = NULL;
+  EXPECT_EQ(PrefStore::READ_OK,
+            underlay_->GetMutableValue(overlay_key, &original_in_underlay));
+  ASSERT_TRUE(original_in_underlay);
+  ASSERT_TRUE(original_in_underlay->GetType() == Value::TYPE_DICTIONARY);
+  EXPECT_TRUE(static_cast<DictionaryValue*>(original_in_underlay)->empty());
+
+  Value* modified = NULL;
+  EXPECT_EQ(PrefStore::READ_OK,
+            overlay_->GetMutableValue(overlay_key, &modified));
+  ASSERT_TRUE(modified);
+  ASSERT_TRUE(modified->GetType() == Value::TYPE_DICTIONARY);
+  EXPECT_TRUE(Value::Equals(modify, static_cast<DictionaryValue*>(modified)));
+}
+
+// Here we consider a global preference that is not overlayed.
+TEST_F(OverlayUserPrefStoreTest, GlobalPref) {
+  PrefStoreObserverMock obs;
+  overlay_->AddObserver(&obs);
+
+  const Value* value = NULL;
+  int i = -1;
+
+  // Check that underlay first value is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(1);
+  underlay_->SetValue(regular_key, Value::CreateIntegerValue(42));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that underlay overwriting is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(1);
+  underlay_->SetValue(regular_key, Value::CreateIntegerValue(43));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that we get this value from the overlay
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(regular_key, &value));
+  ASSERT_TRUE(value);

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

FundamentalValue.Equals() shortcut also works here.

[mnaganov (inactive), 2011/11/10 14:49:40]

Done.

+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+
+  // Check that overwriting change in overlay is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(1);
+  overlay_->SetValue(regular_key, Value::CreateIntegerValue(44));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that we get this value from the overlay and the underlay.
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(regular_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(44, i);
+  EXPECT_EQ(PrefStore::READ_OK, underlay_->GetValue(regular_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(44, i);
+
+  // Check that overlay remove is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(1);
+  overlay_->RemoveValue(regular_key);
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that value was removed from overlay and underlay
+  EXPECT_EQ(PrefStore::READ_NO_VALUE, overlay_->GetValue(regular_key, &value));
+  EXPECT_EQ(PrefStore::READ_NO_VALUE, underlay_->GetValue(regular_key, &value));
+
+  // Check respecting of silence.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(0);
+  overlay_->SetValueSilently(regular_key, Value::CreateIntegerValue(46));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  overlay_->RemoveObserver(&obs);
+
+  // Check successful unsubscription.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(regular_key))).Times(0);
+  underlay_->SetValue(regular_key, Value::CreateIntegerValue(47));
+  overlay_->SetValue(regular_key, Value::CreateIntegerValue(48));
+  Mock::VerifyAndClearExpectations(&obs);
+}
+
+// Check that names mapping works correctly.
+TEST_F(OverlayUserPrefStoreTest, NamesMapping) {
+  PrefStoreObserverMock obs;
+  overlay_->AddObserver(&obs);
+
+  const Value* value = NULL;
+  int i = -1;
+
+  // Check that if there is no override in the overlay, changing underlay value
+  // is reported as changing an overlay value.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(1);
+  underlay_->SetValue(mapped_underlay_key, Value::CreateIntegerValue(42));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that underlay overwriting is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(1);
+  underlay_->SetValue(mapped_underlay_key, Value::CreateIntegerValue(43));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that we get this value from the overlay with both keys
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(mapped_overlay_key, &value));
+  ASSERT_TRUE(value);

[Mattias Nissler (ping if slow), 2011/11/10 12:45:17]

FundamentalValue::Equals shortcut possible here (and below)

[mnaganov (inactive), 2011/11/10 14:49:40]

Done.

+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+  // In this case, overlay reads directly from the underlay.
+  EXPECT_EQ(PrefStore::READ_OK,
+            overlay_->GetValue(mapped_underlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+
+  // Check that overwriting change in overlay is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(1);
+  overlay_->SetValue(mapped_overlay_key, Value::CreateIntegerValue(44));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that we get an overriden value from overlay, while reading the
+  // value from underlay still holds an old value.
+  EXPECT_EQ(PrefStore::READ_OK, overlay_->GetValue(mapped_overlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(44, i);
+  EXPECT_EQ(PrefStore::READ_OK,
+            overlay_->GetValue(mapped_underlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+  EXPECT_EQ(PrefStore::READ_OK,
+            underlay_->GetValue(mapped_underlay_key, &value));
+  ASSERT_TRUE(value);
+  EXPECT_TRUE(value->GetAsInteger(&i));
+  EXPECT_EQ(43, i);
+
+  // Check that hidden underlay change is not reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(0);
+  underlay_->SetValue(mapped_underlay_key, Value::CreateIntegerValue(45));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that overlay remove is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(1);
+  overlay_->RemoveValue(mapped_overlay_key);
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that underlay remove is reported.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(1);
+  underlay_->RemoveValue(mapped_underlay_key);
+  Mock::VerifyAndClearExpectations(&obs);
+
+  // Check that value was removed.
+  EXPECT_EQ(PrefStore::READ_NO_VALUE,
+            overlay_->GetValue(mapped_overlay_key, &value));
+  EXPECT_EQ(PrefStore::READ_NO_VALUE,
+            overlay_->GetValue(mapped_underlay_key, &value));
+
+  // Check respecting of silence.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(0);
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_underlay_key))).Times(0);
+  overlay_->SetValueSilently(mapped_overlay_key, Value::CreateIntegerValue(46));
+  Mock::VerifyAndClearExpectations(&obs);
+
+  overlay_->RemoveObserver(&obs);
+
+  // Check successful unsubscription.
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_overlay_key))).Times(0);
+  EXPECT_CALL(obs, OnPrefValueChanged(StrEq(mapped_underlay_key))).Times(0);
+  underlay_->SetValue(mapped_underlay_key, Value::CreateIntegerValue(47));
+  overlay_->SetValue(mapped_overlay_key, Value::CreateIntegerValue(48));
+  Mock::VerifyAndClearExpectations(&obs);
+}