Remove the retargeting notification

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

Index: chrome/browser/extensions/api/web_navigation/web_navigation_api.h
diff --git a/chrome/browser/extensions/api/web_navigation/web_navigation_api.h b/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
index e467d996d21ca4075fa414320189d7c785a18afb..92eaf3d38ac955066691c572bc086b2138ceca74 100644
--- a/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
+++ b/chrome/browser/extensions/api/web_navigation/web_navigation_api.h
@@ -27,8 +27,6 @@
 #include "extensions/browser/event_router.h"
 #include "url/gurl.h"
 
-struct RetargetingDetails;
-
 namespace extensions {
 
 // Tab contents observer that forwards navigation events to the event router.
@@ -69,7 +67,8 @@ class WebNavigationTabObserver
                            const content::Referrer& referrer,
                            WindowOpenDisposition disposition,
                            ui::PageTransition transition,
-                           bool started_from_context_menu) override;
+                           bool started_from_context_menu,
+                           bool not_yet_in_tabstrip) override;
   void WebContentsDestroyed() override;
 
   // This method dispatches the already created onBeforeNavigate event.
@@ -119,6 +118,17 @@ class WebNavigationEventRouter : public TabStripModelObserver,
   explicit WebNavigationEventRouter(Profile* profile);
   ~WebNavigationEventRouter() override;
 
+  // Router level handler for the creation of WebContents. Stores information
+  // about the newly created WebContents. This information is later used when
+  // the WebContents for the tab is added to the tabstrip and we receive the
+  // TAB_ADDED notification.
+  void RecordNewWebContents(content::WebContents* source_web_contents,
+                            int source_render_process_id,
+                            int64_t source_render_frame_id,
+                            GURL target_url,
+                            content::WebContents* target_web_contents,
+                            bool not_yet_in_tabstrip);
+
  private:
   // Used to cache the information about newly created WebContents objects.
   struct PendingWebContents{
@@ -149,11 +159,6 @@ class WebNavigationEventRouter : public TabStripModelObserver,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // Handler for the NOTIFICATION_RETARGETING event. The method takes the
-  // details of such an event and stores them for the later
-  // NOTIFICATION_TAB_ADDED event.
-  void Retargeting(const RetargetingDetails* details);
-
   // Handler for the NOTIFICATION_TAB_ADDED event. The method takes the details
   // of such an event and creates a JSON formated extension event from it.
   void TabAdded(content::WebContents* tab);
@@ -209,6 +214,7 @@ class WebNavigationAPI : public BrowserContextKeyedAPI,
 
  private:
   friend class BrowserContextKeyedAPIFactory<WebNavigationAPI>;
+  friend class WebNavigationTabObserver;
 
   content::BrowserContext* browser_context_;
 
@@ -216,6 +222,7 @@ class WebNavigationAPI : public BrowserContextKeyedAPI,
   static const char* service_name() {
     return "WebNavigationAPI";
   }
+  static const bool kServiceRedirectedInIncognito = true;

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

Why do we need this?

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

Without this, attempts to access the WebNavigationAPI instance for an incognito
browser context return null. This makes it impossible for the
WebNavigationTabObserver to report the event up to the router in
DidOpenRequestedURL. Not reporting the event for an incognito browser context
caused tests to fail, so it seemed like we needed that to continue to happen.

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

Thanks for the info! Devlin, this make sense to me, but having an extra pair of
eyes to confirm will be great.

[Devlin, 2017/03/02 01:33:36]

This makes sense if you want to share an instance between incognito and
on-the-record.  If instead there should be two separate instances, you can use
kServiceHasOwnInstanceInIncognito.  I *think* this was designed to be used
across the incognito boundary, so this would be correct.

[Patrick Noland, 2017/03/02 22:15:21]

There's a browser test (TargetBlankIncognito) that relies on the service
instance being shared across the incognito boundary, so I think
kServiceRedirectedInIncognito is the option we want. 

   static const bool kServiceIsNULLWhileTesting = true;
 
   // Created lazily upon OnListenerAdded.