Fix for Bug 50726 "Save extension list and "winning" prefs from extensions"

chrome/browser/extensions/test_extension_prefs.cc

Index: chrome/browser/extensions/test_extension_prefs.cc
diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc
index 2cbb2d16857e1db7e58a60001988a81f1d76913d..69e96ee1d86cef0c38f6148fb1d89945318dba21 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -10,14 +10,76 @@
 #include "base/values.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/extensions/extension_prefs.h"
+#include "chrome/browser/extensions/extension_pref_store.h"
+#include "chrome/browser/prefs/browser_prefs.h"
+#include "chrome/browser/prefs/dummy_pref_store.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/pref_value_store.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/json_pref_store.h"
+#include "chrome/test/testing_pref_service.h"
+#include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TestExtensionPrefs::TestExtensionPrefs() {
+// Mock ExtensionPrefs class with artificial clock to guarantee that no two
+// extensions get the same installation time stamp and we can reliably
+// assert the installation order in the tests below.
+class MockExtensionPrefs : public ExtensionPrefs {
+ public:
+  MockExtensionPrefs(Profile* profile,
+                     PrefService* prefs,
+                     const FilePath& root_dir_)
+    : ExtensionPrefs(profile, prefs, root_dir_),
+      currentTime(base::Time::Now())
+  {}
+  ~MockExtensionPrefs() {}
+
+ protected:
+  mutable base::Time currentTime;
+
+  virtual base::Time GetCurrentTime() const {
+    currentTime += base::TimeDelta::FromSeconds(10);
+    return currentTime;
+  }
+};
+
+class TestingProfileWithFixedPath : public TestingProfile {
+ public:
+  explicit TestingProfileWithFixedPath(const FilePath& path) : path_(path) {}
+  virtual ~TestingProfileWithFixedPath() {}
+
+  virtual void CreateTestingPrefService() {
+    DCHECK(!prefs_.get());
+    PrefStore* managed_platform_prefs = new DummyPrefStore();
+    PrefStore* device_management_prefs = new DummyPrefStore();
+    PrefStore* extension_prefs = new ExtensionPrefStore(this);
+    PrefStore* command_line_prefs = NULL;
+    PrefStore* user_prefs = new JsonPrefStore(
+        path_.AppendASCII("Preferences"),
+        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
+    PrefStore* recommended_prefs = NULL;
+    PrefStore* default_pref = new DummyPrefStore();
+    testing_prefs_ = new TestingPrefService(managed_platform_prefs,
+                                            device_management_prefs,
+                                            extension_prefs,
+                                            command_line_prefs,
+                                            user_prefs,
+                                            recommended_prefs,
+                                            default_pref);
+    prefs_.reset(testing_prefs_);
+    Profile::RegisterUserPrefs(prefs_.get());
+    browser::RegisterAllPrefs(prefs_.get(), prefs_.get());
+  }
+
+  virtual FilePath GetPath() {
+    return path_;
+  }
+ private:
+  FilePath path_;
+};
+
+TestExtensionPrefs::TestExtensionPrefs() : pref_service_(NULL) {
   EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
   preferences_file_ = temp_dir_.path().AppendASCII("Preferences");
   extensions_dir_ = temp_dir_.path().AppendASCII("Extensions");
@@ -29,7 +91,7 @@ TestExtensionPrefs::TestExtensionPrefs() {
 TestExtensionPrefs::~TestExtensionPrefs() {}
 
 void TestExtensionPrefs::RecreateExtensionPrefs() {
-  if (pref_service_.get()) {
+  if (pref_service_) {
     // The PrefService writes its persistent file on the file thread, so we
     // need to wait for any pending I/O to complete before creating a new
     // PrefService.
@@ -39,10 +101,11 @@ void TestExtensionPrefs::RecreateExtensionPrefs() {
     file_loop.RunAllPending();
   }
 
-  // Create a |PrefService| instance that contains only user defined values.
-  pref_service_.reset(PrefService::CreateUserPrefService(preferences_file_));

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Why doesn't this work any longer? Anyway, these tests should verify that the
ExtensionPrefs reinitializes correctly, not that the PrefService is capable of
persisting values. So I think recreating the PrefService is really not necessary
here.

[battre (please use the other), 2010/12/01 17:44:38]

@Why doesn't this work any longer?
The PrefService is now part of the Profile.

@I think creating the PrefService is really not necessary here.
There are two possible choices:
a) remove the persistence and reloading of the PrefValueStore
b) keep it as it is.

The advantage of a) is that we have a stricter unit-test. We not not test the
persistence of PrefValueStores.

Unfortunately, persisting and reloading a PrefValueStore has a side effect:
every empty dictionary is discarded and the ExtensionPrefs need to be able to
deal with this. Therefore, we need to test it. If we go for a), we need to
implement pruning the PrefStores.

The advantage of b) is actually that we have this kind of integration testing
and can be sure that the side effects and changes therein are covered. I tend to
prefer b) while mnissler prefers a). I have addressed the issue with a comment.
Please let me know if you have a strong feeling for either a) or b).

-  ExtensionPrefs::RegisterUserPrefs(pref_service_.get());
-  prefs_.reset(new ExtensionPrefs(pref_service_.get(), temp_dir_.path()));
+  profile_.reset(new TestingProfileWithFixedPath(temp_dir_.path()));
+  pref_service_ = profile_->GetPrefs();
+  prefs_.reset(new MockExtensionPrefs(profile_.get(),
+                                      pref_service_,
+                                      temp_dir_.path()));
 }
 
 scoped_refptr<Extension> TestExtensionPrefs::AddExtension(std::string name) {