Move ViewHostMsg_CreateWindow to mojom

content/browser/renderer_host/render_message_filter.cc

Index: content/browser/renderer_host/render_message_filter.cc
diff --git a/content/browser/renderer_host/render_message_filter.cc b/content/browser/renderer_host/render_message_filter.cc
index 29b8c8a1a9020d17e256e28e6c51e813e8c08fe2..6679aaf43c651887f89dcb1bdce654e8b2ee230a 100644
--- a/content/browser/renderer_host/render_message_filter.cc
+++ b/content/browser/renderer_host/render_message_filter.cc
@@ -44,6 +44,7 @@
 #include "content/common/child_process_messages.h"
 #include "content/common/content_constants_internal.h"
 #include "content/common/host_shared_bitmap_manager.h"
+#include "content/common/render_message_filter.mojom.h"
 #include "content/common/render_process_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_child_process_host.h"
@@ -166,7 +167,6 @@ RenderMessageFilter::~RenderMessageFilter() {
 bool RenderMessageFilter::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderMessageFilter, message)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWindow, OnCreateWindow)
     IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWidget, OnCreateWidget)
     IPC_MESSAGE_HANDLER(ViewHostMsg_CreateFullscreenWidget,
                         OnCreateFullscreenWidget)
@@ -240,64 +240,65 @@ void RenderMessageFilter::OverrideThreadForMessage(const IPC::Message& message,
     *thread = BrowserThread::UI;
 }
 
-void RenderMessageFilter::OnCreateWindow(
-    const ViewHostMsg_CreateWindow_Params& params,
-    ViewHostMsg_CreateWindow_Reply* reply) {
-  bool no_javascript_access;
+void RenderMessageFilter::OnCreateWidget(int opener_id,
+                                         blink::WebPopupType popup_type,
+                                         int* route_id) {
+  render_widget_helper_->CreateNewWidget(opener_id, popup_type, route_id);
+}
 
+void RenderMessageFilter::OnCreateFullscreenWidget(int opener_id,
+                                                   int* route_id) {
+  render_widget_helper_->CreateNewFullscreenWidget(opener_id, route_id);
+}
+
+void RenderMessageFilter::GenerateRoutingID(
+    const GenerateRoutingIDCallback& callback) {
+  callback.Run(render_widget_helper_->GetNextRoutingID());
+}
+
+void RenderMessageFilter::CreateNewWindow(
+    mojom::CreateNewWindowParamsPtr params,
+    const CreateNewWindowCallback& callback) {
+  bool no_javascript_access;
   bool can_create_window =
       GetContentClient()->browser()->CanCreateWindow(
-          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,
+          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,
           resource_context_,
           render_process_id_,
-          params.opener_id,
-          params.opener_render_frame_id,
+          params->opener_id,
+          params->opener_render_frame_id,
           &no_javascript_access);
 
+  mojom::CreateNewWindowReplyPtr reply = mojom::CreateNewWindowReply::New();
   if (!can_create_window) {
     reply->route_id = MSG_ROUTING_NONE;
     reply->main_frame_route_id = MSG_ROUTING_NONE;
     reply->main_frame_widget_route_id = MSG_ROUTING_NONE;
     reply->cloned_session_storage_namespace_id = 0;
-    return;
+    return callback.Run(std::move(reply));
   }
 
   // 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);
+                                      params->session_storage_namespace_id);
   reply->cloned_session_storage_namespace_id = cloned_namespace->id();
 
   render_widget_helper_->CreateNewWindow(
-      params, no_javascript_access, PeerHandle(), &reply->route_id,
+      std::move(params), no_javascript_access, PeerHandle(), &reply->route_id,
       &reply->main_frame_route_id, &reply->main_frame_widget_route_id,
       cloned_namespace.get());
-}
-
-void RenderMessageFilter::OnCreateWidget(int opener_id,
-                                         blink::WebPopupType popup_type,
-                                         int* route_id) {
-  render_widget_helper_->CreateNewWidget(opener_id, popup_type, route_id);
-}
-
-void RenderMessageFilter::OnCreateFullscreenWidget(int opener_id,
-                                                   int* route_id) {
-  render_widget_helper_->CreateNewFullscreenWidget(opener_id, route_id);
-}
-
-void RenderMessageFilter::GenerateRoutingID(
-    const GenerateRoutingIDCallback& callback) {
-  callback.Run(render_widget_helper_->GetNextRoutingID());
+  callback.Run(std::move(reply));

[ncarter (slow), 2016/09/27 20:32:41]

Can we now guarantee that the task posted to the UI thread by |CreateNewWindow|
will run before any UI-thread destined mojo message that may be sent from the
child process after the reply message is received?

[Ken Rockot(use gerrit already), 2016/09/27 21:05:41]

Yes. Any interfaces (such as mojom::RenderMessageFilter) which are associated
with the IPC::Channel  exhibit perfect ordering with respect to Channel on the
IO thread and ChannelProxy on the main thread.

To put it another way, this conversion has zero impact on the ordering of the
message or its reply.

[ncarter (slow), 2016/09/27 22:47:52]

Thanks, and awesome that we have this safety guarantee.

I was particularly worried about the ordering once the next message gets
mojofied, the one that shows the view, since it comes in (pretty weirdly!) on
the opener's interface.

 }
 
 #if defined(OS_MACOSX)