Revert of Elimate NOTIFICATION_GOOGLE_URL_UPDATED (https://codereview.chromium.org/284343003/)

Reason for revert:
speculative, browser_tests stated timing out on all non-dbg win main waterfall test bots, with a message such as


E:\b\depot_tools\python276_bin\python_slave.exe E:\b\build\scripts\slave\runisolatedtest.py --test_name browser_tests --builder_name "Win7 Tests (1)" --checkout_dir E:\b\build\slave\Win7_Tests__1_\build E:\b\build\slave\Win7_Tests__1_\build\src\out\Release\browser_tests.exe -- E:\b\build\slave\Win7_Tests__1_\build\src\out\Release\browser_tests.exe --brave-new-test-launcher --test-launcher-bot-mode --test-launcher-total-shards=3 --test-launcher-shard-index=0 --lib=browser_tests --gtest_print_time --gtest_output=xml:E:\b\build\slave\Win7_Tests__1_\build\gtest-results\browser_tests\browser_tests.xml --test-launcher-summary-output=c:\users\chrome~2\appdata\local\temp\tmpfbmzn9

E:\b\depot_tools\python276_bin\python_slave.exe E:\b\build\slave\Win7_Tests__1_\build\src\tools\swarming_client\isolate.py run --isolated E:\b\build\slave\Win7_Tests__1_\build\src\out\Release\browser_tests.isolated -v -- --no-cr --test-launcher-bot-mode --test-launcher-total-shards=3 --test-launcher-shard-index=0 --gtest_output=xml:E:\b\build\slave\Win7_Tests__1_\build\gtest-results\browser_tests\browser_tests.xml --test-launcher-summary-output=c:\users\chrome~2\appdata\local\temp\tmpfbmzn9
 INFO         isolate(685): CompleteState.load_isolate(E:\b\build\slave\Win7_Tests__1_\build, E:\b\build\slave\Win7_Tests__1_\build\src\chrome\browser_tests.isolate, {}, {}, {'EXECUTABLE_SUFFIX': '.exe'}, False)
 INFO         isolate(156): normalize_path_variables(E:\b\build\slave\Win7_Tests__1_\build, {}, E:\b\build\slave\Win7_Tests__1_\build\src\chrome)
 INFO  isolate_format( 57): determine_root_dir(E:\b\build\slave\Win7_Tests__1_\build\src\chrome, 80 files) -> E:\b\build\slave\Win7_Tests__1_\build\src
 INFO         isolate( 84): recreate_tree(outdir=E:\b\build\slave\Win7_Tests__1_\isolate-2014-05-18gqemnl, indir=E:\b\build\slave\Win7_Tests__1_\build\src, files=9443, action=4, as_hash=False)
 INFO         isolate(1266): Running ['E:\\b\\depot_tools\\python276_bin\\python_slave.exe', '../testing/test_env.py', u'..\\out\\Release/browser_tests.exe', '--test-launcher-bot-mode', '--no-cr', '--test-launcher-bot-mode', '--test-launcher-total-shards=3', '--test-launcher-shard-index=0', '--gtest_output=xml:E:\\b\\build\\slave\\Win7_Tests__1_\\build\\gtest-results\\browser_tests\\browser_tests.xml', '--test-launcher-summary-output=c:\\users\\chrome~2\\appdata\\local\\temp\\tmpfbmzn9'], cwd=E:\b\build\slave\Win7_Tests__1_\isolate-2014-05-18gqemnl\chrome

command timed out: 600 seconds without output, attempting to kill
program finished with exit code 1

Original issue's description:
> Elimate NOTIFICATION_GOOGLE_URL_UPDATED
> 
> This CL moves the remaining client of the NOTIFICATION_GOOGLE_URL_UPDATED
> notification to instead register a callback with GoogleURLTracker and
> eliminates the notification. It also migrate the GoogleURLTracker unittest from
> listening for the notification to listening for the callback.
> 
> BUG=373261,373237
> TBR=thakis
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271313

TBR=pkasting@chromium.org,blundell@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=373261,373237

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

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

chrome/browser/chrome_notification_types.h

@@ -353,6 +353,18 @@ enum NotificationType {
   // This is sent from Instant when the omnibox focus state changes.
   NOTIFICATION_OMNIBOX_FOCUS_CHANGED,
 
+  // Sent when the Google URL for a profile has been updated.  Some services
+  // cache this value and need to update themselves when it changes.  See
+  // google_util::GetGoogleURLAndUpdateIfNecessary().  The source is the
+  // Profile, the details a GoogleURLTracker::UpdatedDetails containing the old
+  // and new URLs.
+  //
+  // Note that because incognito mode requests for the GoogleURLTracker are
+  // redirected to the non-incognito profile's copy, this notification will only
+  // ever fire on non-incognito profiles; thus listeners should use
+  // GetOriginalProfile() when constructing a Source to filter against.
+  NOTIFICATION_GOOGLE_URL_UPDATED,
+
   // Printing ----------------------------------------------------------------
 
   // Notification from PrintJob that an event occurred. It can be that a page

chrome/browser/google/google_url_tracker.cc

@@ -102,13 +102,19 @@ void GoogleURLTracker::GoogleURLSearchCommitted(Profile* profile) {
 }
 
 void GoogleURLTracker::AcceptGoogleURL(bool redo_searches) {
-  GURL old_google_url = google_url_;
+  UpdatedDetails urls(google_url_, fetched_google_url_);
   google_url_ = fetched_google_url_;
   PrefService* prefs = profile_->GetPrefs();
   prefs->SetString(prefs::kLastKnownGoogleURL, google_url_.spec());
   prefs->SetString(prefs::kLastPromptedGoogleURL, google_url_.spec());
-  NotifyGoogleURLUpdated(old_google_url, google_url_);
-
+  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_),
+      content::Details<UpdatedDetails>(&urls));
   need_to_prompt_ = false;
   CloseAllEntries(redo_searches);
 }

