Introduce incognito preference settings.

chrome/browser/extensions/extension_prefs_unittest.cc

Index: chrome/browser/extensions/extension_prefs_unittest.cc
diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc
index 6b59740e9307d2c7cc2d2814145b42124ba4df47..edf454574e7c16429cafa3d52e7f7cfc7bb7b314 100644
--- a/chrome/browser/extensions/extension_prefs_unittest.cc
+++ b/chrome/browser/extensions/extension_prefs_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -22,6 +22,7 @@
 using base::Time;
 using base::TimeDelta;
 
+namespace {
 const char kPref1[] = "path1.subpath";
 const char kPref2[] = "path2";
 const char kPref3[] = "path3";
@@ -32,6 +33,7 @@ const char kDefaultPref1[] = "default pref 1";
 const char kDefaultPref2[] = "default pref 2";
 const char kDefaultPref3[] = "default pref 3";
 const char kDefaultPref4[] = "default pref 4";
+}
 
 static void AddPattern(ExtensionExtent* extent, const std::string& pattern) {
   int schemes = URLPattern::SCHEME_ALL;
@@ -591,20 +593,31 @@ class ExtensionPrefsPreferencesBase : public ExtensionPrefsTest {
   void InstallExtControlledPref(Extension *ext,
                                 const std::string& key,
                                 Value* val) {
-    // Install extension the first time a preference is set for it.
+    EnsureExtensionInstalled(ext);
+    const bool incognito = false;

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

that's a very elaborate way of saying false below :)

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

I got the same comment from Danno, so I will change it. I don't agree though. I
don't like passing boolean values to a function as they are somewhat untyped.
You cannot parse the meaning of false in "SetExtensionControlledPref(ext->id(),
key, false, val)" without lookup up the definition of
SetExtensionControlledPref.

In Java it is good practice to cast null to the correct type (even though it
does not make any difference at all) to improve readability.

Done.

+    prefs()->SetExtensionControlledPref(ext->id(), key, incognito, val);
+  }
+
+  void InstallExtControlledPrefIncognito(Extension *ext,
+                                const std::string& key,

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

indentation

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

Done.

+                                Value* val) {
+    EnsureExtensionInstalled(ext);
+    const bool incognito = true;

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

same here, just inline.

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

Done.

+    prefs()->SetExtensionControlledPref(ext->id(), key, incognito, val);
+  }
+
+  void InstallExtension(Extension *ext) {
+    EnsureExtensionInstalled(ext);
+  }
+
+  void UninstallExtension(const std::string& extension_id) {
     Extension* extensions[] = {ext1_, ext2_, ext3_};
     for (int i = 0; i < 3; ++i) {
-      if (ext == extensions[i] && !installed[i]) {
-        prefs()->OnExtensionInstalled(ext, Extension::ENABLED, true);
-        installed[i] = true;
+      if (extensions[i]->id() == extension_id) {
+        installed[i] = false;
         break;
       }
     }
-
-    prefs()->SetExtensionControlledPref(ext->id(), key, val);
-  }
-
-  void UninstallExtension(const std::string& extension_id) {
     prefs()->OnExtensionUninstalled(extension_id, Extension::INTERNAL, false);
   }
 
@@ -617,6 +630,18 @@ class ExtensionPrefsPreferencesBase : public ExtensionPrefsTest {
   bool installed[3];
 
  private:
+  void EnsureExtensionInstalled(Extension *ext) {

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

Why not just put this code into InstallExtension?

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

Because InstallExtensionControlled{Incognito}Pref need to ensure that the
extension is installed. They shall not install one each time a preference value
is installed for an extension. At the same time we sometimes need to just
install an extension w/o setting any pref values, i.e. we need an
InstallExtension function.

Moving this code into InstallExtension would not reflect what it does.

For symmetry reasons I have moved the code of UninstallExtension into
EnsureExtensionUinstalled.

+    // Install extension the first time a preference is set for it.
+    Extension* extensions[] = {ext1_, ext2_, ext3_};
+    for (int i = 0; i < 3; ++i) {
+      if (ext == extensions[i] && !installed[i]) {
+        prefs()->OnExtensionInstalled(ext, Extension::ENABLED, true);
+        installed[i] = true;
+        break;
+      }
+    }
+  }
+
   scoped_refptr<Extension> ext1_scoped_;
   scoped_refptr<Extension> ext2_scoped_;
   scoped_refptr<Extension> ext3_scoped_;
@@ -634,6 +659,39 @@ class ExtensionPrefsInstallOneExtension
 };

[battre, 2011/01/05 09:59:40]

In the following code there is quite some overlap with the new
extension_pref_value_map_unittest.cc. Yet I see the need for this kind of
integration testing. What do you think?

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

Yes, we need the integration tests, but they don't need to check for each corner
case, but only if the operations on ExtensionPrefs do show up in the
ExtensionPrefValueMap (which is easy to check if we make that a dependency we
pass to ExtensionPrefs at construction time btw).

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

Done.

 TEST_F(ExtensionPrefsInstallOneExtension, ExtensionPrefsInstallOneExtension) {}
 
+// Check that we forget incognito values after a reload.
+class ExtensionPrefsInstallIncognito
+    : public ExtensionPrefsPreferencesBase {
+ public:
+  ExtensionPrefsInstallIncognito() : iteration_(0) {}
+
+  virtual void Initialize() {
+    InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
+    InstallExtControlledPrefIncognito(ext1_, kPref1,
+                                      Value::CreateStringValue("val2"));
+    scoped_ptr<PrefService> incog_prefs(prefs()->CreateIncognitoPrefService());
+    std::string actual = incog_prefs->GetString(kPref1);
+    EXPECT_EQ("val2", actual);
+  }
+  virtual void Verify() {
+    // Main pref service shall see only non-incognito settings.
+    std::string actual = prefs()->pref_service()->GetString(kPref1);
+    EXPECT_EQ("val1", actual);
+    // Incognito pref service shall see incognito values only during first run.
+    // Once the pref service was reloaded, all values shall be discarded.
+    scoped_ptr<PrefService> incog_prefs(prefs()->CreateIncognitoPrefService());
+    actual = incog_prefs->GetString(kPref1);
+    if (iteration_ == 0) {
+      EXPECT_EQ("val2", actual);
+    } else {
+      EXPECT_EQ("val1", actual);
+    }
+    ++iteration_;
+  }
+  int iteration_;
+};
+TEST_F(ExtensionPrefsInstallIncognito, ExtensionPrefsInstallOneExtension) {}
+
 // Make sure the last-installed extension wins for each preference.
 class ExtensionPrefsInstallOverwrittenExtensions
     : public ExtensionPrefsPreferencesBase {
@@ -662,6 +720,80 @@ class ExtensionPrefsInstallOverwrittenExtensions
 TEST_F(ExtensionPrefsInstallOverwrittenExtensions,
     ExtensionPrefsInstallOverwrittenExtensions) {}
 
+// Make sure the last-installed extension wins for each preference.
+class ExtensionPrefsInstallOverwrittenExtensionsIncognito
+    : public ExtensionPrefsPreferencesBase {
+
+  struct TestCase {
+    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
+  };
+
+  virtual void Initialize() {
+    const char* strings[] = {
+        kDefaultPref1,
+        "val1",
+        "val2",
+        "val3",
+        "val4"
+    };
+    TestCase 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".
+      { 1, 0, 0, 0,  1, 1},
+      { 1, 2, 0, 0,  1, 2},
+      { 1, 0, 3, 0,  3, 3},
+      { 1, 0, 0, 4,  1, 4},
+      { 1, 2, 3, 0,  3, 3},  // The last 3 here is intentional!
+      { 1, 2, 0, 4,  1, 4},
+      { 1, 2, 3, 4,  3, 4}
+    };
+    for (size_t row = 0; row < arraysize(values); ++row) {
+      LOG(INFO) << "Testing row " << row;
+      InstallExtension(ext1_);
+      InstallExtension(ext2_);
+      if (values[row].val_ext1_regular_) {
+        InstallExtControlledPref(
+            ext1_, kPref1,
+            Value::CreateStringValue(strings[values[row].val_ext1_regular_]));
+      }
+      if (values[row].val_ext1_incognito_) {
+        InstallExtControlledPrefIncognito(
+            ext1_, kPref1,
+            Value::CreateStringValue(strings[values[row].val_ext1_incognito_]));
+      }
+      if (values[row].val_ext2_regular_) {
+        InstallExtControlledPref(
+            ext2_, kPref1,
+            Value::CreateStringValue(strings[values[row].val_ext2_regular_]));
+      }
+      if (values[row].val_ext2_incognito_) {
+        InstallExtControlledPrefIncognito(
+            ext2_, kPref1,
+            Value::CreateStringValue(strings[values[row].val_ext2_incognito_]));
+      }
+      std::string actual;
+      actual = prefs()->pref_service()->GetString(kPref1);
+      EXPECT_EQ(strings[values[row].effective_value_regular_], actual);
+      scoped_ptr<PrefService> incog_prefs(
+          prefs()->CreateIncognitoPrefService());
+      actual = incog_prefs->GetString(kPref1);
+      EXPECT_EQ(strings[values[row].effective_value_incognito_], actual);
+      UninstallExtension(ext1_->id());
+      UninstallExtension(ext2_->id());
+    }
+  }
+  virtual void Verify() {}
+};
+TEST_F(ExtensionPrefsInstallOverwrittenExtensionsIncognito,
+    ExtensionPrefsInstallOverwrittenExtensionsIncognito) {}
+
 // Make sure the last-installed extension wins even if other extensions set
 // the same or different preferences later.
 class ExtensionPrefsInstallInterleavedExtensions
@@ -678,13 +810,21 @@ class ExtensionPrefsInstallInterleavedExtensions
     InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val7"));
   }
   virtual void Verify() {
+    scoped_ptr<PrefService> incog_prefs(
+        prefs()->CreateIncognitoPrefService());
     std::string actual;
     actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val7", actual);
+    actual = incog_prefs->GetString(kPref1);
+    EXPECT_EQ("val7", actual);
     actual = prefs()->pref_service()->GetString(kPref2);
     EXPECT_EQ("val2", actual);
+    actual = incog_prefs->GetString(kPref2);
+    EXPECT_EQ("val2", actual);
     actual = prefs()->pref_service()->GetString(kPref3);
     EXPECT_EQ("val4", actual);
+    actual = incog_prefs->GetString(kPref3);
+    EXPECT_EQ("val4", actual);
   }
 };
 TEST_F(ExtensionPrefsInstallInterleavedExtensions,
@@ -735,23 +875,45 @@ TEST_F(ExtensionPrefsUninstallIrrelevantExtension,
 // Tests uninstalling an extension that was winning for all preferences.
 class ExtensionPrefsUninstallExtensionFromTop
     : public ExtensionPrefsPreferencesBase {
+ public:
+  ExtensionPrefsUninstallExtensionFromTop() : iteration_(0) {}
+
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
+    InstallExtControlledPrefIncognito(ext1_, kPref1,
+                                      Value::CreateStringValue("val1i"));
     InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2"));
+    InstallExtControlledPrefIncognito(ext2_, kPref1,
+                                      Value::CreateStringValue("val2i"));
     InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val3"));
+    InstallExtControlledPrefIncognito(ext3_, kPref1,
+                                      Value::CreateStringValue("val3i"));
 
     InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val4"));
+    InstallExtControlledPrefIncognito(ext1_, kPref2,
+                                      Value::CreateStringValue("val4i"));
     InstallExtControlledPref(ext3_, kPref2, Value::CreateStringValue("val5"));
+    InstallExtControlledPrefIncognito(ext3_, kPref2,
+                                      Value::CreateStringValue("val5i"));
 
     UninstallExtension(ext3_->id());
   }
   virtual void Verify() {
+    scoped_ptr<PrefService> incog_prefs(
+        prefs()->CreateIncognitoPrefService());
     std::string actual;
     actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val2", actual);
     actual = prefs()->pref_service()->GetString(kPref2);
     EXPECT_EQ("val4", actual);
+    // After the reload (iteration_ == 1) all incognito settings are forgotten.
+    actual = incog_prefs->GetString(kPref1);
+    EXPECT_EQ((iteration_ == 0 ? "val2i" : "val2"), actual);
+    actual = incog_prefs->GetString(kPref2);
+    EXPECT_EQ((iteration_ == 0 ? "val4i" : "val4"), actual);
+    ++iteration_;
   }
+  int iteration_;
 };
 TEST_F(ExtensionPrefsUninstallExtensionFromTop,
     ExtensionPrefsUninstallExtensionFromTop) {}
@@ -788,7 +950,7 @@ class ExtensionPrefsUninstallExtensionFromMiddle
 TEST_F(ExtensionPrefsUninstallExtensionFromMiddle,
     ExtensionPrefsUninstallExtensionFromMiddle) {}
 
-// Tests triggering of notifications to registered observers
+// Tests triggering of notifications to registered observers.
 class ExtensionPrefsNotifyWhenNeeded
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
@@ -827,7 +989,7 @@ class ExtensionPrefsNotifyWhenNeeded
 TEST_F(ExtensionPrefsNotifyWhenNeeded,
     ExtensionPrefsNotifyWhenNeeded) {}
 
-// Tests disabling an extension
+// Tests disabling an extension.
 class ExtensionPrefsDisableExt
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
@@ -843,7 +1005,7 @@ class ExtensionPrefsDisableExt
 };
 TEST_F(ExtensionPrefsDisableExt,  ExtensionPrefsDisableExt) {}
 
