Service CreateNewWindow on the UI thread with a new mojo interface

content/browser/renderer_host/render_message_filter_ui.cc

Index: content/browser/renderer_host/render_message_filter_ui.cc
diff --git a/content/browser/renderer_host/render_message_filter_ui.cc b/content/browser/renderer_host/render_message_filter_ui.cc
new file mode 100644
index 0000000000000000000000000000000000000000..0d95362c992241dee075bae7ecae61c10c46847c
--- /dev/null
+++ b/content/browser/renderer_host/render_message_filter_ui.cc
@@ -0,0 +1,145 @@
+// Copyright 2017 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 <memory>
+#include <utility>
+
+#include "base/memory/ptr_util.h"
+#include "content/browser/dom_storage/dom_storage_context_wrapper.h"
+#include "content/browser/dom_storage/session_storage_namespace_impl.h"
+#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/loader/resource_dispatcher_host_impl.h"
+#include "content/browser/renderer_host/render_message_filter_ui.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/common/view_messages.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host.h"
+#include "content/public/common/content_client.h"
+#include "ipc/ipc_message.h"
+#include "mojo/public/cpp/bindings/strong_associated_binding.h"
+
+namespace content {
+
+void RenderMessageFilterUI::CreateFilter(
+    RenderProcessHost* rph,
+    DOMStorageContextWrapper* dom_storage_context,
+    mojom::RenderMessageFilterUIAssociatedRequest request) {
+  mojo::MakeStrongAssociatedBinding(
+      base::MakeUnique<RenderMessageFilterUI>(rph, dom_storage_context),
+      std::move(request));
+}
+
+RenderMessageFilterUI::RenderMessageFilterUI(
+    RenderProcessHost* render_process_host,
+    DOMStorageContextWrapper* dom_storage_context)
+    : render_process_host_(render_process_host),
+      dom_storage_context_(dom_storage_context) {}
+
+RenderMessageFilterUI::~RenderMessageFilterUI() {}
+
+void RenderMessageFilterUI::CreateNewWindow(
+    mojom::CreateNewWindowParamsPtr params,
+    const CreateNewWindowCallback& callback) {
+  bool no_javascript_access = false;
+  int render_process_id = render_process_host_->GetID();
+
+  bool can_create_window = GetContentClient()->browser()->CanCreateWindow(
+      render_process_id, params->opener_render_frame_id, 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.
+  scoped_refptr<SessionStorageNamespaceImpl> cloned_namespace =
+      new SessionStorageNamespaceImpl(dom_storage_context_.get(),
+                                      params->session_storage_namespace_id);
+  reply->cloned_session_storage_namespace_id = cloned_namespace->id();
+
+  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) {
+    // 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.
+  } else {
+    render_view_route_id = render_process_host_->GetNextRoutingID();
+    main_frame_route_id = render_process_host_->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.
+    auto block_requests_for_route = base::Bind(
+        [](const GlobalFrameRoutingId& id) {
+          auto* rdh = ResourceDispatcherHostImpl::Get();
+          if (rdh)
+            rdh->BlockRequestsForRoute(id);
+        },
+        GlobalFrameRoutingId(render_process_id, main_frame_route_id));
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            block_requests_for_route);

[alexmos, 2017/04/17 19:41:08]

Is this ok from race conditions point of view?  I don't really know the history
of this check, but is it possible for a request (I guess from a plugin, given
the comment above?) to come in before block_requests_for_route gets posted to
the IO thread? 

[Charlie Harrison, 2017/04/17 20:10:52]

I filed crbug.com/581037 a while ago trying to rip this logic out. AFAIK we
don't need this for the plugin case since the NPAPI deprecation (plugins no
longer need the HWND). When I briefly tried to remove it the only thing failing
was <webview> (as far as I remember).


From a pure race condition perspective I think we should be fine, given that a
task posted from here will be strictly before any IPC handler task for a
resource request. At least, that's how I assume IPC message handlers work but if
I'm wrong we could be in more trouble.

[alexmos, 2017/04/19 22:28:02]

Acknowledged.  Thanks for adding a comment with that reference.

+  }
+
+  RenderFrameHostImpl* opener = RenderFrameHostImpl::FromID(
+      render_process_id, params->opener_render_frame_id);
+  if (opener && opener->IsRenderFrameLive()) {
+    opener->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, destroy them.
+  if (main_frame_route_id != MSG_ROUTING_NONE) {
+    bool succeeded =
+        RenderWidgetHost::FromID(render_process_id,
+                                 main_frame_widget_route_id) != nullptr;
+    if (!succeeded) {
+      DCHECK(!RenderFrameHost::FromID(render_process_id, main_frame_route_id));
+      DCHECK(!RenderViewHost::FromID(render_process_id, render_view_route_id));
+
+      render_process_host_->Send(new ViewMsg_Close(render_view_route_id));
+    } else {
+      // If a RWH was created, there should also be an RFH and RVH.
+      DCHECK(RenderFrameHost::FromID(render_process_id, main_frame_route_id));
+      DCHECK(RenderViewHost::FromID(render_process_id, render_view_route_id));
+    }
+  }
+  RunCreateWindowCompleteCallback(
+      callback, std::move(reply), render_view_route_id, main_frame_route_id,
+      main_frame_widget_route_id, cloned_namespace->id());
+}
+
+void RenderMessageFilterUI::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));
+}
+
+}  // namespace content