Service CreateNewWindow on the UI thread with a new mojo interface

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index ccd8ee7a90b9ac9e586d7e55cd15a34526814c05..c16f29b1143018e9b780332d08f0b643c261107a 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -23,6 +23,7 @@
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
+#include "content/browser/dom_storage/dom_storage_context_wrapper.h"
 #include "content/browser/download/mhtml_generation_manager.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/debug_urls.h"
@@ -59,6 +60,7 @@
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/accessibility_messages.h"
 #include "content/common/associated_interface_provider_impl.h"
+#include "content/common/associated_interface_registry_impl.h"
 #include "content/common/associated_interfaces.mojom.h"
 #include "content/common/content_security_policy/content_security_policy.h"
 #include "content/common/frame_messages.h"
@@ -394,6 +396,7 @@ RenderFrameHostImpl::RenderFrameHostImpl(SiteInstance* site_instance,
       has_selection_(false),
       last_navigation_previews_state_(PREVIEWS_UNSPECIFIED),
       frame_host_binding_(this),
+      frame_host_associated_binding_(this),
       waiting_for_init_(renderer_initiated_creation),
       has_focused_editable_element_(false),
       weak_ptr_factory_(this) {
@@ -828,8 +831,12 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
 void RenderFrameHostImpl::OnAssociatedInterfaceRequest(
     const std::string& interface_name,
     mojo::ScopedInterfaceEndpointHandle handle) {
-  delegate_->OnAssociatedInterfaceRequest(
-      this, interface_name, std::move(handle));
+  if (associated_registry_->CanBindRequest(interface_name)) {
+    associated_registry_->BindRequest(interface_name, std::move(handle));
+  } else {
+    delegate_->OnAssociatedInterfaceRequest(this, interface_name,
+                                            std::move(handle));
+  }
 }
 
 void RenderFrameHostImpl::AccessibilityPerformAction(
@@ -2442,6 +2449,107 @@ void RenderFrameHostImpl::OnShowCreatedWindow(int pending_widget_routing_id,
                                disposition, initial_rect, user_gesture);
 }
 
+void RenderFrameHostImpl::CreateNewWindow(
+    mojom::CreateNewWindowParamsPtr params,
+    const CreateNewWindowCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  bool no_javascript_access = false;
+
+  bool can_create_window = GetContentClient()->browser()->CanCreateWindow(
+      this, params->opener_url, params->opener_top_level_frame_url,
+      params->opener_security_origin, params->window_container_type,
+      params->target_url, params->referrer, params->frame_name,
+      params->disposition, *params->features, params->user_gesture,
+      params->opener_suppressed, &no_javascript_access);
+
+  mojom::CreateNewWindowReplyPtr reply = mojom::CreateNewWindowReply::New();
+  if (!can_create_window) {
+    RunCreateWindowCompleteCallback(callback, std::move(reply),
+                                    MSG_ROUTING_NONE, MSG_ROUTING_NONE,
+                                    MSG_ROUTING_NONE, 0);
+    return;
+  }
+
+  // This will clone the sessionStorage for namespace_id_to_clone.
+  DOMStorageContextWrapper* dom_storage_context =
+      static_cast<DOMStorageContextWrapper*>(
+          BrowserContext::GetStoragePartition(
+              GetSiteInstance()->GetBrowserContext(), GetSiteInstance())
+              ->GetDOMStorageContext());
+  scoped_refptr<SessionStorageNamespaceImpl> cloned_namespace =
+      new SessionStorageNamespaceImpl(dom_storage_context,
+                                      params->session_storage_namespace_id);
+  reply->cloned_session_storage_namespace_id = cloned_namespace->id();
+
+  // If the opener is supppressed or script access is disallowed, we should
+  // open the window in a new BrowsingInstance, and thus a new process. That
+  // means the current renderer process will not be able to route messages to
+  // it. Because of this, we will immediately show and navigate the window
+  // in OnCreateNewWindowOnUI, using the params provided here.
+  int render_view_route_id = MSG_ROUTING_NONE;
+  int main_frame_route_id = MSG_ROUTING_NONE;
+  int main_frame_widget_route_id = MSG_ROUTING_NONE;
+  if (!params->opener_suppressed && !no_javascript_access) {
+    render_view_route_id = GetProcess()->GetNextRoutingID();
+    main_frame_route_id = GetProcess()->GetNextRoutingID();
+    // TODO(avi): When RenderViewHostImpl has-a RenderWidgetHostImpl, this
+    // should be updated to give the widget a distinct routing ID.
+    // https://crbug.com/545684
+    main_frame_widget_route_id = render_view_route_id;
+    // Block resource requests until the frame is created, since the HWND might
+    // be needed if a response ends up creating a plugin. We'll only have a
+    // single frame at this point. These requests will be resumed either in
+    // WebContentsImpl::CreateNewWindow or RenderFrameHost::Init.
+    // TODO(crbug.com/581037): Now that NPAPI is deprecated we should be able to
+    // remove this, but more investigation is needed.
+    auto block_requests_for_route = base::Bind(
+        [](const GlobalFrameRoutingId& id) {
+          auto* rdh = ResourceDispatcherHostImpl::Get();
+          if (rdh)
+            rdh->BlockRequestsForRoute(id);
+        },
+        GetGlobalFrameRoutingId());
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            block_requests_for_route);
+  }
+
+  DCHECK(IsRenderFrameLive());
+  OnCreateNewWindow(render_view_route_id, main_frame_route_id,
+                    main_frame_widget_route_id, *params,
+                    cloned_namespace.get());
+
+  // If we did not create a WebContents to host the renderer-created
+  // RenderFrame/RenderView/RenderWidget objects, make sure to send invalid
+  // routing ids back to the renderer.
+  if (main_frame_route_id != MSG_ROUTING_NONE &&

[Charlie Harrison, 2017/04/18 20:38:53]

ncarter: is this what you meant about sending MSG_ROUTING_NONE if we failed to
successfully create the new window?

[ncarter (slow), 2017/04/18 21:28:18]

Yes, this is what I was suggesting. I'll think about it more deeply as part of
the overall review but it looks correct at first glance.

(One somewhat awful corner case here is the bug fixed by this CL --
https://chromium.googlesource.com/chromium/src/+/0d558487988e9047ca7b6b26e170...
-- where, inside of OnCreateNewWindow, we actually created and deleted the
popup; this can happens in practice on Android, but unfortunately I didn't write
a test for it; IIRC it involved a window.open racing a user-initiated tab close.
But from what I can see, we're only improving things here.)

+      RenderWidgetHost::FromID(GetProcess()->GetID(),
+                               main_frame_widget_route_id) == nullptr) {
+    RunCreateWindowCompleteCallback(callback, std::move(reply),
+                                    MSG_ROUTING_NONE, MSG_ROUTING_NONE,
+                                    MSG_ROUTING_NONE, 0);
+    return;
+  }
+
+  RunCreateWindowCompleteCallback(
+      callback, std::move(reply), render_view_route_id, main_frame_route_id,
+      main_frame_widget_route_id, cloned_namespace->id());
+}
+
+void RenderFrameHostImpl::RunCreateWindowCompleteCallback(
+    const CreateNewWindowCallback& callback,
+    mojom::CreateNewWindowReplyPtr reply,
+    int render_view_route_id,
+    int main_frame_route_id,
+    int main_frame_widget_route_id,
+    int cloned_session_storage_namespace_id) {
+  reply->route_id = render_view_route_id;
+  reply->main_frame_route_id = main_frame_route_id;
+  reply->main_frame_widget_route_id = main_frame_widget_route_id;
+  reply->cloned_session_storage_namespace_id =
+      cloned_session_storage_namespace_id;
+  callback.Run(std::move(reply));
+}
+
 void RenderFrameHostImpl::RegisterMojoInterfaces() {
   device::GeolocationServiceContext* geolocation_service_context =
       delegate_ ? delegate_->GetGeolocationServiceContext() : NULL;
@@ -2890,9 +2998,17 @@ void RenderFrameHostImpl::SetUpMojoIfNeeded() {
   if (interface_registry_.get())
     return;
 
+  associated_registry_ = base::MakeUnique<AssociatedInterfaceRegistryImpl>();
   interface_registry_ = base::MakeUnique<service_manager::InterfaceRegistry>(
       mojom::kNavigation_FrameSpec);
 
+  auto make_binding = [](RenderFrameHostImpl* impl,
+                         mojom::FrameHostIPCAssociatedRequest request) {
+    impl->frame_host_associated_binding_.Bind(std::move(request));
+  };
+  static_cast<AssociatedInterfaceRegistry*>(associated_registry_.get())
+      ->AddInterface(base::Bind(make_binding, base::Unretained(this)));
+
   ServiceManagerConnection* service_manager_connection =
       BrowserContext::GetServiceManagerConnectionFor(
           GetProcess()->GetBrowserContext());