Service CreateNewWindow on the UI thread with a new mojo interface

content/common/frame.mojom

Index: content/common/frame.mojom
diff --git a/content/common/frame.mojom b/content/common/frame.mojom
index affcff1f5cbb82374823444bcbd30a46388c8e84..c33c1504d761831d760c6a30e14ad9e45e52fb9a 100644
--- a/content/common/frame.mojom
+++ b/content/common/frame.mojom
@@ -4,7 +4,12 @@
 
 module content.mojom;
 
+import "content/public/common/window_container_type.mojom";
 import "services/service_manager/public/interfaces/interface_provider.mojom";
+import "third_party/WebKit/public/platform/referrer.mojom";
+import "third_party/WebKit/public/web/window_features.mojom";
+import "ui/base/mojo/window_open_disposition.mojom";
+import "url/mojo/url.mojom";
 
 // The name of the InterfaceProviderSpec in service manifests used by the
 // frame tree to expose frame-specific interfaces between renderer and browser.
@@ -35,3 +40,76 @@ interface FrameHost {
 interface FrameFactory {
   CreateFrame(int32 frame_routing_id, Frame& frame, FrameHost host);
 };
+
+// TODO(csharrison): Clean up this struct. Some of the entries (like
+// opener_top_level_frame_url) are better suited to be computed in the browser
+// process.

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

Mention http://crbug.com/674307

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

Done.

+struct CreateNewWindowParams {
+  // True if this open request came in the context of a user gesture.
+  bool user_gesture;
+
+  // Type of window requested.
+  WindowContainerType window_container_type;
+
+  // The session storage namespace ID this view should use.

[nasko, 2017/04/20 18:58:41]

nit: Let's move away from "view". Maybe "window", since this is what we are
creating and session storage is window/tab global.

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

Done.

+  int64 session_storage_namespace_id;
+
+  // The name of the resulting frame that should be created (empty if none
+  // has been specified). UTF8 encoded string.
+  string frame_name;
+
+  // The URL of the frame initiating the open.
+  url.mojom.Url opener_url;
+
+  // The URL of the top frame containing the opener.
+  url.mojom.Url opener_top_level_frame_url;
+
+  // The security origin of the frame initiating the open.
+  url.mojom.Url opener_security_origin;
+
+  // Whether the opener will be suppressed in the new window, in which case
+  // scripting the new window is not allowed.
+  bool opener_suppressed;
+
+  // Whether the window should be opened in the foreground, background, etc.
+  ui.mojom.WindowOpenDisposition disposition;
+
+  // The URL that will be loaded in the new window (empty if none has been
+  // specified).
+  url.mojom.Url target_url;
+
+  // The referrer that will be used to load |target_url| (empty if none has
+  // been specified).
+  blink.mojom.Referrer referrer;
+
+  // The window features to use for the new view.
+  blink.mojom.WindowFeatures features;
+};
+
+struct CreateNewWindowReply {
+  // The ID of the view to be created. If the ID is MSG_ROUTING_NONE, then the
+  // opener RenderFrame should not create a RenderView in its process.
+  // MSG_ROUTING_NONE does not necessarily indicate failure; it may also occur
+  // in cases where a window was created, but the opener relationship is
+  // severed.
+  int32 route_id;
+
+  // The ID of the main frame hosted in the view.
+  int32 main_frame_route_id;
+
+  // The ID of the widget for the main frame.
+  int32 main_frame_widget_route_id;
+
+  // Duplicated from CreateNewWindowParams because legacy code.
+  int64 cloned_session_storage_namespace_id;
+};
+
+// Implemented by the frame server (i.e. the browser process). For messages that
+// must be associated with the IPC channel.
+interface FrameHostIPC {

[nasko, 2017/04/20 18:58:41]

Why do we need a new interface here? It seems to me that FrameHost is the main
interface where RenderFrame and RenderFrameHost will be connected together, so
adding one just for the IPCs seems a bit strange. Also, as we move more to mojo,
I'm afraid that all legacy IPCs will move to FrameHostIPC and FrameHost will be
an empty interface as it is now.

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

I'm neutral on whether we just add methods to FrameHost, or have a FrameHostIPC
for messages that are handled directly by the RenderFrameHostImpl.

If we're not sure, we can ask chromium-mojo for advice.

If we do add the extra layer, I think 'FrameHostImpl' might be a reasonable
choice of name.

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

Yeah I could go both ways, I'll post in chromium-mojo if rockot@ doesn't have a
strong opinion.

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

Ack, addressed earlier.

+  // 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

[nasko, 2017/04/20 18:58:41]

nit: s/view/window/?

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

Done.

+  // be created.
+  [Sync] CreateNewWindow(CreateNewWindowParams params)
+      => (CreateNewWindowReply reply);
+};