Turn GoogleURLTrackerNavigationHelper(Impl) into a per-tab object.

chrome/browser/google/google_url_tracker_navigation_helper_impl.cc

Index: chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
diff --git a/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
index 64e4c5cb1adbfb69f00aaf928d729ddf6ba26597..7838e200f56b095839caec42154a49c6b3a64574 100644
--- a/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
+++ b/chrome/browser/google/google_url_tracker_navigation_helper_impl.cc
@@ -12,25 +12,34 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
 
-GoogleURLTrackerNavigationHelperImpl::
-    GoogleURLTrackerNavigationHelperImpl() : tracker_(NULL) {
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(GoogleURLTrackerNavigationHelperImpl);
+
+// static
+void GoogleURLTrackerNavigationHelperImpl::CreateForWebContentsAndTracker(

[Peter Kasting, 2014/05/15 21:29:52]

Function definition order should match .h declaration order.

[blundell, 2014/05/16 11:54:18]

Function removed.

On 2014/05/15 21:29:52, Peter Kasting wrote:

+    content::WebContents* contents,
+    GoogleURLTracker* tracker) {
+  if (FromWebContents(contents))
+    return;

[Peter Kasting, 2014/05/15 21:29:52]

Is this conditional actually needed?

[blundell, 2014/05/16 11:54:18]

Function removed.

On 2014/05/15 21:29:52, Peter Kasting wrote:

+
+  contents->SetUserData(
+      UserDataKey(),
+      new GoogleURLTrackerNavigationHelperImpl(contents, tracker));
 }
 
-GoogleURLTrackerNavigationHelperImpl::
-    ~GoogleURLTrackerNavigationHelperImpl() {
+GoogleURLTrackerNavigationHelperImpl::GoogleURLTrackerNavigationHelperImpl(
+    content::WebContents* web_contents,
+    GoogleURLTracker* tracker)
+    : GoogleURLTrackerNavigationHelper(tracker), web_contents_(web_contents) {

[Peter Kasting, 2014/05/15 21:29:52]

Nit: One line per initializer

[blundell, 2014/05/16 11:54:18]

Done.

 }
 
-void GoogleURLTrackerNavigationHelperImpl::SetGoogleURLTracker(
-    GoogleURLTracker* tracker) {
-  DCHECK(tracker);
-  tracker_ = tracker;
+GoogleURLTrackerNavigationHelperImpl::~GoogleURLTrackerNavigationHelperImpl() {
 }
 
 void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
-    const content::NavigationController* nav_controller,
     bool listen) {
   content::NotificationSource navigation_controller_source =
-      content::Source<content::NavigationController>(nav_controller);
+      content::Source<content::NavigationController>(
+          &web_contents_->GetController());
   if (listen) {
     registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
                     navigation_controller_source);
@@ -40,47 +49,35 @@ void GoogleURLTrackerNavigationHelperImpl::SetListeningForNavigationCommit(
   }
 }
 
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit(
-    const content::NavigationController* nav_controller) {
+bool GoogleURLTrackerNavigationHelperImpl::IsListeningForNavigationCommit() {
   content::NotificationSource navigation_controller_source =
-      content::Source<content::NavigationController>(nav_controller);
+      content::Source<content::NavigationController>(
+          &web_contents_->GetController());
   return registrar_.IsRegistered(
       this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
       navigation_controller_source);
 }
 
 void GoogleURLTrackerNavigationHelperImpl::SetListeningForTabDestruction(
-    const content::NavigationController* nav_controller,
     bool listen) {
-  content::NotificationSource navigation_controller_source =
-      content::Source<content::NavigationController>(nav_controller);
+  content::NotificationSource web_contents_source =
+      content::Source<content::WebContents>(web_contents_);
   if (listen) {
-    registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                   GetWebContentsSource(navigation_controller_source));
+    registrar_.Add(this,
+                   content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+                   web_contents_source);
   } else {
-    registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-                      GetWebContentsSource(navigation_controller_source));
+    registrar_.Remove(this,
+                      content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+                      web_contents_source);
   }
 }
 
-bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction(
-    const content::NavigationController* nav_controller) {
-  content::NotificationSource navigation_controller_source =
-      content::Source<content::NavigationController>(nav_controller);
+bool GoogleURLTrackerNavigationHelperImpl::IsListeningForTabDestruction() {
   return registrar_.IsRegistered(
       this,
       content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
-      GetWebContentsSource(navigation_controller_source));
-}
-
-content::NotificationSource
-    GoogleURLTrackerNavigationHelperImpl::GetWebContentsSource(
-        const content::NotificationSource& nav_controller_source) {
-  content::NavigationController* controller =
-      content::Source<content::NavigationController>(
-          nav_controller_source).ptr();
-  content::WebContents* web_contents = controller->GetWebContents();
-  return content::Source<content::WebContents>(web_contents);
+      content::Source<content::WebContents>(web_contents_));
 }
 
 void GoogleURLTrackerNavigationHelperImpl::Observe(
@@ -91,23 +88,24 @@ void GoogleURLTrackerNavigationHelperImpl::Observe(
     case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
       content::NavigationController* controller =
           content::Source<content::NavigationController>(source).ptr();
+      DCHECK(controller->GetWebContents() == web_contents_);

[Peter Kasting, 2014/05/15 21:29:52]

Nit: I think this can be DCHECK_EQ()

[blundell, 2014/05/16 11:54:18]

Done.

+
       // Here we're only listening to notifications where we already know
       // there's an associated InfoBarService.
-      content::WebContents* web_contents = controller->GetWebContents();
       InfoBarService* infobar_service =
-          InfoBarService::FromWebContents(web_contents);
+          InfoBarService::FromWebContents(web_contents_);
       DCHECK(infobar_service);
       const GURL& search_url = controller->GetActiveEntry()->GetURL();
       if (!search_url.is_valid())  // Not clear if this can happen.
-        tracker_->OnTabClosed(controller);
-      tracker_->OnNavigationCommitted(infobar_service, search_url);
+        google_url_tracker()->OnTabClosed(this);
+      google_url_tracker()->OnNavigationCommitted(infobar_service, search_url);
       break;
     }
 
     case content::NOTIFICATION_WEB_CONTENTS_DESTROYED: {
-      content::WebContents* web_contents =
-          content::Source<content::WebContents>(source).ptr();
-      tracker_->OnTabClosed(&web_contents->GetController());
+      DCHECK(content::Source<content::WebContents>(source).ptr() ==
+             web_contents_);
+      google_url_tracker()->OnTabClosed(this);
       break;
     }