Service CreateNewWindow on the UI thread with a new mojo interface

content/common/render_message_filter.mojom

Index: content/common/render_message_filter.mojom
diff --git a/content/common/render_message_filter.mojom b/content/common/render_message_filter.mojom
index 0e6cfc3c85c1cc2ea4b4a3581ba15d85892d64ba..0eaae541a5e7b992b4a63ef91465c97ca1d0e613 100644
--- a/content/common/render_message_filter.mojom
+++ b/content/common/render_message_filter.mojom
@@ -80,12 +80,6 @@ interface RenderMessageFilter {
   // Synchronously generates a new routing ID for the caller.
   [Sync] GenerateRoutingID() => (int32 routing_id);
 
-  // Sent by the renderer when it is creating a new window.  The browser creates
-  // a tab for it.  If |reply.route_id| is MSG_ROUTING_NONE, the view couldn't
-  // be created.
-  [Sync] CreateNewWindow(CreateNewWindowParams params)
-      => (CreateNewWindowReply reply);
-
   // Similar to CreateWindow, except used for sub-widgets, like <select>
   // dropdowns.
   [Sync] CreateNewWidget(int32 opener_id, content.mojom.WebPopupType popup_type)
@@ -97,3 +91,12 @@ interface RenderMessageFilter {
 
   GetSharedBitmapManager(associated cc.mojom.SharedBitmapManager& bitmap_manager);
 };
+
+// Similar to RenderMessageFilter, but all IPCs are serviced on the UI thread.
+interface RenderMessageFilterUI {

[ncarter (slow), 2017/04/17 19:56:38]

Did you consider just routing this to the opener RenderFrameHost?

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

Yep, discussed this with dcheng@ and we considered it. I'll happily do it in
this CL if you prefer it.

[ncarter (slow), 2017/04/17 20:44:26]

I think it's preferable to use the opener here, since CreateNewWindow is
essentially an operation on the opener frame already.

Creating a UI version (RenderMessageFilterUI) of the IO helper
(RenderMessageFilter) of UI thread entity (RenderProcessHost / RenderFrameHost)
is a bit like "Street Fighter: The Movie: The Game".

One thing that at first glance gets a little tricky, is the
cleanup-after-failure logic -- the rph->Send(new ViewMsg_Close) that used to
happen in OnCreateNewWindowOnUI. However, on reflection, I think it actually
gets greatly simplified. The original complexity there arose from how we would
mint (and reply with) routing IDs on the IO thread, before validating any of the
parameters (such as the opener_ route ID) that could only be validated on the UI
thread. In that case we're left trying to cancel a half-created RenderView. But
if we mint the routing IDs on the UI thread, we can actually now validate all
those params too, and just reply with MSG_ROUTING_NONE if validation fails.

[dcheng, 2017/04/17 20:50:40]

I did some more thinking, and I think routing it via opener frame makes sense.

Originally I was concerned about the case where an iframe removes itself from
the DOM, and then tries to open a new window--but in this case already doesn't
work today, so we're not breaking anything.

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

Alright the current version attempts to route through the opener.

+  // Sent by the renderer when it is creating a new window.  The browser creates
+  // a tab for it.  If |reply.route_id| is MSG_ROUTING_NONE, the view couldn't
+  // be created.
+  [Sync] CreateNewWindow(CreateNewWindowParams params)
+      => (CreateNewWindowReply reply);
+};