Service CreateNewWindow on the UI thread with a new mojo interface

content/browser/web_contents/web_contents_impl.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.
 
 #include "content/browser/web_contents/web_contents_impl.h"
 
 #include <stddef.h>
 
 #include <cmath>
 #include <utility>
@@ skipping 2027 matching lines @@
 
 void WebContentsImpl::OnRenderFrameProxyVisibilityChanged(bool visible) {
   if (visible && !GetOuterWebContents()->IsHidden())
     WasShown();
   else if (!visible)
     WasHidden();
 }
 
 void WebContentsImpl::CreateNewWindow(
     SiteInstance* source_site_instance,
+    RenderFrameHost* opener,
     int32_t render_view_route_id,
     int32_t main_frame_route_id,
     int32_t main_frame_widget_route_id,
     const mojom::CreateNewWindowParams& params,
     SessionStorageNamespace* session_storage_namespace) {
   // We should have zero valid routing ids, or three valid routing IDs.
   DCHECK_EQ((render_view_route_id == MSG_ROUTING_NONE),
             (main_frame_route_id == MSG_ROUTING_NONE));
   DCHECK_EQ((render_view_route_id == MSG_ROUTING_NONE),
             (main_frame_widget_route_id == MSG_ROUTING_NONE));
 
   int render_process_id = source_site_instance->GetProcess()->GetID();
   // The route IDs passed into this function can be trusted not to already be in
   // use; they were allocated by the RenderWidgetHelper on the IO thread.

[ncarter (slow), 2017/04/20 21:30:36]

The "on the IO thread" clause is now inaccurate.

[Charlie Harrison, 2017/04/21 15:30:07]

Done.

   DCHECK(!RenderFrameHostImpl::FromID(render_process_id, main_frame_route_id));
 
   // We usually create the new window in the same BrowsingInstance (group of
   // script-related windows), by passing in the current SiteInstance.  However,
   // if the opener is being suppressed (in a non-guest), we create a new
   // SiteInstance in its own BrowsingInstance.
   bool is_guest = BrowserPluginGuest::IsGuest(this);
 
   // If the opener is to be suppressed, the new window can be in any process.
   // Since routing ids are process specific, we must not have one passed in
@@ skipping 33 matching lines @@
     // have a chance to create more frames at this point.
     RenderFrameHostImpl* rfh =
         RenderFrameHostImpl::FromID(render_process_id, main_frame_route_id);
     if (rfh) {
       DCHECK(rfh->IsRenderFrameLive());
       rfh->Init();
     }
     return;
   }
 
+  int opener_render_frame_id = opener->GetRoutingID();

[ncarter (slow), 2017/04/20 21:30:36]

Just inline this: there are multiple render_frame_id's in scope in this
function, and I think readability is maximized if we keeping the number of
aliases down.

[Charlie Harrison, 2017/04/21 15:30:07]

Done.

+
   // Create the new web contents. This will automatically create the new
   // WebContentsView. In the future, we may want to create the view separately.
   CreateParams create_params(GetBrowserContext(), site_instance.get());
   create_params.routing_id = render_view_route_id;
   create_params.main_frame_routing_id = main_frame_route_id;
   create_params.main_frame_widget_routing_id = main_frame_widget_route_id;
   create_params.main_frame_name = params.frame_name;
   create_params.opener_render_process_id = render_process_id;
-  create_params.opener_render_frame_id = params.opener_render_frame_id;
+  create_params.opener_render_frame_id = opener_render_frame_id;
   create_params.opener_suppressed = params.opener_suppressed;
   if (params.disposition == WindowOpenDisposition::NEW_BACKGROUND_TAB)
     create_params.initially_hidden = true;
   create_params.renderer_initiated_creation =
       main_frame_route_id != MSG_ROUTING_NONE;
 
   WebContentsImpl* new_contents = NULL;
   if (!is_guest) {
     create_params.context = view_->GetNativeView();
     create_params.initial_size = GetContainerBounds().size();
@@ skipping 26 matching lines @@
     }
     // Save the created window associated with the route so we can show it
     // later.
     DCHECK_NE(MSG_ROUTING_NONE, main_frame_widget_route_id);
     pending_contents_[std::make_pair(
         render_process_id, main_frame_widget_route_id)] = new_contents;
     AddDestructionObserver(new_contents);
   }
 
   if (delegate_) {
-    delegate_->WebContentsCreated(
-        this, render_process_id, params.opener_render_frame_id,
-        params.frame_name, params.target_url, new_contents);
+    delegate_->WebContentsCreated(this, render_process_id,
+                                  opener->GetRoutingID(), params.frame_name,
+                                  params.target_url, new_contents);
   }
 
-  RenderFrameHost* source_render_frame_host =
-      RenderFrameHost::FromID(render_process_id, params.opener_render_frame_id);
-
-  if (source_render_frame_host) {
+  if (opener) {
     for (auto& observer : observers_) {
-      observer.DidOpenRequestedURL(new_contents, source_render_frame_host,
-                                   params.target_url, params.referrer,
-                                   params.disposition, ui::PAGE_TRANSITION_LINK,
+      observer.DidOpenRequestedURL(new_contents, opener, params.target_url,
+                                   params.referrer, params.disposition,
+                                   ui::PAGE_TRANSITION_LINK,
                                    false,  // started_from_context_menu
                                    true);  // renderer_initiated
     }
   }
 
   if (params.opener_suppressed) {
     // When the opener is suppressed, the original renderer cannot access the
     // new window.  As a result, we need to show and navigate the window here.
     bool was_blocked = false;
     if (delegate_) {
@@ skipping 3279 matching lines @@
     RenderViewHost* render_view_host = RenderViewHost::From(render_widget_host);
     if (!render_view_host)
       continue;
     render_view_host_set.insert(render_view_host);
   }
   for (RenderViewHost* render_view_host : render_view_host_set)
     render_view_host->OnWebkitPreferencesChanged();
 }
 
 }  // namespace content