Create GoogleURLTrackerClient interface and //chrome implementation

chrome/browser/google/google_url_tracker.cc

Index: chrome/browser/google/google_url_tracker.cc
diff --git a/chrome/browser/google/google_url_tracker.cc b/chrome/browser/google/google_url_tracker.cc
index 535cbbaa8ecac4a3a3c9bec030592abe7f3d8783..f3574eeed4a338985d042a7c364d03d34c703328 100644
--- a/chrome/browser/google/google_url_tracker.cc
+++ b/chrome/browser/google/google_url_tracker.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "components/google/core/browser/google_url_tracker_client.h"
 #include "components/infobars/core/infobar.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -34,9 +35,11 @@ const char GoogleURLTracker::kSearchDomainCheckURL[] =
 
 GoogleURLTracker::GoogleURLTracker(
     Profile* profile,
+    scoped_ptr<GoogleURLTrackerClient> client,
     scoped_ptr<GoogleURLTrackerNavigationHelper> nav_helper,
     Mode mode)
     : profile_(profile),
+      client_(client.Pass()),
       nav_helper_(nav_helper.Pass()),
       infobar_creator_(base::Bind(&GoogleURLTrackerInfoBarDelegate::Create)),
       google_url_(mode == UNIT_TEST_MODE ? kDefaultGoogleHomepage :
@@ -49,6 +52,7 @@ GoogleURLTracker::GoogleURLTracker(
       search_committed_(false),
       weak_ptr_factory_(this) {
   net::NetworkChangeNotifier::AddIPAddressObserver(this);
+  client_->SetGoogleURLTracker(this);
   nav_helper_->SetGoogleURLTracker(this);
 
   // Because this function can be called during startup, when kicking off a URL
@@ -201,6 +205,7 @@ void GoogleURLTracker::OnIPAddressChanged() {
 }
 
 void GoogleURLTracker::Shutdown() {
+  client_.reset();
   nav_helper_.reset();
   fetcher_.reset();
   weak_ptr_factory_.InvalidateWeakPtrs();

[Peter Kasting, 2014/05/14 23:30:28]

Why are the four lines above necessary?

[blundell, 2014/05/15 07:58:03]

I'm not familiar with this code. For KeyedServices, the general pattern is to
tear down state in Shutdown(). I'd prefer to avoid changing this code in this
CL.

On 2014/05/14 23:30:28, Peter Kasting wrote:

[Peter Kasting, 2014/05/15 21:19:21]

Ah, I overlooked that this is Shutdown() instead of the destructor.  I still
think we could remove some of these, but I'm not as worried in that case.

@@ -271,8 +276,8 @@ void GoogleURLTracker::SearchCommitted() {
     search_committed_ = true;
     // These notifications will fire a bit later in the same call chain we're
     // currently in.
-    if (!nav_helper_->IsListeningForNavigationStart())
-      nav_helper_->SetListeningForNavigationStart(true);
+    if (!client_->IsListeningForNavigationStart())
+      client_->SetListeningForNavigationStart(true);
   }
 }
 
@@ -412,12 +417,12 @@ void GoogleURLTracker::UnregisterForEntrySpecificNotifications(
        ++i) {
     if (nav_helper_->IsListeningForNavigationCommit(
             i->second->navigation_controller())) {
-      DCHECK(nav_helper_->IsListeningForNavigationStart());
+      DCHECK(client_->IsListeningForNavigationStart());
       return;
     }
   }
-  if (nav_helper_->IsListeningForNavigationStart()) {
+  if (client_->IsListeningForNavigationStart()) {
     DCHECK(!search_committed_);
-    nav_helper_->SetListeningForNavigationStart(false);
+    client_->SetListeningForNavigationStart(false);
   }
 }