Fix WebContentsDelegate::ShouldCreateWebContents implementations.

content/public/browser/web_contents_delegate.h

 // 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_DELEGATE_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_DELEGATE_H_
 
 #include <stdint.h>
 
 #include <set>
@@ skipping 24 matching lines @@
 namespace base {
 class FilePath;
 class ListValue;
 }
 
 namespace content {
 class ColorChooser;
 class JavaScriptDialogManager;
 class PageState;
 class RenderFrameHost;
+class RenderWidgetHost;
 class SessionStorageNamespace;
+class SiteInstance;
 class WebContents;
 class WebContentsImpl;
 struct ColorSuggestion;
 struct ContextMenuParams;
 struct DropData;
 struct FileChooserParams;
 struct NativeWebKeyboardEvent;
 struct Referrer;
 struct SecurityStyleExplanations;
 }  // namespace content
 
 namespace gfx {
 class Point;
 class Rect;
 class Size;
 }
 
 namespace net {
 class X509Certificate;
 }
 
 namespace blink {
 class WebGestureEvent;
 }
 
 namespace content {
-class RenderWidgetHost;
 
 struct OpenURLParams;
 struct WebContentsUnresponsiveState;
 
 // Objects implement this interface to get notified about changes in the
 // WebContents and to provide necessary functionality.
 class CONTENT_EXPORT WebContentsDelegate {
  public:
   WebContentsDelegate();
 
@@ skipping 202 matching lines @@
   // Shows the repost form confirmation dialog box.
   virtual void ShowRepostFormWarningDialog(WebContents* source) {}
 
   // Allows delegate to override navigation to the history entries.
   // Returns true to allow WebContents to continue with the default processing.
   virtual bool OnGoToEntryOffset(int offset);
 
   // Allows delegate to control whether a WebContents will be created. Returns
   // true to allow the creation. Default is to allow it. In cases where the
   // delegate handles the creation/navigation itself, it will use |target_url|.
   // The embedder has to synchronously adopt |route_id| or else the view will

[Charlie Reis, 2016/11/23 06:59:48]

Is this comment wrong as well?  We should probably give some guidelines of when
to use the various routing IDs and when not to.

[ncarter (slow), 2016/11/24 00:00:05]

The comment wasn't wrong, just imprecise. In truth, this is an absolutely awful
API, but I've hopefully clarified it with the new comment.

[Charlie Reis, 2016/11/24 00:31:04]

Wow.  Yes, that sounds more accurate, and clearly shows why this API is awful. 
:)  Good candidate for future cleanup.

   // be destroyed.
   virtual bool ShouldCreateWebContents(
       WebContents* web_contents,
+      SiteInstance* source_site_instance,

[Charlie Reis, 2016/11/23 06:59:48]

Maybe also something in the comment about how web_contents and
source_site_instance are related?  Is |web_contents| the opener?  Is
|source_site_instance| guaranteed to come from a frame inside |web_contents|?

[ncarter (slow), 2016/11/24 00:00:05]

Done.

       int32_t route_id,
       int32_t main_frame_route_id,
       int32_t main_frame_widget_route_id,
       WindowContainerType window_container_type,
+      const GURL& opener_url,
       const std::string& frame_name,
       const GURL& target_url,
       const std::string& partition_id,
       SessionStorageNamespace* session_storage_namespace);
 
   // Notifies the delegate about the creation of a new WebContents. This
   // typically happens when popups are created.
   virtual void WebContentsCreated(WebContents* source_contents,
                                   int opener_render_process_id,
                                   int opener_render_frame_id,
@@ skipping 234 matching lines @@
   // Called when |this| is no longer the WebContentsDelegate for |source|.
   void Detach(WebContents* source);
 
   // The WebContents that this is currently a delegate for.
   std::set<WebContents*> attached_contents_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_WEB_CONTENTS_DELEGATE_H_