Remove the retargeting notification

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Implements the Chrome Extensions WebNavigation API.
 
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api.h"
 
 #include "base/lazy_instance.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_constants.h"
 #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"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_request_details.h"
@@ skipping 43 matching lines @@
       target_web_contents(target_web_contents),
       target_url(target_url) {
 }
 
 WebNavigationEventRouter::PendingWebContents::~PendingWebContents() {}
 
 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,
                  content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
                  content::NotificationService::AllSources());
 
   browser_tab_strip_tracker_.Init(
       BrowserTabStripTracker::InitWith::ALL_BROWERS);
 }
 
@@ skipping 22 matching lines @@
     return;
 
   helpers::DispatchOnTabReplaced(old_contents, profile_, new_contents);
 }
 
 void WebNavigationEventRouter::Observe(
     int type,
     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());
       break;
 
     case content::NOTIFICATION_WEB_CONTENTS_DESTROYED:
       TabDestroyed(content::Source<content::WebContents>(source).ptr());
       break;
 
     default:
       NOTREACHED();
   }
 }
 
-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);
   }
 }
 
 void WebNavigationEventRouter::TabAdded(content::WebContents* tab) {
   std::map<content::WebContents*, PendingWebContents>::iterator iter =
       pending_web_contents_.find(tab);
   if (iter == pending_web_contents_.end())
     return;
 
   WebNavigationTabObserver* tab_observer =
@@ skipping 187 matching lines @@
   navigation_state_.SetErrorOccurredInFrame(render_frame_host);
 }
 
 void WebNavigationTabObserver::DidOpenRequestedURL(
     content::WebContents* new_contents,
     content::RenderFrameHost* source_render_frame_host,
     const GURL& url,
     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;
 
   // We only send the onCreatedNavigationTarget if we end up creating a new
   // window.
   if (disposition != WindowOpenDisposition::SINGLETON_TAB &&
       disposition != WindowOpenDisposition::NEW_FOREGROUND_TAB &&
       disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB &&
       disposition != WindowOpenDisposition::NEW_POPUP &&
       disposition != WindowOpenDisposition::NEW_WINDOW &&
       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() {
   g_tab_observer.Get().erase(web_contents());
   registrar_.RemoveAll();
 }
 
 void WebNavigationTabObserver::DispatchCachedOnBeforeNavigate() {
   if (!pending_on_before_navigate_event_)
     return;
@@ skipping 172 matching lines @@
   return g_factory.Pointer();
 }
 
 void WebNavigationAPI::OnListenerAdded(const EventListenerInfo& details) {
   web_navigation_event_router_.reset(new WebNavigationEventRouter(
       Profile::FromBrowserContext(browser_context_)));
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
 }  // namespace extensions