Prototype: Use Chromium IPC for plugin LOAD_MODULE.

components/nacl/browser/nacl_process_host.cc

 // Copyright (c) 2012 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 "components/nacl/browser/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 555 matching lines @@
     IPC_MESSAGE_HANDLER(NaClProcessMsg_SetKnownToValidate,
                         OnSetKnownToValidate)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(NaClProcessMsg_ResolveFileToken,
                                     OnResolveFileToken)
 #if defined(OS_WIN)
     IPC_MESSAGE_HANDLER_DELAY_REPLY(NaClProcessMsg_AttachDebugExceptionHandler,
                                     OnAttachDebugExceptionHandler)
 #endif
     IPC_MESSAGE_HANDLER(NaClProcessHostMsg_PpapiChannelCreated,
                         OnPpapiBrowserChannelCreated)
+    IPC_MESSAGE_HANDLER(NaClProcessHostMsg_TrustedPluginChannelCreated,
+                        OnTrustedPluginChannelCreated)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void NaClProcessHost::OnProcessLaunched() {
   if (!StartWithLaunchedProcess())
     delete this;
 }
 
 // Called when the NaClBrowser singleton has been fully initialized.
 void NaClProcessHost::OnResourcesReady() {
   NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
   if (!nacl_browser->IsReady()) {
     SendErrorToRenderer("could not acquire shared resources needed by NaCl");
     delete this;
   } else if (!SendStart()) {
     delete this;
   }
 }
 
-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
   // need to inform the renderer's sandbox about the NaCl process so
   // that the renderer can send handles to the NaCl process using
   // BrokerDuplicateHandle().
   if (RunningOnWOW64()) {
     if (!content::BrokerAddTargetPeer(process_->GetData().handle)) {
       SendErrorToRenderer("BrokerAddTargetPeer() failed");
       return false;
@@ skipping 21 matching lines @@
 #else
   // No need to dup the imc_handle - we don't pass it anywhere else so
   // it cannot be closed.
   FileDescriptor imc_handle;
   imc_handle.fd = internal_->socket_for_renderer;
   imc_handle.auto_close = true;
   handle_for_renderer = imc_handle;
 #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;
 }
 
 void NaClProcessHost::SendErrorToRenderer(const std::string& error_message) {
   LOG(ERROR) << "NaCl process launch failed: " << error_message;
   SendMessageToRenderer(NaClLaunchResult(), error_message);
 }
 
@@ skipping 116 matching lines @@
 #endif
 
   process_->Send(new NaClProcessMsg_Start(params));
 
   internal_->socket_for_sel_ldr = NACL_INVALID_HANDLE;
   return true;
 }
 
 bool NaClProcessHost::SendStart() {
   if (!enable_ppapi_proxy()) {
-    if (!ReplyToRenderer(IPC::ChannelHandle()))
+    if (!ReplyToRenderer())
       return false;
   }
   return StartNaClExecution();
 }
 
 // This method is called when NaClProcessHostMsg_PpapiChannelCreated is
 // received.
 void NaClProcessHost::OnPpapiBrowserChannelCreated(
     const IPC::ChannelHandle& channel_handle) {
   // Only renderer processes should create a channel.
@@ skipping 49 matching lines @@
             args,
             SerializedHandle(SerializedHandle::CHANNEL_HANDLE,
                              IPC::InvalidPlatformFileForTransit())));
   } else {
     // Attempt to open more than 1 browser channel is not supported.
     // Shut down the NaCl process.
     process_->GetHost()->ForceShutdown();
   }
 }
 
+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();
+    }
   }
 }
 
 bool NaClProcessHost::OnUntrustedMessageForwarded(const IPC::Message& msg) {
   // Handle messages that have been forwarded from our PluginListener.
   // These messages come from untrusted code so should be handled with care.
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NaClProcessHost, msg)
     IPC_MESSAGE_HANDLER(PpapiHostMsg_NaClChannelCreated,
                         OnPpapiRendererChannelCreated)
@@ skipping 169 matching lines @@
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl