Make window.open() IPCs be frame-based

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 1991 matching lines @@
 
 void WebContentsImpl::OnRenderFrameProxyVisibilityChanged(bool visible) {
   if (visible && !GetOuterWebContents()->IsHidden())
     WasShown();
   else if (!visible)
     WasHidden();
 }
 
 void WebContentsImpl::CreateNewWindow(
     SiteInstance* source_site_instance,
-    int32_t route_id,
+    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) {
+  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.
+  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
   // as argument here.
-  DCHECK(!params.opener_suppressed || route_id == MSG_ROUTING_NONE);
+  DCHECK(!params.opener_suppressed || render_view_route_id == MSG_ROUTING_NONE);
 
   scoped_refptr<SiteInstance> site_instance =
       params.opener_suppressed && !is_guest
           ? SiteInstance::CreateForURL(GetBrowserContext(), params.target_url)
           : source_site_instance;
 
-  // A message to create a new window can only come from a process for a frame
-  // in this WebContents' FrameTree. If any other process sends the request, it
-  // is invalid and the process must be terminated.
-  int render_process_id = source_site_instance->GetProcess()->GetID();
-  if (!HasMatchingProcess(&frame_tree_, render_process_id)) {
-    RenderProcessHost* rph = source_site_instance->GetProcess();
-    base::ProcessHandle process_handle = rph->GetHandle();
-    if (process_handle != base::kNullProcessHandle) {
-      RecordAction(
-          base::UserMetricsAction("Terminate_ProcessMismatch_CreateNewWindow"));
-      rph->Shutdown(RESULT_CODE_KILLED, false);
-    }
-    return;
-  }
-
   // We must assign the SessionStorageNamespace before calling Init().
   //
   // http://crbug.com/142685
   const std::string& partition_id =
       GetContentClient()->browser()->
           GetStoragePartitionIdForSite(GetBrowserContext(),
                                        site_instance->GetSiteURL());
   StoragePartition* partition = BrowserContext::GetStoragePartition(
       GetBrowserContext(), site_instance.get());
   DOMStorageContextWrapper* dom_storage_context =
       static_cast<DOMStorageContextWrapper*>(partition->GetDOMStorageContext());
   SessionStorageNamespaceImpl* session_storage_namespace_impl =
       static_cast<SessionStorageNamespaceImpl*>(session_storage_namespace);
   CHECK(session_storage_namespace_impl->IsFromContext(dom_storage_context));
 
   if (delegate_ &&
       !delegate_->ShouldCreateWebContents(
-          this, source_site_instance, route_id, main_frame_route_id,
+          this, source_site_instance, render_view_route_id, main_frame_route_id,
           main_frame_widget_route_id, params.window_container_type,
           params.opener_url, params.frame_name, params.target_url, partition_id,
           session_storage_namespace)) {
-    if (route_id != MSG_ROUTING_NONE &&
-        !RenderViewHost::FromID(render_process_id, route_id)) {
-      // If the embedder didn't create a WebContents for this route, we need to
-      // delete the RenderView that had already been created.
-      Send(new ViewMsg_Close(route_id));
-    }
     // Note: even though we're not creating a WebContents here, it could have
     // been created by the embedder so ensure that the RenderFrameHost is
     // properly initialized.
     // It's safe to only target the frame because the render process will not
     // have a chance to create more frames at this point.
     RenderFrameHostImpl* rfh =
         RenderFrameHostImpl::FromID(render_process_id, main_frame_route_id);
-    if (rfh)
+    if (rfh) {
+      DCHECK(rfh->IsRenderFrameLive());
       rfh->Init();
+    }
     return;
   }
 
   // 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 = route_id;
+  create_params.routing_id = render_view_route_id;

[alexmos, 2016/12/13 18:41:41]

Something random and not for this CL, but it'd be nice if we used "route_id" and
"routing_id" consistently everywhere...

[ncarter (slow), 2016/12/15 00:33:16]

Acknowledged.

   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_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;
@@ skipping 24 matching lines @@
     if (!is_guest) {
       WebContentsView* new_view = new_contents->view_.get();
 
       // TODO(brettw): It seems bogus that we have to call this function on the
       // newly created object and give it one of its own member variables.
       new_view->CreateViewForWidget(
           new_contents->GetRenderViewHost()->GetWidget(), false);
     }
     // Save the created window associated with the route so we can show it
     // later.
-    DCHECK_NE(MSG_ROUTING_NONE, route_id);
-    pending_contents_[std::make_pair(render_process_id, route_id)] =
-        new_contents;
+    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);
   }
 
   if (params.opener_suppressed) {
@@ skipping 71 matching lines @@
       widget_view;
 
 #if defined(OS_MACOSX)
   // A RenderWidgetHostViewMac has lifetime scoped to the view. We'll retain it
   // to allow it to survive the trip without being hosted.
   base::mac::NSObjectRetain(widget_view->GetNativeView());
 #endif
 }
 
 void WebContentsImpl::ShowCreatedWindow(int process_id,
-                                        int route_id,
+                                        int main_frame_widget_route_id,
                                         WindowOpenDisposition disposition,
                                         const gfx::Rect& initial_rect,
                                         bool user_gesture) {
-  WebContentsImpl* contents = GetCreatedWindow(process_id, route_id);
+  WebContentsImpl* contents =
+      GetCreatedWindow(process_id, main_frame_widget_route_id);
   if (contents) {
     WebContentsDelegate* delegate = GetDelegate();
     contents->is_resume_pending_ = true;
     if (!delegate || delegate->ShouldResumeRequestsForCreatedWindow())
       contents->ResumeLoadingCreatedWebContents();
 
     if (delegate) {
-      delegate->AddNewContents(
-          this, contents, disposition, initial_rect, user_gesture, NULL);
+      delegate->AddNewContents(this, contents, disposition, initial_rect,
+                               user_gesture, NULL);
     }
+
+    RenderWidgetHostImpl* rwh = contents->GetMainFrame()->GetRenderWidgetHost();
+    DCHECK_EQ(main_frame_widget_route_id, rwh->GetRoutingID());
+    rwh->Send(new ViewMsg_Move_ACK(rwh->GetRoutingID()));
   }
 }
 
 void WebContentsImpl::ShowCreatedWidget(int process_id,
                                         int route_id,
                                         const gfx::Rect& initial_rect) {
   ShowCreatedWidget(process_id, route_id, false, initial_rect);
 }
 
 void WebContentsImpl::ShowCreatedFullscreenWidget(int process_id,
@@ skipping 46 matching lines @@
   render_widget_host_impl->set_allow_privileged_mouse_lock(is_fullscreen);
 
 #if defined(OS_MACOSX)
   // A RenderWidgetHostViewMac has lifetime scoped to the view. Now that it's
   // properly embedded (or purposefully ignored) we can release the retain we
   // took in CreateNewWidget().
   base::mac::NSObjectRelease(widget_host_view->GetNativeView());
 #endif
 }
 
