ABANDONED: Remove Chrome dependencies from TestExtensionPrefs.

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 c14320cf96a8258b0dbc79ce571a60199830a90b..95007b060297d86f82e211cd5a505cf8da4f383a 100644
--- a/chrome/browser/extensions/test_extension_prefs.cc
+++ b/chrome/browser/extensions/test_extension_prefs.cc
@@ -11,7 +11,10 @@
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/prefs/json_pref_store.h"
+#include "base/prefs/pref_service.h"
+#include "base/prefs/pref_service_factory.h"
 #include "base/prefs/pref_value_store.h"
+#include "base/prefs/testing_pref_store.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
 #include "base/synchronization/waitable_event.h"
@@ -19,8 +22,6 @@
 #include "chrome/browser/extensions/extension_pref_store.h"
 #include "chrome/browser/extensions/extension_pref_value_map.h"
 #include "chrome/browser/extensions/extension_prefs.h"
-#include "chrome/browser/prefs/pref_service_mock_factory.h"
-#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "components/user_prefs/pref_registry_syncable.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extensions_browser_client.h"
@@ -58,8 +59,14 @@ class IncrementalTimeProvider : public ExtensionPrefs::TimeProvider {
 
 }  // namespace
 
-TestExtensionPrefs::TestExtensionPrefs(base::SequencedTaskRunner* task_runner)
-    : task_runner_(task_runner), extensions_disabled_(false) {
+TestExtensionPrefs::TestExtensionPrefs(
+    base::SequencedTaskRunner* task_runner,
+    base::PrefServiceFactory* custom_pref_service_factory)
+    : pref_service_factory_(custom_pref_service_factory ?
+                            custom_pref_service_factory :
+                            new base::PrefServiceFactory),

[miket_OOO, 2013/12/06 20:42:03]

After offline discussion: it looks like you've expanded the
PrefServiceMockFactory here into a real one that does the same work that test
class was doing (principally setting user prefs to a new TestingPrefStore). This
was the easiest path because of an ongoing PrefServiceSyncable dependency. This
is all fine, just wanted something in print here explaining why the mock seems
to have disappeared (and this comment constitutes that "something in print").

[Daniel Erat, 2013/12/06 20:47:55]

Just to elaborate: PrefServiceMockFactory doesn't actually appear to be a mock.
It derives from PrefServiceSyncableFactory with no differences apart from a
"user_prefs_ = new TestingPrefStore;" in its constructor. That's the reason for
the set_user_prefs() call that I added below.

+      task_runner_(task_runner),
+      extensions_disabled_(false) {
   EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
   preferences_file_ = temp_dir_.path().AppendASCII("Preferences");
   extensions_dir_ = temp_dir_.path().AppendASCII("Extensions");
@@ -104,11 +111,12 @@ void TestExtensionPrefs::RecreateExtensionPrefs() {
   }
 
   extension_pref_value_map_.reset(new ExtensionPrefValueMap);
-  PrefServiceMockFactory factory;
-  factory.SetUserPrefsFile(preferences_file_, task_runner_.get());
-  factory.set_extension_prefs(
+  pref_service_factory_->SetUserPrefsFile(preferences_file_,
+                                          task_runner_.get());
+  pref_service_factory_->set_user_prefs(new TestingPrefStore);
+  pref_service_factory_->set_extension_prefs(
       new ExtensionPrefStore(extension_pref_value_map_.get(), false));
-  pref_service_ = factory.CreateSyncable(pref_registry_.get()).Pass();

[Daniel Erat, 2013/12/06 21:01:17]

ugh, i think i see the problem. i changed this CreateSyncable() call to
Create(). PrefServiceSyncableFactory (which PrefServiceMockFactory derives from)
doesn't override Create(), so it's presumably returning a normal PrefService
instead of a PrefServiceSyncable.

i'm not sure why PrefServiceSyncableFactory adds a new create method instead of
overriding the existing one -- maybe it's so it can return a
scoped_ptr<PrefServiceSyncable> instead of a scoped_ptr<PrefService>?

i'm not sure what the solution is. i could change PrefServiceSyncableFactory to
override Create() instead and then make its (few) callers do the downcast if
they need a PrefServiceSyncable.

[Daniel Erat, 2013/12/06 21:07:43]

worse than i thought: CreateSyncable also takes a PrefRegistrySyncable rather
than a PrefRegistry (as Create does). the registry gets passed to
PrefServiceSyncable's c'tor, which requires a syncable registry.

maybe i should just keep all of this testing code in chrome/browser/extensions.
the downside is that extension_prefs_unittest.cc uses TestExtensionPrefs, so it
wouldn't be able to live alongside extension_prefs.cc after that gets moved.

+  pref_service_ = pref_service_factory_->Create(pref_registry_.get()).Pass();
 
   prefs_.reset(ExtensionPrefs::Create(
       pref_service_.get(),
@@ -172,11 +180,6 @@ std::string TestExtensionPrefs::AddExtensionAndReturnId(std::string name) {
   return extension->id();
 }
 
-PrefService* TestExtensionPrefs::CreateIncognitoPrefService() const {
-  return pref_service_->CreateIncognitoPrefService(
-      new ExtensionPrefStore(extension_pref_value_map_.get(), true));
-}
-
 void TestExtensionPrefs::set_extensions_disabled(bool extensions_disabled) {
   extensions_disabled_ = extensions_disabled;
 }