Parameterize the PrefService that AutofillDownloadManager uses.

Rather than AutofillDownloadManager obtaining the PrefService to use from
BrowserContext, have AutofillDownloadManager's creator supply it with the
PrefService to use. Incremental step toward abstracting BrowserContext
knowledge out of AutofillDownloadManager.

BUG=303050

Review URL: https://codereview.chromium.org/49303005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233375 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc

@@ -4,13 +4,14 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/prefs/testing_pref_service.h"
+#include "base/prefs/pref_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
 #include "chrome/browser/ui/autofill/autofill_popup_view.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/autofill/content/browser/autofill_driver_impl.h"
+#include "components/autofill/core/browser/autofill_common_test.h"
 #include "components/autofill/core/browser/autofill_external_delegate.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/test_autofill_external_delegate.h"
@@ -53,13 +54,15 @@ class MockAutofillExternalDelegate : public AutofillExternalDelegate {
 class MockAutofillManagerDelegate
     : public autofill::TestAutofillManagerDelegate {
  public:
-  MockAutofillManagerDelegate() {}
+  MockAutofillManagerDelegate()
+      : prefs_(autofill::test::PrefServiceForTesting()) {
+  }
   virtual ~MockAutofillManagerDelegate() {}
 
-  virtual PrefService* GetPrefs() OVERRIDE { return &prefs_; }
+  virtual PrefService* GetPrefs() OVERRIDE { return prefs_.get(); }
 
  private:
-  TestingPrefServiceSimple prefs_;
+  scoped_ptr<PrefService> prefs_;
 
   DISALLOW_COPY_AND_ASSIGN(MockAutofillManagerDelegate);
 };

components/autofill/DEPS

@@ -7,5 +7,8 @@ include_rules = [
   # Autofill is a layered component; subdirectories must explicitly introduce
   # the ability to use the content layer as appropriate.
   "-components/autofill/content",
+  # TODO(blundell): This subtraction can be eliminated once crbug.com/314560 is
+  # fixed.
+  "-components/user_prefs/user_prefs.h",
   "-content/public/common",
 ]

components/autofill/core/browser/DEPS

@@ -29,6 +29,7 @@ specific_include_rules = {
     #
     # Do not add to the list of temporarily-allowed dependencies below,
     # and please do not introduce more #includes of these files.
+    "!components/user_prefs/user_prefs.h",
     "!content/public/browser/browser_thread.h",
     "!content/public/test",
 

components/autofill/core/browser/autocomplete_history_manager_unittest.cc

@@ -6,7 +6,7 @@
 
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop_proxy.h"
-#include "base/prefs/testing_pref_service.h"
+#include "base/prefs/pref_service.h"
 #include "base/run_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
@@ -15,6 +15,7 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "components/autofill/core/browser/autocomplete_history_manager.h"
+#include "components/autofill/core/browser/autofill_common_test.h"
 #include "components/autofill/core/browser/autofill_external_delegate.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/test_autofill_driver.h"
@@ -82,12 +83,14 @@ class MockWebDataServiceWrapperCurrent : public MockWebDataServiceWrapperBase {
 class MockAutofillManagerDelegate
     : public autofill::TestAutofillManagerDelegate {
  public:
-  MockAutofillManagerDelegate() {}
+  MockAutofillManagerDelegate()
+      : prefs_(test::PrefServiceForTesting()) {
+  }
   virtual ~MockAutofillManagerDelegate() {}
-  virtual PrefService* GetPrefs() OVERRIDE { return &prefs_; }
+  virtual PrefService* GetPrefs() OVERRIDE { return prefs_.get(); }
 
  private:
-  TestingPrefServiceSimple prefs_;
+  scoped_ptr<PrefService> prefs_;
 
   DISALLOW_COPY_AND_ASSIGN(MockAutofillManagerDelegate);
 };

components/autofill/core/browser/autofill_common_test.cc

@@ -6,13 +6,17 @@
 
 #include "base/guid.h"
 #include "base/prefs/pref_service.h"
+#include "base/prefs/pref_service_builder.h"
+#include "base/prefs/testing_pref_store.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
 #include "components/autofill/core/common/form_data.h"
 #include "components/autofill/core/common/form_field_data.h"
+#include "components/user_prefs/pref_registry_syncable.h"
 #include "components/user_prefs/user_prefs.h"
 #include "components/webdata/encryptor/encryptor.h"
 #include "content/public/browser/browser_context.h"
@@ -26,6 +30,15 @@ const char kSettingsOrigin[] = "Chrome settings";
 
 }  // namespace
 
+scoped_ptr<PrefService> PrefServiceForTesting() {
+  scoped_refptr<user_prefs::PrefRegistrySyncable> registry(
+      new user_prefs::PrefRegistrySyncable());
+  AutofillManager::RegisterProfilePrefs(registry.get());
+  PrefServiceBuilder builder;
+  builder.WithUserPrefs(new TestingPrefStore());
+  return scoped_ptr<PrefService>(builder.Create(registry.get()));
+}
+
 void CreateTestFormField(const char* label,
                          const char* name,
                          const char* value,

components/autofill/core/browser/autofill_common_test.h

@@ -5,6 +5,10 @@
 #ifndef COMPONENTS_AUTOFILL_CORE_BROWSER_AUTOFILL_COMMON_TEST_H_
 #define COMPONENTS_AUTOFILL_CORE_BROWSER_AUTOFILL_COMMON_TEST_H_
 
+#include "base/memory/scoped_ptr.h"
+
+class PrefService;
+
 namespace content {
 class BrowserContext;
 }
@@ -19,6 +23,13 @@ struct FormFieldData;
 // Common utilities shared amongst Autofill tests.
 namespace test {
 
+// Returns a PrefService that can be used for Autofill-related testing in
+// contexts where the PrefService would otherwise have to be constructed
+// manually (e.g., in unit tests within Autofill core code). The returned
+// PrefService has had Autofill preferences registered on its associated
+// registry.
+scoped_ptr<PrefService> PrefServiceForTesting();
+
 // Provides a quick way to populate a FormField with c-strings.
 void CreateTestFormField(const char* label,
                          const char* name,

components/autofill/core/browser/autofill_download.cc

@@ -18,7 +18,6 @@
 #include "components/autofill/core/browser/autofill_xml_parser.h"
 #include "components/autofill/core/browser/form_structure.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
-#include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
@@ -70,8 +69,10 @@ struct AutofillDownloadManager::FormRequestData {
 };
 
 AutofillDownloadManager::AutofillDownloadManager(BrowserContext* context,
+                                                 PrefService* pref_service,
                                                  Observer* observer)
     : browser_context_(context),
+      pref_service_(pref_service),
       observer_(observer),
       max_form_cache_size_(kMaxFormCacheSize),
       next_query_request_(base::Time::Now()),
@@ -80,11 +81,10 @@ AutofillDownloadManager::AutofillDownloadManager(BrowserContext* context,
       negative_upload_rate_(0),
       fetcher_id_for_unittest_(0) {
   DCHECK(observer_);
-  PrefService* preferences = user_prefs::UserPrefs::Get(browser_context_);
   positive_upload_rate_ =
-      preferences->GetDouble(prefs::kAutofillPositiveUploadRate);
+      pref_service_->GetDouble(prefs::kAutofillPositiveUploadRate);
   negative_upload_rate_ =
-      preferences->GetDouble(prefs::kAutofillNegativeUploadRate);
+      pref_service_->GetDouble(prefs::kAutofillNegativeUploadRate);
 }
 
 AutofillDownloadManager::~AutofillDownloadManager() {
@@ -170,8 +170,7 @@ void AutofillDownloadManager::SetPositiveUploadRate(double rate) {
   positive_upload_rate_ = rate;
   DCHECK_GE(rate, 0.0);
   DCHECK_LE(rate, 1.0);
-  PrefService* preferences = user_prefs::UserPrefs::Get(browser_context_);
-  preferences->SetDouble(prefs::kAutofillPositiveUploadRate, rate);
+  pref_service_->SetDouble(prefs::kAutofillPositiveUploadRate, rate);
 }
 
 void AutofillDownloadManager::SetNegativeUploadRate(double rate) {
@@ -180,8 +179,7 @@ void AutofillDownloadManager::SetNegativeUploadRate(double rate) {
   negative_upload_rate_ = rate;
   DCHECK_GE(rate, 0.0);
   DCHECK_LE(rate, 1.0);
-  PrefService* preferences = user_prefs::UserPrefs::Get(browser_context_);
-  preferences->SetDouble(prefs::kAutofillNegativeUploadRate, rate);
+  pref_service_->SetDouble(prefs::kAutofillNegativeUploadRate, rate);
 }
 
 bool AutofillDownloadManager::StartRequest(

components/autofill/core/browser/autofill_download.h

@@ -18,6 +18,8 @@
 #include "components/autofill/core/browser/autofill_type.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
+class PrefService;
+
 namespace content {
 class BrowserContext;
 }  // namespace content
@@ -62,8 +64,10 @@ class AutofillDownloadManager : public net::URLFetcherDelegate {
     virtual ~Observer() {}
   };
 
+  // |context| and |pref_service| must outlive this instance.
   // |observer| - observer to notify on successful completion or error.
   AutofillDownloadManager(content::BrowserContext* context,
+                          PrefService* pref_service,
                           Observer* observer);
   virtual ~AutofillDownloadManager();
 
@@ -132,12 +136,15 @@ class AutofillDownloadManager : public net::URLFetcherDelegate {
   void SetPositiveUploadRate(double rate);
   void SetNegativeUploadRate(double rate);
 
-  // The pointer value is const, so this can only be set in the
-  // constructor.  Must not be null.
+  // The BrowserContext that this instance will use. Must not be null, and must
+  // outlive this instance.
   content::BrowserContext* const browser_context_;  // WEAK
 
+  // The PrefService that this instance will use. Must not be null, and must
+  // outlive this instance.
+  PrefService* const pref_service_;  // WEAK
+
   // The observer to notify when server predictions are successfully received.
-  // The pointer value is const, so this can only be set in the constructor.
   // Must not be null.
   AutofillDownloadManager::Observer* const observer_;  // WEAK
 

components/autofill/core/browser/autofill_download_unittest.cc

@@ -64,7 +64,7 @@ class AutofillDownloadTest : public AutofillDownloadManager::Observer,
                              public testing::Test {
  public:
   AutofillDownloadTest()
-      : download_manager_(&profile_, this) {
+      : download_manager_(&profile_, profile_.GetPrefs(), this) {
   }
 
   void LimitCache(size_t cache_size) {

components/autofill/core/browser/autofill_manager.cc

@@ -202,7 +202,9 @@ AutofillManager::AutofillManager(
   if (enable_download_manager == ENABLE_AUTOFILL_DOWNLOAD_MANAGER) {
     download_manager_.reset(
         new AutofillDownloadManager(
-            driver->GetWebContents()->GetBrowserContext(), this));
+            driver->GetWebContents()->GetBrowserContext(),
+            manager_delegate_->GetPrefs(),
+            this));
   }
 }