-// Tests disabling and reenabling an extension
+// Tests disabling and reenabling an extension.
 class ExtensionPrefsReenableExt
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
@@ -876,15 +1038,27 @@ class ExtensionPrefsSetExtensionControlledPref
   virtual void Initialize() {
     MockStringValue* v1 = new MockStringValue("https://www.chromium.org");
     MockStringValue* v2 = new MockStringValue("https://www.chromium.org");
+    MockStringValue* v1i = new MockStringValue("https://www.chromium.org");
+    MockStringValue* v2i = new MockStringValue("https://www.chromium.org");
     // Ownership is taken, value shall not be deleted.
     EXPECT_CALL(*v1, Die()).Times(0);
+    EXPECT_CALL(*v1i, Die()).Times(0);
     InstallExtControlledPref(ext1_, kPref1, v1);
+    InstallExtControlledPrefIncognito(ext1_, kPref1, v1i);
     testing::Mock::VerifyAndClearExpectations(v1);
+    testing::Mock::VerifyAndClearExpectations(v1i);
     // Make sure there is no memory leak and both values are deleted.
-    EXPECT_CALL(*v2, Die()).Times(1);
     EXPECT_CALL(*v1, Die()).Times(1);
+    EXPECT_CALL(*v1i, Die()).Times(1);
+    EXPECT_CALL(*v2, Die()).Times(1);
+    EXPECT_CALL(*v2i, Die()).Times(1);
     InstallExtControlledPref(ext1_, kPref1, v2);
+    InstallExtControlledPrefIncognito(ext1_, kPref1, v2i);
     prefs_.RecreateExtensionPrefs();
+    testing::Mock::VerifyAndClearExpectations(v1);
+    testing::Mock::VerifyAndClearExpectations(v1i);
+    testing::Mock::VerifyAndClearExpectations(v2);
+    testing::Mock::VerifyAndClearExpectations(v2i);
   }
 
   virtual void Verify() {