Introduce ability to register callback with GoogleURLTracker

This callback interface will replace the NOTIFICATION_GOOGLE_URL_UPDATED
notification once all clients have been converted. This CL converts
TemplateURLService and SearchProviderInstallData.

BUG=373236,373254,373258,373260,373253
TBR=brettw

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

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

chrome/browser/google/google_url_tracker.cc

@@ -107,6 +107,10 @@ void GoogleURLTracker::AcceptGoogleURL(bool redo_searches) {
   PrefService* prefs = profile_->GetPrefs();
   prefs->SetString(prefs::kLastKnownGoogleURL, google_url_.spec());
   prefs->SetString(prefs::kLastPromptedGoogleURL, google_url_.spec());
+  NotifyGoogleURLUpdated(urls.first, urls.second);
+
+  // TODO(blundell): Convert all clients to use the callback interface and
+  // eliminate this notification. crbug.com/373237
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
       content::Source<Profile>(profile_),
@@ -374,6 +378,11 @@ void GoogleURLTracker::OnTabClosed(
   NOTREACHED();
 }
 
+scoped_ptr<GoogleURLTracker::Subscription> GoogleURLTracker::RegisterCallback(
+    const OnGoogleURLUpdatedCallback& cb) {
+  return callback_list_.Add(cb);
+}
+
 void GoogleURLTracker::CloseAllEntries(bool redo_searches) {
   // Delete all entries, whether they have infobars or not.
   while (!entry_map_.empty())
@@ -421,3 +430,7 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
     nav_helper_->SetListeningForNavigationStart(false);
   }
 }
+
+void GoogleURLTracker::NotifyGoogleURLUpdated(GURL old_url, GURL new_url) {
+  callback_list_.Notify(old_url, new_url);
+}

chrome/browser/google/google_url_tracker.h

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "base/callback_forward.h"
+#include "base/callback_list.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -49,6 +50,12 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
                          public net::NetworkChangeNotifier::IPAddressObserver,
                          public KeyedService {
  public:
+  // Callback that is called when the Google URL is updated. The arguments are
+  // the old and new URLs.
+  typedef base::Callback<void(GURL, GURL)> OnGoogleURLUpdatedCallback;
+  typedef base::CallbackList<void(GURL, GURL)> CallbackList;
+  typedef CallbackList::Subscription Subscription;
+
   // The contents of the Details for a NOTIFICATION_GOOGLE_URL_UPDATED.
   typedef std::pair<GURL, GURL> UpdatedDetails;
 
@@ -59,6 +66,9 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
     UNIT_TEST_MODE,
   };
 
+  static const char kDefaultGoogleHomepage[];
+  static const char kSearchDomainCheckURL[];
+
   // Only the GoogleURLTrackerFactory and tests should call this.  No code other
   // than the GoogleURLTracker itself should actually use
   // GoogleURLTrackerFactory::GetForProfile().
@@ -125,8 +135,8 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   virtual void OnTabClosed(
       content::NavigationController* navigation_controller);
 
-  static const char kDefaultGoogleHomepage[];
-  static const char kSearchDomainCheckURL[];
+  scoped_ptr<Subscription> RegisterCallback(
+      const OnGoogleURLUpdatedCallback& cb);
 
  private:
   friend class GoogleURLTrackerTest;
@@ -175,6 +185,10 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
       const GoogleURLTrackerMapEntry& map_entry,
       bool must_be_listening_for_commit);
 
+  void NotifyGoogleURLUpdated(GURL old_url, GURL new_url);
+
+  CallbackList callback_list_;
+
   Profile* profile_;
 
   scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper_;

chrome/browser/search/instant_unittest_base.cc

