Introduce incognito preference settings.

chrome/browser/extensions/extension_prefs_unittest.cc

-// 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.
 
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/stl_util-inl.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/extensions/extension_prefs.h"
 #include "chrome/browser/extensions/test_extension_prefs.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/notification_details.h"
 #include "chrome/common/notification_observer_mock.h"
 #include "chrome/common/notification_source.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::Time;
 using base::TimeDelta;
 
+namespace {
 const char kPref1[] = "path1.subpath";
 const char kPref2[] = "path2";
 const char kPref3[] = "path3";
 const char kPref4[] = "path4";
 
 // Default values in case an extension pref value is not overridden.
 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;
   extent->AddPattern(URLPattern(schemes, pattern));
 }
 
 static void AssertEqualExtents(ExtensionExtent* extent1,
                                ExtensionExtent* extent2) {
   std::vector<URLPattern> patterns1 = extent1->patterns();
   std::vector<URLPattern> patterns2 = extent2->patterns();
@@ skipping 539 matching lines @@
   void RegisterPreferences() {
     prefs()->pref_service()->RegisterStringPref(kPref1, kDefaultPref1);
     prefs()->pref_service()->RegisterStringPref(kPref2, kDefaultPref2);
     prefs()->pref_service()->RegisterStringPref(kPref3, kDefaultPref3);
     prefs()->pref_service()->RegisterStringPref(kPref4, kDefaultPref4);
   }
 
   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);
   }
 
   // Weak references, for convenience.
   Extension* ext1_;
   Extension* ext2_;
   Extension* ext3_;
 
   // Flags indicating whether each of the extensions has been installed, yet.
   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_;
 };
 
 class ExtensionPrefsInstallOneExtension
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
   }
   virtual void Verify() {
     std::string actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val1", actual);
   }
 };

[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 {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2"));
     InstallExtControlledPref(ext3_, kPref1, Value::CreateStringValue("val3"));
 
     InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val4"));
     InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val5"));
 
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val6"));
     InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val7"));
     InstallExtControlledPref(ext1_, kPref3, Value::CreateStringValue("val8"));
   }
   virtual void Verify() {
     std::string actual;
     actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val3", actual);
     actual = prefs()->pref_service()->GetString(kPref2);
     EXPECT_EQ("val5", actual);
     actual = prefs()->pref_service()->GetString(kPref3);
     EXPECT_EQ("val8", actual);
   }
 };
 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
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     InstallExtControlledPref(ext2_, kPref2, Value::CreateStringValue("val2"));
     InstallExtControlledPref(ext3_, kPref3, Value::CreateStringValue("val3"));
 
     InstallExtControlledPref(ext3_, kPref3, Value::CreateStringValue("val4"));
     InstallExtControlledPref(ext2_, kPref3, Value::CreateStringValue("val5"));
     InstallExtControlledPref(ext1_, kPref3, Value::CreateStringValue("val6"));
 
     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,
     ExtensionPrefsInstallInterleavedExtensions) {}
 
 class ExtensionPrefsUninstallOnlyExtension
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     InstallExtControlledPref(ext1_, kPref2, Value::CreateStringValue("val2"));
@@ skipping 30 matching lines @@
     actual = prefs()->pref_service()->GetString(kPref2);
     EXPECT_EQ("val4", actual);
   }
 };
 TEST_F(ExtensionPrefsUninstallIrrelevantExtension,
     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) {}
 
 // Tests uninstalling an extension that was winning for only some preferences.
 class ExtensionPrefsUninstallExtensionFromMiddle
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     InstallExtControlledPref(ext2_, kPref1, Value::CreateStringValue("val2"));
@@ skipping 16 matching lines @@
     EXPECT_EQ("val4", actual);
     actual = prefs()->pref_service()->GetString(kPref3);
     EXPECT_EQ("val6", actual);
     actual = prefs()->pref_service()->GetString(kPref4);
     EXPECT_EQ(kDefaultPref4, actual);
   }
 };
 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() {
     using testing::_;
     using testing::Mock;
     using testing::StrEq;
 
     scoped_ptr<NotificationObserverMock> observer(
         new NotificationObserverMock());
     PrefChangeRegistrar registrar;
@@ skipping 18 matching lines @@
     registrar.Remove(kPref1, observer.get());
   }
   virtual void Verify() {
     std::string actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ(kDefaultPref1, actual);
   }
 };
 TEST_F(ExtensionPrefsNotifyWhenNeeded,
     ExtensionPrefsNotifyWhenNeeded) {}
 
-// Tests disabling an extension
+// Tests disabling an extension.
 class ExtensionPrefsDisableExt
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     std::string actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val1", actual);
     prefs()->SetExtensionState(ext1_, Extension::DISABLED);
   }
   virtual void Verify() {
     std::string actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ(kDefaultPref1, actual);
   }
 };
 TEST_F(ExtensionPrefsDisableExt,  ExtensionPrefsDisableExt) {}
 
-// Tests disabling and reenabling an extension
+// Tests disabling and reenabling an extension.
 class ExtensionPrefsReenableExt
     : public ExtensionPrefsPreferencesBase {
   virtual void Initialize() {
     InstallExtControlledPref(ext1_, kPref1, Value::CreateStringValue("val1"));
     prefs()->SetExtensionState(ext1_, Extension::DISABLED);
     prefs()->SetExtensionState(ext1_, Extension::ENABLED);
   }
   virtual void Verify() {
     std::string actual = prefs()->pref_service()->GetString(kPref1);
     EXPECT_EQ("val1", actual);
@@ skipping 12 matching lines @@
   }
   MOCK_METHOD0(Die, void());
 };
 
 class ExtensionPrefsSetExtensionControlledPref
     : public ExtensionPrefsPreferencesBase {
  public:
   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(*v1, Die()).Times(1);
+    EXPECT_CALL(*v1i, Die()).Times(1);
     EXPECT_CALL(*v2, Die()).Times(1);
-    EXPECT_CALL(*v1, 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() {
   }
 };
 TEST_F(ExtensionPrefsSetExtensionControlledPref,
     ExtensionPrefsSetExtensionControlledPref) {}