From 4a23327d451274446261b18ded7b7f014176995e Mon Sep 17 00:00:00 2001 From: "blundell@chromium.org" Date: Thu, 5 Jun 2014 10:54:17 +0000 Subject: [PATCH] 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 --- .../chrome_google_url_tracker_client.cc | 13 ++++++++++- .../google/chrome_google_url_tracker_client.h | 8 ++++++- chrome/browser/google/google_url_tracker.cc | 20 ++++++++-------- chrome/browser/google/google_url_tracker.h | 7 +----- .../google/google_url_tracker_factory.cc | 11 ++++----- .../google/google_url_tracker_unittest.cc | 23 ++++++++++++++----- chrome/browser/prefs/browser_prefs.cc | 1 + .../profile_resetter/profile_resetter.cc | 1 + .../profile_resetter_unittest.cc | 1 + .../browser/search/instant_unittest_base.cc | 1 + chrome/common/pref_names.cc | 11 --------- chrome/common/pref_names.h | 2 -- components/google.gypi | 2 ++ .../google/core/browser/google_pref_names.cc | 20 ++++++++++++++++ .../google/core/browser/google_pref_names.h | 17 ++++++++++++++ .../core/browser/google_url_tracker_client.h | 12 ++++++++++ 16 files changed, 106 insertions(+), 44 deletions(-) create mode 100644 components/google/core/browser/google_pref_names.cc create mode 100644 components/google/core/browser/google_pref_names.h diff --git a/chrome/browser/google/chrome_google_url_tracker_client.cc b/chrome/browser/google/chrome_google_url_tracker_client.cc index 5068afece4ca5b..85278ab7f5acdf 100644 --- 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 index 58fdcc9a861b86..08a4e2eeeeffca 100644 --- a/chrome/browser/google/chrome_google_url_tracker_client.h +++ b/chrome/browser/google/chrome_google_url_tracker_client.h @@ -9,16 +9,20 @@ #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: @@ -26,6 +30,8 @@ class ChromeGoogleURLTrackerClient : public GoogleURLTrackerClient, 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 index 13145cca4b369a..77cdaad3af8360 100644 --- 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 client, +GoogleURLTracker::GoogleURLTracker(scoped_ptr 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 index 3687427cded32f..c87a83ff9bcbc6 100644 --- 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 client, - Mode mode); + GoogleURLTracker(scoped_ptr client, Mode mode); virtual ~GoogleURLTracker(); @@ -169,8 +166,6 @@ class GoogleURLTracker : public net::URLFetcherDelegate, CallbackList callback_list_; - Profile* profile_; - scoped_ptr client_; GURL google_url_; diff --git a/chrome/browser/google/google_url_tracker_factory.cc b/chrome/browser/google/google_url_tracker_factory.cc index 6c45c85427bc74..be2018729e57c7 100644 --- 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 client(new ChromeGoogleURLTrackerClient()); - return new GoogleURLTracker(static_cast(profile), - client.Pass(), - GoogleURLTracker::NORMAL_MODE); + content::BrowserContext* context) const { + scoped_ptr 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 index 6f29909bbb195a..59ac6c019e9ee4 100644 --- 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 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 index fcc319162025f8..80e80af508612c 100644 --- 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 index c42fac41361bd4..6dcb8cd14fd944 100644 --- 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 index 28f87c5103f0fd..6156d2e212c82e 100644 --- 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 index a4821acb4028d1..cc37be9ffaf01e 100644 --- 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 index 6211af2727bc8b..6b4df83da34e2b 100644 --- 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 index bda86ac601ec7a..f672cee96bc13a 100644 --- 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 index 69c43ffc563cb7..676fe81ef8895e 100644 --- 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 new file mode 100644 index 00000000000000..81e6749d4812cd --- /dev/null +++ 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 new file mode 100644 index 00000000000000..2c58dd29187477 --- /dev/null +++ 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 index 5cc4f98d7431ac..044a89abcac4e6 100644 --- 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_; }