Eliminate GoogleURLTracker having a Profile ivar

This CL eliminates GoogleURLTracker needing to have a Profile ivar by adding APIs to GoogleURLTrackerClient to get the PrefService and URLRequestContextGetter to use. This CL additionally eliminates GoogleURLTracker's dependence on //chrome-level prefs by moving the kLastKnownGoogleURL and kLastPromptedGoogleURL prefs into the google component. BUG=373222,373239 TBR=thakis Review URL: https://codereview.chromium.org/307113002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275060 0039d316-1c4b-4281-b951-d872f2087c98
qweri0p · Jun 5, 2014 · 4a23327 · 4a23327
1 parent 446138c
commit 4a23327
Show file tree

Hide file tree

Showing 16 changed files with 106 additions and 44 deletions.
diff --git a/chrome/browser/google/chrome_google_url_tracker_client.cc b/chrome/browser/google/chrome_google_url_tracker_client.cc
@@ -9,13 +9,15 @@
 #include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/google/google_url_tracker_navigation_helper_impl.h"
 #include "chrome/browser/infobars/infobar_service.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 
-ChromeGoogleURLTrackerClient::ChromeGoogleURLTrackerClient() {
+ChromeGoogleURLTrackerClient::ChromeGoogleURLTrackerClient(Profile* profile)
+    : profile_(profile) {
 }
 
 ChromeGoogleURLTrackerClient::~ChromeGoogleURLTrackerClient() {
@@ -47,6 +49,15 @@ bool ChromeGoogleURLTrackerClient::IsBackgroundNetworkingEnabled() {
       switches::kDisableBackgroundNetworking);
 }
 
+PrefService* ChromeGoogleURLTrackerClient::GetPrefs() {
+  return profile_->GetPrefs();
+}
+
+net::URLRequestContextGetter*
+ChromeGoogleURLTrackerClient::GetRequestContext() {
+  return profile_->GetRequestContext();
+}
+
 void ChromeGoogleURLTrackerClient::Observe(
     int type,
     const content::NotificationSource& source,

diff --git a/chrome/browser/google/chrome_google_url_tracker_client.h b/chrome/browser/google/chrome_google_url_tracker_client.h
@@ -9,23 +9,29 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
+class Profile;
+
 class ChromeGoogleURLTrackerClient : public GoogleURLTrackerClient,
                                      public content::NotificationObserver {
  public:
-  ChromeGoogleURLTrackerClient();
+  explicit ChromeGoogleURLTrackerClient(Profile* profile);
   virtual ~ChromeGoogleURLTrackerClient();
 
   // GoogleURLTrackerClient:
   virtual void SetListeningForNavigationStart(bool listen) OVERRIDE;
   virtual bool IsListeningForNavigationStart() OVERRIDE;
   virtual bool IsBackgroundNetworkingEnabled() OVERRIDE;
+  virtual PrefService* GetPrefs() OVERRIDE;
+  virtual net::URLRequestContextGetter* GetRequestContext() OVERRIDE;
 
  private:
   // content::NotificationObserver:
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  Profile* profile_;
+
   content::NotificationRegistrar registrar_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeGoogleURLTrackerClient);

diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
@@ -13,8 +13,8 @@
 #include "chrome/browser/google/google_url_tracker_infobar_delegate.h"
 #include "chrome/browser/google/google_url_tracker_navigation_helper.h"
 #include "chrome/browser/google/google_util.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "components/google/core/browser/google_switches.h"
 #include "components/google/core/browser/google_url_tracker_client.h"
 #include "components/infobars/core/infobar.h"
@@ -31,14 +31,12 @@ const char GoogleURLTracker::kDefaultGoogleHomepage[] =
 const char GoogleURLTracker::kSearchDomainCheckURL[] =
     "https://www.google.com/searchdomaincheck?format=url&type=chrome";
 
-GoogleURLTracker::GoogleURLTracker(Profile* profile,
-                                   scoped_ptr<GoogleURLTrackerClient> client,
+GoogleURLTracker::GoogleURLTracker(scoped_ptr<GoogleURLTrackerClient> client,
                                    Mode mode)
-    : profile_(profile),
-      client_(client.Pass()),
+    : client_(client.Pass()),
       google_url_(mode == UNIT_TEST_MODE ?
           kDefaultGoogleHomepage :
-          profile->GetPrefs()->GetString(prefs::kLastKnownGoogleURL)),
+          client_->GetPrefs()->GetString(prefs::kLastKnownGoogleURL)),
       fetcher_id_(0),
       in_startup_sleep_(true),
       already_fetched_(false),
@@ -96,7 +94,7 @@ void GoogleURLTracker::SearchCommitted() {
 void GoogleURLTracker::AcceptGoogleURL(bool redo_searches) {
   GURL old_google_url = google_url_;
   google_url_ = fetched_google_url_;
-  PrefService* prefs = profile_->GetPrefs();
+  PrefService* prefs = client_->GetPrefs();
   prefs->SetString(prefs::kLastKnownGoogleURL, google_url_.spec());
   prefs->SetString(prefs::kLastPromptedGoogleURL, google_url_.spec());
   NotifyGoogleURLUpdated(old_google_url, google_url_);
@@ -106,8 +104,8 @@ void GoogleURLTracker::AcceptGoogleURL(bool redo_searches) {
 }
 
 void GoogleURLTracker::CancelGoogleURL() {
-  profile_->GetPrefs()->SetString(prefs::kLastPromptedGoogleURL,
-                                  fetched_google_url_.spec());
+  client_->GetPrefs()->SetString(prefs::kLastPromptedGoogleURL,
+                                 fetched_google_url_.spec());
   need_to_prompt_ = false;
   CloseAllEntries(false);
 }
@@ -136,7 +134,7 @@ void GoogleURLTracker::OnURLFetchComplete(const net::URLFetcher* source) {
 
   std::swap(url, fetched_google_url_);
   GURL last_prompted_url(
-      profile_->GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));
+      client_->GetPrefs()->GetString(prefs::kLastPromptedGoogleURL));
 
   if (last_prompted_url.is_empty()) {
     // On the very first run of Chrome, when we've never looked up the URL at
@@ -246,7 +244,7 @@ void GoogleURLTracker::StartFetchIfDesirable() {
   // we alarm the user.
   fetcher_->SetLoadFlags(net::LOAD_DISABLE_CACHE |
                          net::LOAD_DO_NOT_SAVE_COOKIES);
-  fetcher_->SetRequestContext(profile_->GetRequestContext());
+  fetcher_->SetRequestContext(client_->GetRequestContext());
 
   // Configure to max_retries at most kMaxRetries times for 5xx errors.
   static const int kMaxRetries = 5;

diff --git a/chrome/browser/google/google_url_tracker.h b/chrome/browser/google/google_url_tracker.h
@@ -24,7 +24,6 @@
 class GoogleURLTrackerClient;
 class GoogleURLTrackerNavigationHelper;
 class PrefService;
-class Profile;
 
 namespace infobars {
 class InfoBar;
@@ -61,9 +60,7 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   static const char kDefaultGoogleHomepage[];
 
   // Only the GoogleURLTrackerFactory and tests should call this.
-  GoogleURLTracker(Profile* profile,
-                   scoped_ptr<GoogleURLTrackerClient> client,
-                   Mode mode);
+  GoogleURLTracker(scoped_ptr<GoogleURLTrackerClient> client, Mode mode);
 
   virtual ~GoogleURLTracker();
 
@@ -169,8 +166,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
 
   CallbackList callback_list_;
 
-  Profile* profile_;
-
   scoped_ptr<GoogleURLTrackerClient> client_;
 
   GURL google_url_;

diff --git a/chrome/browser/google/google_url_tracker_factory.cc b/chrome/browser/google/google_url_tracker_factory.cc
@@ -9,7 +9,7 @@
 #include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/common/pref_names.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 
@@ -35,11 +35,10 @@ GoogleURLTrackerFactory::~GoogleURLTrackerFactory() {
 }
 
 KeyedService* GoogleURLTrackerFactory::BuildServiceInstanceFor(
-    content::BrowserContext* profile) const {
-  scoped_ptr<GoogleURLTrackerClient> client(new ChromeGoogleURLTrackerClient());
-  return new GoogleURLTracker(static_cast<Profile*>(profile),
-                              client.Pass(),
-                              GoogleURLTracker::NORMAL_MODE);
+    content::BrowserContext* context) const {
+  scoped_ptr<GoogleURLTrackerClient> client(
+      new ChromeGoogleURLTrackerClient(Profile::FromBrowserContext(context)));
+  return new GoogleURLTracker(client.Pass(), GoogleURLTracker::NORMAL_MODE);
 }
 
 void GoogleURLTrackerFactory::RegisterProfilePrefs(

diff --git a/chrome/browser/google/google_url_tracker_unittest.cc b/chrome/browser/google/google_url_tracker_unittest.cc
@@ -13,8 +13,8 @@
 #include "chrome/browser/google/google_url_tracker_factory.h"
 #include "chrome/browser/google/google_url_tracker_infobar_delegate.h"
 #include "chrome/browser/google/google_url_tracker_navigation_helper.h"
-#include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "components/google/core/browser/google_url_tracker_client.h"
 #include "components/infobars/core/infobar.h"
 #include "components/infobars/core/infobar_delegate.h"
@@ -71,21 +71,25 @@ void TestCallbackListener::RegisterCallback(
 
 class TestGoogleURLTrackerClient : public GoogleURLTrackerClient {
  public:
-  TestGoogleURLTrackerClient();
+  TestGoogleURLTrackerClient(Profile* profile_);
   virtual ~TestGoogleURLTrackerClient();
 
   virtual void SetListeningForNavigationStart(bool listen) OVERRIDE;
   virtual bool IsListeningForNavigationStart() OVERRIDE;
   virtual bool IsBackgroundNetworkingEnabled() OVERRIDE;
+  virtual PrefService* GetPrefs() OVERRIDE;
+  virtual net::URLRequestContextGetter* GetRequestContext() OVERRIDE;
 
  private:
+  Profile* profile_;
   bool observe_nav_start_;
 
   DISALLOW_COPY_AND_ASSIGN(TestGoogleURLTrackerClient);
 };
 
-TestGoogleURLTrackerClient::TestGoogleURLTrackerClient()
-    : observe_nav_start_(false) {
+TestGoogleURLTrackerClient::TestGoogleURLTrackerClient(Profile* profile)
+    : profile_(profile),
+      observe_nav_start_(false) {
 }
 
 TestGoogleURLTrackerClient::~TestGoogleURLTrackerClient() {
@@ -103,6 +107,13 @@ bool TestGoogleURLTrackerClient::IsBackgroundNetworkingEnabled() {
   return true;
 }
 
+PrefService* TestGoogleURLTrackerClient::GetPrefs() {
+  return profile_->GetPrefs();
+}
+
+net::URLRequestContextGetter* TestGoogleURLTrackerClient::GetRequestContext() {
+  return profile_->GetRequestContext();
+}
 
 // TestGoogleURLTrackerNavigationHelper ---------------------------------------
 
@@ -261,10 +272,10 @@ void GoogleURLTrackerTest::SetUp() {
   network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
   // Ownership is passed to google_url_tracker_, but a weak pointer is kept;
   // this is safe since GoogleURLTracker keeps the client for its lifetime.
-  client_ = new TestGoogleURLTrackerClient();
+  client_ = new TestGoogleURLTrackerClient(&profile_);
   scoped_ptr<GoogleURLTrackerClient> client(client_);
   google_url_tracker_.reset(new GoogleURLTracker(
-      &profile_, client.Pass(), GoogleURLTracker::UNIT_TEST_MODE));
+      client.Pass(), GoogleURLTracker::UNIT_TEST_MODE));
 }
 
 void GoogleURLTrackerTest::TearDown() {

diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
@@ -88,6 +88,7 @@
 #include "chrome/common/pref_names.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/bookmarks/browser/bookmark_utils.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/password_manager/core/browser/password_manager.h"
 #include "components/pref_registry/pref_registry_syncable.h"

diff --git a/chrome/browser/profile_resetter/profile_resetter.cc b/chrome/browser/profile_resetter/profile_resetter.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/installer/util/browser_distribution.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/management_policy.h"

diff --git a/chrome/browser/profile_resetter/profile_resetter_unittest.cc b/chrome/browser/profile_resetter/profile_resetter_unittest.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_browser_thread.h"
 #include "extensions/common/extension.h"

diff --git a/chrome/browser/search/instant_unittest_base.cc b/chrome/browser/search/instant_unittest_base.cc
@@ -20,6 +20,7 @@
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/google/core/browser/google_pref_names.h"
 #include "components/variations/entropy_provider.h"
 
 InstantUnitTestBase::InstantUnitTestBase() {

diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
@@ -1595,17 +1595,6 @@ const char kCertificateManagerWindowLastTabIndex[] =
 // This preference is only registered by the first-run procedure.
 const char kShowFirstRunBubbleOption[] = "show-first-run-bubble-option";
 
-// String containing the last known Google URL.  We re-detect this on startup in
-// most cases, and use it to send traffic to the correct Google host or with the
-// correct Google domain/country code for whatever location the user is in.
-const char kLastKnownGoogleURL[] = "browser.last_known_google_url";
-
-// String containing the last prompted Google URL to the user.
-// If the user is using .x TLD for Google URL and gets prompted about .y TLD
-// for Google URL, and says "no", we should leave the search engine set to .x
-// but not prompt again until the domain changes away from .y.
-const char kLastPromptedGoogleURL[] = "browser.last_prompted_google_url";
-
 // String containing the last known intranet redirect URL, if any.  See
 // intranet_redirect_detector.h for more information.
 const char kLastKnownIntranetRedirectOrigin[] = "browser.last_redirect_origin";

diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
@@ -525,8 +525,6 @@ extern const char kContentSettingsWindowLastTabIndex[];
 extern const char kCertificateManagerWindowLastTabIndex[];
 extern const char kShowFirstRunBubbleOption[];
 
-extern const char kLastKnownGoogleURL[];
-extern const char kLastPromptedGoogleURL[];
 extern const char kLastKnownIntranetRedirectOrigin[];
 
 extern const char kCountryIDAtInstall[];

diff --git a/components/google.gypi b/components/google.gypi
@@ -14,6 +14,8 @@
         '..',
       ],
       'sources': [
+        'google/core/browser/google_pref_names.cc',
+        'google/core/browser/google_pref_names.h',
         'google/core/browser/google_search_metrics.cc',
         'google/core/browser/google_search_metrics.h',
         'google/core/browser/google_switches.cc',

diff --git a/components/google/core/browser/google_pref_names.cc b/components/google/core/browser/google_pref_names.cc
@@ -0,0 +1,20 @@
+// Copyright 2014 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 "components/google/core/browser/google_pref_names.h"
+
+namespace prefs {
+
+// String containing the last known Google URL.  We re-detect this on startup in
+// most cases, and use it to send traffic to the correct Google host or with the
+// correct Google domain/country code for whatever location the user is in.
+const char kLastKnownGoogleURL[] = "browser.last_known_google_url";
+
+// String containing the last prompted Google URL to the user.
+// If the user is using .x TLD for Google URL and gets prompted about .y TLD
+// for Google URL, and says "no", we should leave the search engine set to .x
+// but not prompt again until the domain changes away from .y.
+const char kLastPromptedGoogleURL[] = "browser.last_prompted_google_url";
+
+}  // namespace prefs
diff --git a/components/google/core/browser/google_pref_names.h b/components/google/core/browser/google_pref_names.h
@@ -0,0 +1,17 @@
+// Copyright 2014 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.
+
+#ifndef COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_PREF_NAMES_H_
+#define COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_PREF_NAMES_H_
+
+namespace prefs {
+
+// Alphabetical list of preference names specific to the Google
+// component. Keep alphabetized, and document each in the .cc file.
+extern const char kLastKnownGoogleURL[];
+extern const char kLastPromptedGoogleURL[];
+
+}  // namespace prefs
+
+#endif  // COMPONENTS_GOOGLE_CORE_BROWSER_GOOGLE_PREF_NAMES_H_
diff --git a/components/google/core/browser/google_url_tracker_client.h b/components/google/core/browser/google_url_tracker_client.h
@@ -8,6 +8,11 @@
 #include "base/macros.h"
 
 class GoogleURLTracker;
+class PrefService;
+
+namespace net {
+class URLRequestContextGetter;
+}
 
 // Interface by which GoogleURLTracker communicates with its embedder.
 class GoogleURLTrackerClient {
@@ -31,6 +36,13 @@ class GoogleURLTrackerClient {
   // Returns whether background networking is enabled.
   virtual bool IsBackgroundNetworkingEnabled() = 0;
 
+  // Returns the PrefService that the GoogleURLTracker should use.
+  virtual PrefService* GetPrefs() = 0;
+
+  // Returns the URL request context information that the GoogleURLTracker
+  // should use.
+  virtual net::URLRequestContextGetter* GetRequestContext() = 0;
+
  protected:
   GoogleURLTracker* google_url_tracker() { return google_url_tracker_; }