Introduce incognito preference settings.

chrome/browser/extensions/extension_pref_value_map_unittest.cc

+// 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/basictypes.h"
+#include "base/ref_counted.h"
+#include "base/scoped_ptr.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/extension_pref_value_map.h"
+#include "chrome/common/pref_store_observer_mock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+const char kExt1[] = "ext1";
+const char kExt2[] = "ext2";
+const char kExt3[] = "ext3";
+
+const char kPref1[] = "path1.subpath";
+const char kPref2[] = "path2";
+const char kPref3[] = "path3";
+const char kPref4[] = "path4";
+}  // namespace
+
+static Value* createVal(const char* str) {
+  return Value::CreateStringValue(str);
+}
+
+template <typename BASECLASS>
+class ExtensionPrefStoreTestBase : public BASECLASS {

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

I think the classes here should be named ExtensionPrefValueMapTest...

[battre, 2011/01/05 20:23:08]

Done.

+ public:
+  // Returns an empty string if the key is not set.
+  std::string GetValue(const char * key, bool incognito) const {
+    const Value *value = epvm_.GetEffectivePrefValue(key, incognito);
+    std::string string_value = "";
+    if (value)
+      value->GetAsString(&string_value);
+    return string_value;
+  }
+
+  static base::Time createTime(long t) {

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

can you sync createVal and createTime to the effect that both are either member
functions or not?

[battre, 2011/01/05 20:23:08]

Done.

+    return base::Time::FromInternalValue(10);
+  }
+
+ protected:
+  ExtensionPrefValueMap epvm_;
+};
+
+class ExtensionPrefStoreTest
+    : public ExtensionPrefStoreTestBase<testing::Test> {
+};
+
+template<typename BASECLASS>
+class ExtensionPrefStoreTestParameterized

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

Seems this class definition is not really helpful, just make
ExtensionPrefStoreTestIncognitoTest derive directly from
ExtensionPrefStoreTestBase.

[battre, 2011/01/05 20:23:08]

Done.

+    : public ExtensionPrefStoreTestBase<testing::TestWithParam<BASECLASS> > {
+};
+
+TEST_F(ExtensionPrefStoreTest, SetAndGetPrefValue) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  EXPECT_EQ("val1", GetValue(kPref1, false));
+};
+
+TEST_F(ExtensionPrefStoreTest, GetNotSetPrefValue) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  EXPECT_EQ("", GetValue(kPref1, false));
+};
+
+// Make sure the last-installed extension wins for each preference.
+TEST_F(ExtensionPrefStoreTest, Overwrite) {

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

nit: name this Override

[battre, 2011/01/05 20:23:08]

Done.

+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(20), true);
+  epvm_.RegisterExtension(kExt3, createTime(30), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.SetExtensionPref(kExt2, kPref1, false, createVal("val2"));
+  epvm_.SetExtensionPref(kExt3, kPref1, false, createVal("val3"));
+
+  epvm_.SetExtensionPref(kExt1, kPref2, false, createVal("val4"));
+  epvm_.SetExtensionPref(kExt2, kPref2, false, createVal("val5"));
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val6"));
+  epvm_.SetExtensionPref(kExt1, kPref2, false, createVal("val7"));
+  epvm_.SetExtensionPref(kExt1, kPref3, false, createVal("val8"));
+
+  EXPECT_EQ("val3", GetValue(kPref1, false));
+  EXPECT_EQ("val5", GetValue(kPref2, false));
+  EXPECT_EQ("val8", GetValue(kPref3, false));
+}
+
+TEST_F(ExtensionPrefStoreTest, SetAndGetPrefValueIncognito) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  EXPECT_EQ("val1", GetValue(kPref1, true));
+}
+
+struct OverwriteIncognitoTestCase {
+  OverwriteIncognitoTestCase(int val_ext1_regular,
+                             int val_ext1_incognito,
+                             int val_ext2_regular,
+                             int val_ext2_incognito,
+                             int effective_value_regular,
+                             int effective_value_incognito)
+      : val_ext1_regular_(val_ext1_regular),
+        val_ext1_incognito_(val_ext1_incognito),
+        val_ext2_regular_(val_ext2_regular),
+        val_ext2_incognito_(val_ext2_incognito),
+        effective_value_regular_(effective_value_regular),
+        effective_value_incognito_(effective_value_incognito) {}
+
+  int val_ext1_regular_;           // pref value of extension 1
+  int val_ext1_incognito_;         // pref value of extension 1 incognito
+  int val_ext2_regular_;           // pref value of extension 2
+  int val_ext2_incognito_;         // pref value of extension 2 incognito
+  int effective_value_regular_;    // desired winner regular
+  int effective_value_incognito_;  // desired winner incognito
+};
+
+class ExtensionPrefStoreTestIncognitoTests

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

