Remove the retargeting notification

chrome/browser/extensions/api/web_navigation/web_navigation_api.cc

Index: chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
index 2616a7568fdc8c527ef4d9ac2a9c7525f9942436..5b440df5c3ce58e6409f6f3a7994176f45fadbe7 100644
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.cc
@@ -12,7 +12,6 @@
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/tab_contents/retargeting_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/extensions/api/web_navigation.h"
@@ -76,9 +75,6 @@ WebNavigationEventRouter::WebNavigationEventRouter(Profile* profile)
     : profile_(profile), browser_tab_strip_tracker_(this, this, nullptr) {
   CHECK(registrar_.IsEmpty());
   registrar_.Add(this,
-                 chrome::NOTIFICATION_RETARGETING,
-                 content::NotificationService::AllSources());
-  registrar_.Add(this,
                  chrome::NOTIFICATION_TAB_ADDED,
                  content::NotificationService::AllSources());
   registrar_.Add(this,
@@ -121,14 +117,6 @@ void WebNavigationEventRouter::Observe(
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
-    case chrome::NOTIFICATION_RETARGETING: {
-      Profile* profile = content::Source<Profile>(source).ptr();
-      if (profile->GetOriginalProfile() == profile_) {
-        Retargeting(
-            content::Details<const RetargetingDetails>(details).ptr());
-      }
-      break;
-    }
 
     case chrome::NOTIFICATION_TAB_ADDED:
       TabAdded(content::Details<content::WebContents>(details).ptr());
@@ -143,40 +131,40 @@ void WebNavigationEventRouter::Observe(
   }
 }
 
-void WebNavigationEventRouter::Retargeting(const RetargetingDetails* details) {
-  if (details->source_render_frame_id == 0)
+void WebNavigationEventRouter::RecordNewWebContents(
+    content::WebContents* source_web_contents,
+    int source_render_process_id,
+    int64_t source_render_frame_id,

[nasko, 2017/03/01 00:48:37]

nit: Why use int64_t when the routing id is just a plain old int?

[Patrick Noland, 2017/03/01 00:57:09]

No clue, I copied this from RetargetingDetails. I'll fix it.

[Patrick Noland, 2017/03/01 23:58:59]

Done.

+    GURL target_url,
+    content::WebContents* target_web_contents,
+    bool not_yet_in_tabstrip) {
+  if (source_render_frame_id == 0)
     return;
   WebNavigationTabObserver* tab_observer =
-      WebNavigationTabObserver::Get(details->source_web_contents);
+      WebNavigationTabObserver::Get(source_web_contents);
   if (!tab_observer) {
     // If you hit this DCHECK(), please add reproduction steps to
     // http://crbug.com/109464.
-    DCHECK(GetViewType(details->source_web_contents) != VIEW_TYPE_TAB_CONTENTS);
+    DCHECK(GetViewType(source_web_contents) != VIEW_TYPE_TAB_CONTENTS);
     return;
   }
   const FrameNavigationState& frame_navigation_state =
       tab_observer->frame_navigation_state();
 
   content::RenderFrameHost* frame_host = content::RenderFrameHost::FromID(
-      details->source_render_process_id, details->source_render_frame_id);
+      source_render_process_id, source_render_frame_id);
   if (!frame_navigation_state.CanSendEvents(frame_host))
     return;
 
   // If the WebContents isn't yet inserted into a tab strip, we need to delay
   // the extension event until the WebContents is fully initialized.
-  if (details->not_yet_in_tabstrip) {
-    pending_web_contents_[details->target_web_contents] =
-        PendingWebContents(details->source_web_contents,
-                           frame_host,
-                           details->target_web_contents,
-                           details->target_url);
+  if (not_yet_in_tabstrip) {
+    pending_web_contents_[target_web_contents] = PendingWebContents(
+        source_web_contents, frame_host, target_web_contents, target_url);
   } else {
     helpers::DispatchOnCreatedNavigationTarget(
-        details->source_web_contents,
-        details->target_web_contents->GetBrowserContext(),
-        frame_host,
-        details->target_web_contents,
-        details->target_url);
+        source_web_contents, target_web_contents->GetBrowserContext(),
+        frame_host, target_web_contents, target_url);
   }
 }
 
@@ -384,7 +372,8 @@ void WebNavigationTabObserver::DidOpenRequestedURL(
     const content::Referrer& referrer,
     WindowOpenDisposition disposition,
     ui::PageTransition transition,
-    bool started_from_context_menu) {
+    bool started_from_context_menu,
+    bool not_yet_in_tabstrip) {
   if (!navigation_state_.CanSendEvents(source_render_frame_host))
     return;
 
@@ -398,11 +387,17 @@ void WebNavigationTabObserver::DidOpenRequestedURL(
       disposition != WindowOpenDisposition::OFF_THE_RECORD)
     return;
 
-  helpers::DispatchOnCreatedNavigationTarget(web_contents(),
-                                             new_contents->GetBrowserContext(),
-                                             source_render_frame_host,
-                                             new_contents,
-                                             url);
+  WebNavigationAPI* api = WebNavigationAPI::GetFactoryInstance()->Get(
+      web_contents()->GetBrowserContext());
+  if (api) {

[nasko, 2017/03/01 00:48:37]

Would api ever be null?

[Patrick Noland, 2017/03/01 00:57:09]

I don't know enough to say for sure. The fact that it was null for incognito
browser contexts without the kServiceRedirectedInIncognito flag scared me enough
to add this check.

[nasko, 2017/03/01 18:36:31]

Since you've added the flag, I expect this will never be null, so I'd suggest
removing the check.

[Patrick Noland, 2017/03/01 23:58:59]

Done.

+    WebNavigationEventRouter* router = api->web_navigation_event_router_.get();
+    if (router) {

[nasko, 2017/03/01 00:48:37]

Since this is the last code in the method, it is better to use the pattern of:

if (!ptr)
  return;

ptr->foo();

It doesn't lead to multiple levels of indenting and returns early.

[Patrick Noland, 2017/03/01 23:58:59]

Done.

+      router->RecordNewWebContents(
+          web_contents(), source_render_frame_host->GetProcess()->GetID(),
+          source_render_frame_host->GetRoutingID(), url, new_contents,
+          not_yet_in_tabstrip);
+    }
+  }
 }
 
 void WebNavigationTabObserver::WebContentsDestroyed() {