chrome/browser/google/google_url_tracker.h

@@ -39,9 +39,9 @@ class InfoBar;
 //
 // Most consumers should only call GoogleURL(), which is guaranteed to
 // synchronously return a value at all times (even during startup or in unittest
-// mode).  Consumers who need to be notified when things change should register
-// a callback that provides the original and updated values via
-// RegisterCallback().
+// mode).  Consumers who need to be notified when things change should listen to
+// the notification service for NOTIFICATION_GOOGLE_URL_UPDATED, which provides
+// the original and updated values.
 //
 // To protect users' privacy and reduce server load, no updates will be
 // performed (ever) unless at least one consumer registers interest by calling
@@ -56,6 +56,9 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   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;
+
   // The constructor does different things depending on which of these values
   // you pass it.  Hopefully these are self-explanatory.
   enum Mode {
@@ -208,8 +211,8 @@ class GoogleURLTracker : public net::URLFetcherDelegate,
   bool need_to_fetch_;     // True if a consumer actually wants us to fetch an
                            // updated URL.  If this is never set, we won't
                            // bother to fetch anything.
-                           // Consumers should register a callback via
-                           // RegisterCallback().
+                           // Consumers should observe
+                           // chrome::NOTIFICATION_GOOGLE_URL_UPDATED.
   bool need_to_prompt_;    // True if the last fetched Google URL is not
                            // matched with current user's default Google URL
                            // nor the last prompted Google URL.

chrome/browser/ui/navigation_correction_tab_observer.cc

@@ -6,12 +6,12 @@
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/google/google_url_tracker_factory.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
@@ -36,13 +36,8 @@ NavigationCorrectionTabObserver::NavigationCorrectionTabObserver(
                    base::Unretained(this)));
   }
 
-  GoogleURLTracker* google_url_tracker =
-      GoogleURLTrackerFactory::GetForProfile(profile_);
-  if (google_url_tracker) {
-    google_url_updated_subscription_ = google_url_tracker->RegisterCallback(
-        base::Bind(&NavigationCorrectionTabObserver::OnGoogleURLUpdated,
-                   base::Unretained(this)));
-  }
+  registrar_.Add(this, chrome::NOTIFICATION_GOOGLE_URL_UPDATED,
+                 content::Source<Profile>(profile_->GetOriginalProfile()));
 }
 
 NavigationCorrectionTabObserver::~NavigationCorrectionTabObserver() {
@@ -65,13 +60,19 @@ void NavigationCorrectionTabObserver::RenderViewCreated(
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// Internal helpers
+// content::NotificationObserver overrides
 
-void NavigationCorrectionTabObserver::OnGoogleURLUpdated(GURL old_url,
-                                                         GURL new_url) {
+void NavigationCorrectionTabObserver::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK_EQ(chrome::NOTIFICATION_GOOGLE_URL_UPDATED, type);
   UpdateNavigationCorrectionInfo(web_contents()->GetRenderViewHost());
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Internal helpers
+
 GURL NavigationCorrectionTabObserver::GetNavigationCorrectionURL() const {
   // Disable navigation corrections when the preference is disabled or when in
   // Incognito mode.

chrome/browser/ui/navigation_correction_tab_observer.h

@@ -6,7 +6,8 @@
 #define CHROME_BROWSER_UI_NAVIGATION_CORRECTION_TAB_OBSERVER_H_
 
 #include "base/prefs/pref_change_registrar.h"
-#include "chrome/browser/google/google_url_tracker.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
@@ -19,6 +20,7 @@ class PrefRegistrySyncable;
 // Per-tab class to implement navigation suggestion service functionality.
 class NavigationCorrectionTabObserver
     : public content::WebContentsObserver,
+      public content::NotificationObserver,
       public content::WebContentsUserData<NavigationCorrectionTabObserver> {
  public:
   virtual ~NavigationCorrectionTabObserver();
@@ -33,10 +35,12 @@ class NavigationCorrectionTabObserver
   virtual void RenderViewCreated(
       content::RenderViewHost* render_view_host) OVERRIDE;
 
-  // Internal helpers ----------------------------------------------------------
+  // content::NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
 
-  // Callback that is called when the Google URL is updated.
-  void OnGoogleURLUpdated(GURL old_url, GURL new_url);
+  // Internal helpers ----------------------------------------------------------
 
   // Returns the URL for the correction service.  If the returned URL
   // is empty, the default error pages will be used.
@@ -49,8 +53,8 @@ class NavigationCorrectionTabObserver
   void UpdateNavigationCorrectionInfo(content::RenderViewHost* rvh);
 
   Profile* profile_;
+  content::NotificationRegistrar registrar_;
   PrefChangeRegistrar pref_change_registrar_;
-  scoped_ptr<GoogleURLTracker::Subscription> google_url_updated_subscription_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationCorrectionTabObserver);
 };