@@ -21,9 +21,6 @@
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/variations/entropy_provider.h"
-#include "content/public/browser/notification_details.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
 
 InstantUnitTestBase::InstantUnitTestBase() {
   field_trial_list_.reset(new base::FieldTrialList(
@@ -75,12 +72,8 @@ void InstantUnitTestBase::NotifyGoogleBaseURLUpdate(
   // UIThreadSearchTermsData::GoogleBaseURLValue()
   // For simulating test behavior, this is overridden below.
   UIThreadSearchTermsData::SetGoogleBaseURL(new_google_base_url);
-  GoogleURLTracker::UpdatedDetails details(GURL("https://www.google.com/"),
-                                           GURL(new_google_base_url));
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-      content::Source<Profile>(profile()->GetOriginalProfile()),
-      content::Details<GoogleURLTracker::UpdatedDetails>(&details));
+  TemplateURLServiceFactory::GetForProfile(profile())->OnGoogleURLUpdated(
+      GURL("https://www.google.com"), GURL(new_google_base_url));
 }
 
 bool InstantUnitTestBase::IsInstantServiceObserver(

chrome/browser/search_engines/search_provider_install_data.cc

@@ -16,6 +16,7 @@
 #include "base/sequenced_task_runner_helpers.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_url_tracker.h"
+#include "chrome/browser/google/google_url_tracker_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search_engines/search_host_to_urls_map.h"
 #include "chrome/browser/search_engines/search_terms_data.h"
@@ -25,10 +26,6 @@
 #include "chrome/browser/search_engines/util.h"
 #include "chrome/browser/webdata/web_data_service.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 
@@ -122,18 +119,12 @@ void GoogleURLChangeNotifier::OnChange(const std::string& google_base_url) {
 
 // Notices changes in the Google base URL and sends them along
 // to the SearchProviderInstallData on the I/O thread.
-class GoogleURLObserver : public content::NotificationObserver,
-                          public content::RenderProcessHostObserver {
+class GoogleURLObserver : public content::RenderProcessHostObserver {
  public:
   GoogleURLObserver(Profile* profile,
                     GoogleURLChangeNotifier* change_notifier,
                     content::RenderProcessHost* host);
 
-  // Implementation of content::NotificationObserver.
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // Implementation of content::RenderProcessHostObserver.
   virtual void RenderProcessHostDestroyed(
         content::RenderProcessHost* host) OVERRIDE;
@@ -141,8 +132,12 @@ class GoogleURLObserver : public content::NotificationObserver,
  private:
   virtual ~GoogleURLObserver() {}
 
+  // Callback that is called when the Google URL is updated.
+  void OnGoogleURLUpdated(GURL old_url, GURL new_url);
+
   scoped_refptr<GoogleURLChangeNotifier> change_notifier_;
-  content::NotificationRegistrar registrar_;
+
+  scoped_ptr<GoogleURLTracker::Subscription> google_url_updated_subscription_;
 
   DISALLOW_COPY_AND_ASSIGN(GoogleURLObserver);
 };
@@ -153,21 +148,24 @@ GoogleURLObserver::GoogleURLObserver(
     content::RenderProcessHost* host)
     : change_notifier_(change_notifier) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  registrar_.Add(this, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-                 content::Source<Profile>(profile->GetOriginalProfile()));
+  GoogleURLTracker* google_url_tracker =
+      GoogleURLTrackerFactory::GetForProfile(profile);
+
+  // GoogleURLTracker is not created in tests.
+  if (google_url_tracker) {
+    google_url_updated_subscription_ =
+        google_url_tracker->RegisterCallback(base::Bind(
+            &GoogleURLObserver::OnGoogleURLUpdated, base::Unretained(this)));
+  }
   host->AddObserver(this);
 }
 
-void GoogleURLObserver::Observe(int type,
-                                const content::NotificationSource& source,
-                                const content::NotificationDetails& details) {
-  DCHECK_EQ(chrome::NOTIFICATION_GOOGLE_URL_UPDATED, type);
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&GoogleURLChangeNotifier::OnChange,
-                 change_notifier_.get(),
-                 content::Details<GoogleURLTracker::UpdatedDetails>(details)->
-                     second.spec()));
+void GoogleURLObserver::OnGoogleURLUpdated(GURL old_url, GURL new_url) {
+  BrowserThread::PostTask(BrowserThread::IO,
+                          FROM_HERE,
+                          base::Bind(&GoogleURLChangeNotifier::OnChange,
+                                     change_notifier_.get(),
+                                     new_url.spec()));
 }
 
 void GoogleURLObserver::RenderProcessHostDestroyed(

chrome/browser/search_engines/search_provider_install_data_unittest.cc

@@ -129,6 +129,10 @@ class SearchProviderInstallDataTest : public testing::Test {
   TemplateURL* AddNewTemplateURL(const std::string& url,
                                  const base::string16& keyword);
 
+  // Sets the Google base URL to |base_url| and runs the IO thread for
+  // |SearchProviderInstallData| to process the update.
+  void SetGoogleBaseURLAndProcessOnIOThread(GURL base_url);
+
   TemplateURLServiceTestUtil util_;
 
   // Provides the search provider install state on the I/O thread. It must be
@@ -182,6 +186,20 @@ TemplateURL* SearchProviderInstallDataTest::AddNewTemplateURL(
   return t_url;
 }
 
+void SearchProviderInstallDataTest::SetGoogleBaseURLAndProcessOnIOThread(
+    GURL base_url) {
+  util_.SetGoogleBaseURL(base_url);
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&SearchProviderInstallData::OnGoogleURLChange,
+                 base::Unretained(install_data_),
+                 base_url.spec()));
+
+  // Wait for the I/O thread to process the update notification.
+  base::RunLoop().RunUntilIdle();
+}
+
 // Actual tests ---------------------------------------------------------------
 
 TEST_F(SearchProviderInstallDataTest, GetInstallState) {
@@ -242,9 +260,7 @@ TEST_F(SearchProviderInstallDataTest, GoogleBaseUrlChange) {
   // Set up the database.
   util_.ChangeModelToLoadState();
   std::string google_host = "w.com";
-  util_.SetGoogleBaseURL(GURL("http://" + google_host + "/"));
-  // Wait for the I/O thread to process the update notification.
-  base::RunLoop().RunUntilIdle();
+  SetGoogleBaseURLAndProcessOnIOThread(GURL("http://" + google_host + "/"));
 
   AddNewTemplateURL("{google:baseURL}?q={searchTerms}",
                     base::ASCIIToUTF16("t"));
@@ -260,9 +276,7 @@ TEST_F(SearchProviderInstallDataTest, GoogleBaseUrlChange) {
 
   // Change the Google base url.
   google_host = "foo.com";
-  util_.SetGoogleBaseURL(GURL("http://" + google_host + "/"));
-  // Wait for the I/O thread to process the update notification.
-  base::RunLoop().RunUntilIdle();
+  SetGoogleBaseURLAndProcessOnIOThread(GURL("http://" + google_host + "/"));
 
   // Verify that the change got picked up.
   test_get_install_state.RunTests(google_host, std::string());

chrome/browser/search_engines/template_url_service.cc

@@ -24,7 +24,7 @@
 #include "base/time/time.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
-#include "chrome/browser/google/google_url_tracker.h"
+#include "chrome/browser/google/google_url_tracker_factory.h"
 #include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
@@ -980,19 +980,12 @@ base::string16 TemplateURLService::GetKeywordShortName(
 void TemplateURLService::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_HISTORY_URL_VISITED) {
-    content::Details<history::URLVisitedDetails> visit_details(details);
-    if (!loaded_)
-      visits_to_add_.push_back(*visit_details.ptr());
-    else
-      UpdateKeywordSearchTermsForURL(*visit_details.ptr());
-  } else {
-    DCHECK_EQ(chrome::NOTIFICATION_GOOGLE_URL_UPDATED, type);
-    if (loaded_) {
-      GoogleBaseURLChanged(
-          content::Details<GoogleURLTracker::UpdatedDetails>(details)->first);
-    }
-  }
+  DCHECK_EQ(type, chrome::NOTIFICATION_HISTORY_URL_VISITED);
+  content::Details<history::URLVisitedDetails> visit_details(details);
+  if (!loaded_)
+    visits_to_add_.push_back(*visit_details.ptr());
+  else
+    UpdateKeywordSearchTermsForURL(*visit_details.ptr());
 }
 
 void TemplateURLService::Shutdown() {
@@ -1508,8 +1501,15 @@ void TemplateURLService::Init(const Initializer* initializers,
     content::Source<Profile> profile_source(profile_->GetOriginalProfile());
     notification_registrar_.Add(this, chrome::NOTIFICATION_HISTORY_URL_VISITED,
                                 profile_source);
-    notification_registrar_.Add(this, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-                                profile_source);
+    GoogleURLTracker* google_url_tracker =
+        GoogleURLTrackerFactory::GetForProfile(profile_);
+
+    // GoogleURLTracker is not created in tests.
+    if (google_url_tracker) {
+      google_url_updated_subscription_ =
+          google_url_tracker->RegisterCallback(base::Bind(
+              &TemplateURLService::OnGoogleURLUpdated, base::Unretained(this)));
+    }
     pref_change_registrar_.Init(GetPrefs());
     pref_change_registrar_.Add(
         prefs::kSyncedDefaultSearchProviderGUID,
@@ -1915,6 +1915,11 @@ void TemplateURLService::GoogleBaseURLChanged(const GURL& old_base_url) {
     NotifyObservers();
 }
 
+void TemplateURLService::OnGoogleURLUpdated(GURL old_url, GURL new_url) {
+  if (loaded_)
+    GoogleBaseURLChanged(old_url);
+}
+
 void TemplateURLService::OnDefaultSearchChange(
     const TemplateURLData* data,
     DefaultSearchManager::Source source) {

chrome/browser/search_engines/template_url_service.h

@@ -16,6 +16,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
+#include "chrome/browser/google/google_url_tracker.h"
 #include "chrome/browser/search_engines/default_search_manager.h"
 #include "chrome/browser/search_engines/template_url_id.h"
 #include "chrome/browser/webdata/web_data_service.h"
@@ -431,6 +432,7 @@ class TemplateURLService : public WebDataServiceConsumer,
   FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, PreSyncDeletes);
   FRIEND_TEST_ALL_PREFIXES(TemplateURLServiceSyncTest, MergeInSyncTemplateURL);
 
+  friend class InstantUnitTestBase;
   friend class TemplateURLServiceTestUtilBase;
 
   typedef std::map<base::string16, TemplateURL*> KeywordToTemplateMap;
@@ -486,6 +488,9 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Transitions to the loaded state.
   void ChangeToLoadedState();
 
+  // Callback that is called when the Google URL is updated.
+  void OnGoogleURLUpdated(GURL old_url, GURL new_url);
+
   // Called by DefaultSearchManager when the effective default search engine has
   // changed.
   void OnDefaultSearchChange(const TemplateURLData* new_dse_data,
@@ -772,6 +777,8 @@ class TemplateURLService : public WebDataServiceConsumer,
   // Helper class to manage the default search engine.
   DefaultSearchManager default_search_manager_;
 
+  scoped_ptr<GoogleURLTracker::Subscription> google_url_updated_subscription_;
+
   // Used to disable the prepopulated search engines in tests.
   static bool g_fallback_search_engines_disabled;
 

chrome/browser/search_engines/template_url_service_test_util.cc

@@ -18,7 +18,6 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service_syncable.h"
 #include "chrome/test/base/testing_profile.h"
-#include "content/public/browser/notification_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_CHROMEOS)
@@ -127,13 +126,10 @@ void TemplateURLServiceTestUtilBase::SetGoogleBaseURL(
     const GURL& base_url) const {
   DCHECK(base_url.is_valid());
   UIThreadSearchTermsData data(profile());
-  GoogleURLTracker::UpdatedDetails urls(GURL(data.GoogleBaseURLValue()),
-                                        base_url);
+  GURL old_url = GURL(data.GoogleBaseURLValue());
   UIThreadSearchTermsData::SetGoogleBaseURL(base_url.spec());
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
-      content::Source<Profile>(profile()),
-      content::Details<GoogleURLTracker::UpdatedDetails>(&urls));
+  TemplateURLServiceFactory::GetForProfile(profile())
+      ->OnGoogleURLUpdated(old_url, base_url);
 }
 
 void TemplateURLServiceTestUtilBase::SetManagedDefaultSearchPreferences(