Can you move this test fixture down below all the other tests in the regular
fixture? I find it easier to make sense of the file if each fixture is in its
own block.

[battre, 2011/01/05 20:23:08]

Done.

+    : public ExtensionPrefStoreTestParameterized<OverwriteIncognitoTestCase> {
+};
+
+TEST_P(ExtensionPrefStoreTestIncognitoTests, OverwriteIncognito) {
+  OverwriteIncognitoTestCase test = GetParam();
+  const char* strings[] = {
+      "undefined",
+      "val1",
+      "val2",
+      "val3",
+      "val4"
+  };
+
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(20), true);
+  if (test.val_ext1_regular_) {
+    epvm_.SetExtensionPref(kExt1, kPref1, false,
+                           createVal(strings[test.val_ext1_regular_]));
+  }
+  if (test.val_ext1_incognito_) {
+    epvm_.SetExtensionPref(kExt1, kPref1, true,
+                           createVal(strings[test.val_ext1_incognito_]));
+  }
+  if (test.val_ext2_regular_) {
+    epvm_.SetExtensionPref(kExt2, kPref1, false,
+                           createVal(strings[test.val_ext2_regular_]));
+  }
+  if (test.val_ext2_incognito_) {
+    epvm_.SetExtensionPref(kExt2, kPref1, true,
+                           createVal(strings[test.val_ext2_incognito_]));
+  }
+  std::string actual;
+  EXPECT_EQ(strings[test.effective_value_regular_], GetValue(kPref1, false));
+  EXPECT_EQ(strings[test.effective_value_incognito_], GetValue(kPref1, true));
+  epvm_.UnregisterExtension(kExt1);
+  epvm_.UnregisterExtension(kExt2);
+}
+
+INSTANTIATE_TEST_CASE_P(
+    ExtensionPrefStoreTestIncognitoTestsInstance,
+    ExtensionPrefStoreTestIncognitoTests,
+    testing::Values(
+        // e.g. (1, 0, 0, 4,  1, 4), means:
+        // ext1 regular is set to "val1", ext2 incognito is set to "val4"
+        // --> the winning regular value is "val1", the winning incognito
+        //     value is "val4".
+        OverwriteIncognitoTestCase(1, 0, 0, 0,  1, 1),
+        OverwriteIncognitoTestCase(1, 2, 0, 0,  1, 2),
+        OverwriteIncognitoTestCase(1, 0, 3, 0,  3, 3),
+        OverwriteIncognitoTestCase(1, 0, 0, 4,  1, 4),
+        // The last 3 in the following line is intentional!
+        OverwriteIncognitoTestCase(1, 2, 3, 0,  3, 3),
+        OverwriteIncognitoTestCase(1, 2, 0, 4,  1, 4),
+        OverwriteIncognitoTestCase(1, 2, 3, 4,  3, 4)
+    ));
+
+// Make sure the last-installed extension wins even if other extensions set
+// the same or different preferences later.
+TEST_F(ExtensionPrefStoreTest, OverrideInterleaved) {

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

Does this really give us additional coverage over Override? Can we perhaps test
all of it in one test case?

[battre, 2011/01/05 20:23:08]

This is mostly legacy from the old unit tests. It does not increase coverage.

Deleted.

+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(20), true);
+  epvm_.RegisterExtension(kExt3, createTime(30), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.SetExtensionPref(kExt2, kPref2, false, createVal("val2"));
+  epvm_.SetExtensionPref(kExt3, kPref3, false, createVal("val3"));
+
+  epvm_.SetExtensionPref(kExt3, kPref3, false, createVal("val4"));
+  epvm_.SetExtensionPref(kExt2, kPref3, false, createVal("val5"));
+  epvm_.SetExtensionPref(kExt1, kPref3, false, createVal("val6"));
+
+  epvm_.SetExtensionPref(kExt3, kPref1, false, createVal("val7"));
+
+  EXPECT_EQ("val7", GetValue(kPref1, false));
+  EXPECT_EQ("val2", GetValue(kPref2, false));
+  EXPECT_EQ("val4", GetValue(kPref3, false));
+  EXPECT_EQ("val7", GetValue(kPref1, true));
+  EXPECT_EQ("val2", GetValue(kPref2, true));
+  EXPECT_EQ("val4", GetValue(kPref3, true));
+}
+
+TEST_F(ExtensionPrefStoreTest, UninstallOnlyExtension) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.UnregisterExtension(kExt1);
+
+  EXPECT_EQ("", GetValue(kPref1, false));
+}
+
+// Tests uninstalling an extension that wasn't winning for any preferences.
+TEST_F(ExtensionPrefStoreTest, UninstallIrrelevantExtension) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(10), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.SetExtensionPref(kExt2, kPref1, false, createVal("val2"));
+
+  epvm_.SetExtensionPref(kExt1, kPref2, false, createVal("val3"));
+  epvm_.SetExtensionPref(kExt2, kPref2, false, createVal("val4"));
+
+  epvm_.UnregisterExtension(kExt1);
+
+  EXPECT_EQ("val2", GetValue(kPref1, false));
+  EXPECT_EQ("val4", GetValue(kPref2, false));
+}
+
+// Tests uninstalling an extension that was winning for all preferences.
+TEST_F(ExtensionPrefStoreTest, UninstallExtensionFromTop) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(20), true);
+  epvm_.RegisterExtension(kExt3, createTime(30), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.SetExtensionPref(kExt2, kPref1, false, createVal("val2"));
+  epvm_.SetExtensionPref(kExt3, kPref1, false, createVal("val3"));
+
+  epvm_.SetExtensionPref(kExt1, kPref2, false, createVal("val4"));
+  epvm_.SetExtensionPref(kExt3, kPref2, false, createVal("val5"));
+
+  epvm_.UnregisterExtension(kExt3);
+
+  EXPECT_EQ("val2", GetValue(kPref1, false));
+  EXPECT_EQ("val4", GetValue(kPref2, false));
+}
+
+// Tests uninstalling an extension that was winning for only some preferences.
+TEST_F(ExtensionPrefStoreTest, UninstallExtensionFromMiddle) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.RegisterExtension(kExt2, createTime(20), true);
+  epvm_.RegisterExtension(kExt3, createTime(30), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.SetExtensionPref(kExt2, kPref1, false, createVal("val2"));
+  epvm_.SetExtensionPref(kExt3, kPref1, false, createVal("val3"));
+
+  epvm_.SetExtensionPref(kExt1, kPref2, false, createVal("val4"));
+  epvm_.SetExtensionPref(kExt2, kPref2, false, createVal("val5"));
+
+  epvm_.SetExtensionPref(kExt1, kPref3, false, createVal("val6"));
+
+  epvm_.SetExtensionPref(kExt2, kPref4, false, createVal("val7"));
+
+  epvm_.UnregisterExtension(kExt2);
+
+  EXPECT_EQ("val3", GetValue(kPref1, false));
+  EXPECT_EQ("val4", GetValue(kPref2, false));
+  EXPECT_EQ("val6", GetValue(kPref3, false));
+  EXPECT_EQ("", GetValue(kPref4, false));
+}
+
+// Tests triggering of notifications to registered observers.
+TEST_F(ExtensionPrefStoreTest, NotifyWhenNeeded) {
+  using testing::_;
+  using testing::Mock;
+  using testing::StrEq;
+
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+
+  PrefStoreObserverMock observer;
+  epvm_.AddObserver(&observer);
+
+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Write the same value again

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

Comment missing period (also below).

[battre, 2011/01/05 20:23:08]

Done.

+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1))).Times(0);
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Overwrite incognito value

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