-WebContentsImpl* WebContentsImpl::GetCreatedWindow(int process_id,
-                                                   int route_id) {
-  auto iter = pending_contents_.find(std::make_pair(process_id, route_id));
+WebContentsImpl* WebContentsImpl::GetCreatedWindow(
+    int process_id,
+    int main_frame_widget_route_id) {
+  auto key = std::make_pair(process_id, main_frame_widget_route_id);
+  auto iter = pending_contents_.find(key);
 
   // Certain systems can block the creation of new windows. If we didn't succeed
   // in creating one, just return NULL.
   if (iter == pending_contents_.end())
     return nullptr;
 
   WebContentsImpl* new_contents = iter->second;
-  pending_contents_.erase(std::make_pair(process_id, route_id));
+  pending_contents_.erase(key);
   RemoveDestructionObserver(new_contents);
 
   // Don't initialize the guest WebContents immediately.
   if (BrowserPluginGuest::IsGuest(new_contents))
     return new_contents;
 
-  if (!new_contents->GetRenderProcessHost()->HasConnection() ||
-      !new_contents->GetRenderViewHost()->GetWidget()->GetView())
+  if (!new_contents->GetMainFrame()->GetProcess()->HasConnection() ||

[alexmos, 2016/12/13 18:41:41]

Thanks for cleaning these up!

[ncarter (slow), 2016/12/15 00:33:17]

Done.

+      !new_contents->GetMainFrame()->GetView()) {
+    // TODO(nick): Who deletes |new_contents|?

[alexmos, 2016/12/13 18:41:41]

Good point, I don't see who deletes it either.  So if the new window's RFH
crashes before we get to show the new contents, we end up leaking it?

[ncarter (slow), 2016/12/15 00:33:16]

I filed a bug & left a breadcrumb here. The second case mentioned in the bug
(tab close) is probably actually more realistic.

[alexmos, 2016/12/15 18:36:48]

Acknowledged.  Good find with the close case.

     return nullptr;
+  }
 
   return new_contents;
 }
 
 RenderWidgetHostView* WebContentsImpl::GetCreatedWidget(int process_id,
                                                         int route_id) {
   auto iter = pending_widget_views_.find(std::make_pair(process_id, route_id));
   if (iter == pending_widget_views_.end()) {
     DCHECK(false);
     return nullptr;
@@ skipping 2940 matching lines @@
   dialog_manager_ = dialog_manager;
 }
 
 void WebContentsImpl::RemoveBindingSet(const std::string& interface_name) {
   auto it = binding_sets_.find(interface_name);
   if (it != binding_sets_.end())
     binding_sets_.erase(it);
 }
 
 }  // namespace content