ABANDONED: Remove Chrome dependencies from TestExtensionPrefs.

chrome/browser/extensions/test_extension_prefs.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/test_extension_prefs.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #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"
 #include "base/values.h"
 #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"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_constants.h"
 #include "sync/api/string_ordinal.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
 
@@ skipping 17 matching lines @@
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(IncrementalTimeProvider);
 
   mutable base::Time current_time_;
 };
 
 }  // 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");
   EXPECT_TRUE(base::CreateDirectory(extensions_dir_));
 
   ResetPrefRegistry();
   RecreateExtensionPrefs();
 }
 
 TestExtensionPrefs::~TestExtensionPrefs() {
@@ skipping 24 matching lines @@
     base::RunLoop run_loop;
     ASSERT_TRUE(
         task_runner_->PostTaskAndReply(
             FROM_HERE,
             base::Bind(&base::DoNothing),
             run_loop.QuitClosure()));
     run_loop.Run();
   }
 
   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(),
       temp_dir_.path(),
       extension_pref_value_map_.get(),
       ExtensionsBrowserClient::Get()->CreateAppSorting().Pass(),
       extensions_disabled_,
       // Guarantee that no two extensions get the same installation time
       // stamp and we can reliably assert the installation order in the tests.
       scoped_ptr<ExtensionPrefs::TimeProvider>(
@@ skipping 43 matching lines @@
                                false,
                                syncer::StringOrdinal::CreateInitialOrdinal());
   return extension;
 }
 
 std::string TestExtensionPrefs::AddExtensionAndReturnId(std::string name) {
   scoped_refptr<Extension> extension(AddExtension(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;
 }
 
 }  // namespace extensions