nit: I think you should decide on either overwrite or override (I prefer the
latter) and be consistent.

[battre, 2011/01/05 20:23:08]

Done.

+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.SetExtensionPref(kExt1, kPref1, true, createVal("val2"));
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Overwrite non-incognito value
+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val3"));
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Disable
+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.UpdateExtensionsState(kExt1, false);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Enable
+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.UpdateExtensionsState(kExt1, true);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  // Uninstall
+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1)));
+  epvm_.UnregisterExtension(kExt1);
+  Mock::VerifyAndClearExpectations(&observer);
+
+  epvm_.RemoveObserver(&observer);
+
+  // Write new value --> no notification ofter removing observer

[Mattias Nissler (ping if slow), 2011/01/05 12:08:07]

s/ofter/after/

[battre, 2011/01/05 20:23:08]

Done.

+  EXPECT_CALL(observer, OnPrefValueChanged(std::string(kPref1))).Times(0);
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val4"));
+  Mock::VerifyAndClearExpectations(&observer);
+}
+
+// Tests disabling an extension
+TEST_F(ExtensionPrefStoreTest, DisableExt) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.UpdateExtensionsState(kExt1, false);
+  EXPECT_EQ("", GetValue(kPref1, false));
+}
+
+// Tests disabling and reenabling an extension
+TEST_F(ExtensionPrefStoreTest, ReenableExt) {
+  epvm_.RegisterExtension(kExt1, createTime(10), true);
+
+  epvm_.SetExtensionPref(kExt1, kPref1, false, createVal("val1"));
+  epvm_.UpdateExtensionsState(kExt1, false);
+  epvm_.UpdateExtensionsState(kExt1, true);
+  EXPECT_EQ("val1", GetValue(kPref1, false));
+}