Prototype: Use Chromium IPC for plugin LOAD_MODULE.

components/nacl/browser/nacl_process_host.cc

Index: components/nacl/browser/nacl_process_host.cc
diff --git a/components/nacl/browser/nacl_process_host.cc b/components/nacl/browser/nacl_process_host.cc
index e6e8ef450177a7df62ee5ecad6db1c6c26430689..eb89043bf4f075877bbbf1ad66be56513bea45c4 100644
--- a/components/nacl/browser/nacl_process_host.cc
+++ b/components/nacl/browser/nacl_process_host.cc
@@ -573,6 +573,8 @@ bool NaClProcessHost::OnMessageReceived(const IPC::Message& msg) {
 #endif
     IPC_MESSAGE_HANDLER(NaClProcessHostMsg_PpapiChannelCreated,
                         OnPpapiBrowserChannelCreated)
+    IPC_MESSAGE_HANDLER(NaClProcessHostMsg_TrustedPluginChannelCreated,
+                        OnTrustedPluginChannelCreated)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -594,8 +596,8 @@ void NaClProcessHost::OnResourcesReady() {
   }
 }
 
-bool NaClProcessHost::ReplyToRenderer(
-    const IPC::ChannelHandle& channel_handle) {
+bool NaClProcessHost::ReplyToRenderer() {
+  fprintf(stderr, "NaClProcessHost::ReplyToRenderer\n");
 #if defined(OS_WIN)
   // If we are on 64-bit Windows, the NaCl process's sandbox is
   // managed by a different process from the renderer's sandbox.  We
@@ -637,11 +639,15 @@ bool NaClProcessHost::ReplyToRenderer(
 #endif
 
   const ChildProcessData& data = process_->GetData();
+  fprintf(stderr, "about to send message to renderer, "
+                  "trusted plugin channel handle name: %s\n",
+                  nacl_renderer_channel_handle_.name.c_str());
   SendMessageToRenderer(
       NaClLaunchResult(handle_for_renderer,
-                             channel_handle,
-                             base::GetProcId(data.handle),
-                             data.id),
+                       ppapi_renderer_channel_handle_,
+                       nacl_renderer_channel_handle_,
+                       base::GetProcId(data.handle),
+                       data.id),
       std::string() /* error_message */);
   internal_->socket_for_renderer = NACL_INVALID_HANDLE;
   return true;
@@ -778,7 +784,7 @@ bool NaClProcessHost::StartNaClExecution() {
 
 bool NaClProcessHost::SendStart() {
   if (!enable_ppapi_proxy()) {
-    if (!ReplyToRenderer(IPC::ChannelHandle()))
+    if (!ReplyToRenderer())
       return false;
   }
   return StartNaClExecution();
@@ -848,14 +854,34 @@ void NaClProcessHost::OnPpapiBrowserChannelCreated(
   }
 }
 
+void NaClProcessHost::OnTrustedPluginChannelCreated(
+    const IPC::ChannelHandle& channel_handle) {
+  fprintf(stderr, "NaClProcessHost::OnTrustedPluginChannelCreated\n");
+  fprintf(stderr, "handle name: %s\n", channel_handle.name.c_str());
+  nacl_renderer_channel_handle_ = channel_handle;
+  if (!ppapi_renderer_channel_handle_.name.empty()) {
+    if (reply_msg_) {
+      ReplyToRenderer();

[dmichael (off chromium), 2014/02/06 18:18:47]

This looks iffy...  you do ReplyToRenderer whenever the first channel has been
created, without waiting for the second. Then you pass both channel handles,
even though one of them may not be ready yet. It seems like you should wait
until both channels have been created before sending the reply. Could this be
the source of some of the races you were seeing?

[teravest1, 2014/02/06 18:21:15]

I do wait until both channels have been created before sending the reply.

OnTrustedPluginChannelCreated
  sets nacl_renderer_channel_handle_
  and checks that ppapi_renderer_channel_handle_ is valid before sending.

OnPpapiRendererChannelCreated
  sets ppapi_renderer_channel_handle_
  and checks that nacl_renderer_channel_handle_ is valid before sending.

Am I misunderstanding you?

Regardless, the race issue I was discussing earlier doesn't happen with this
patch.

[dmichael (off chromium), 2014/02/06 18:24:22]

D'oh, okay, I see that now. Sorry. I should either drink more coffee or less
alcohol.

+    } else {
+      // Attempt to open more than 1 NaCl renderer channel is not supported.

[dmichael (off chromium), 2014/02/06 18:18:47]

nit: "Attempt to open"->"Opening"?

+      // Shut down the NaCl process.
+      process_->GetHost()->ForceShutdown();
+    }
+  }
+}
+
 void NaClProcessHost::OnPpapiRendererChannelCreated(
     const IPC::ChannelHandle& channel_handle) {
-  if (reply_msg_) {
-    ReplyToRenderer(channel_handle);
-  } else {
-    // Attempt to open more than 1 NaCl renderer channel is not supported.
-    // Shut down the NaCl process.
-    process_->GetHost()->ForceShutdown();
+  fprintf(stderr, "NaClProcessHost::OnPpapiRendererChannelCreated\n");
+  ppapi_renderer_channel_handle_ = channel_handle;
+  if (!nacl_renderer_channel_handle_.name.empty()) {
+    if (reply_msg_) {
+      ReplyToRenderer();
+    } else {
+      // Attempt to open more than 1 NaCl renderer channel is not supported.
+      // Shut down the NaCl process.
+      process_->GetHost()->ForceShutdown();
+    }
   }
 }