Connect PPAPI IPC channels for non-SFI mode.

ppapi/proxy/plugin_main_irt.cc

Index: ppapi/proxy/plugin_main_irt.cc
diff --git a/ppapi/proxy/plugin_main_nacl.cc b/ppapi/proxy/plugin_main_irt.cc
similarity index 85%
rename from ppapi/proxy/plugin_main_nacl.cc
rename to ppapi/proxy/plugin_main_irt.cc
index 90ebfd484874e18fe77da9f949eca9f187bd3e04..90d0f9910411ab23252a053943c328f06e694b0a 100644
--- a/ppapi/proxy/plugin_main_nacl.cc
+++ b/ppapi/proxy/plugin_main_irt.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ppapi/proxy/plugin_main_irt.h"
+
 #include <map>
 #include <set>
 
@@ -11,6 +13,7 @@
 // IPC_MESSAGE_MACROS_LOG_ENABLED so ppapi_messages.h will generate the
 // ViewMsgLog et al. functions.
 
+#include "base/at_exit.h"
 #include "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string_number_conversions.h"
@@ -20,11 +23,8 @@
 #include "ipc/ipc_channel_handle.h"
 #include "ipc/ipc_logging.h"
 #include "ipc/ipc_message.h"
-#include "native_client/src/public/chrome_main.h"
-#include "native_client/src/shared/srpc/nacl_srpc.h"
 #include "ppapi/c/ppp.h"
 #include "ppapi/c/ppp_instance.h"
-#include "ppapi/native_client/src/shared/ppapi_proxy/ppruntime.h"
 #include "ppapi/proxy/plugin_dispatcher.h"
 #include "ppapi/proxy/plugin_globals.h"
 #include "ppapi/proxy/plugin_message_filter.h"
@@ -33,6 +33,11 @@
 #include "ppapi/shared_impl/ppapi_switches.h"
 #include "ppapi/shared_impl/ppb_audio_shared.h"
 
+#if defined(__native_client__)
+#include "native_client/src/public/chrome_main.h"
+#include "native_client/src/shared/srpc/nacl_srpc.h"
+#endif
+
 #if defined(IPC_MESSAGE_LOG_ENABLED)
 #include "base/containers/hash_tables.h"
 
@@ -53,6 +58,20 @@ using ppapi::proxy::SerializedHandle;
 
 namespace {
 
+#if defined(__native_client__)
+// On SFI mode, the FDs of IPC channels are NACL_CHROME_DESC_BASE and its

[Mark Seaborn, 2014/02/19 17:00:30]

"On" -> "In"

[hidehiko, 2014/02/20 18:50:01]

Done.

+// successor, which is set in nacl_listener.cc.
+int g_nacl_ipc_browser_fd = NACL_CHROME_DESC_BASE;
+int g_nacl_ipc_renderer_fd = NACL_CHROME_DESC_BASE + 1;
+#else
+// On non-SFI mode, the FDs of IPC channels are different from the hard coded

[Mark Seaborn, 2014/02/19 17:00:30]

"On" -> "In"

[hidehiko, 2014/02/20 18:50:01]

Done.

+// ones. These values will be overwritten by SetIPCFileDescriptors() below.
+// The first FD (based on NACL_CHROME_DESC_BASE) is the IPC channel to the

[Mark Seaborn, 2014/02/19 17:00:30]

This reference to NACL_CHROME_DESC_BASE is incorrect now

[hidehiko, 2014/02/20 18:50:01]

oops, fixed.

+// browser, and the second one is to the renderer.
+int g_nacl_ipc_browser_fd = -1;
+int g_nacl_ipc_renderer_fd = -1;
+#endif
+
 // This class manages communication between the plugin and the browser, and
 // manages the PluginDispatcher instances for communication between the plugin
 // and the renderer.
@@ -106,8 +125,10 @@ PpapiDispatcher::PpapiDispatcher(scoped_refptr<base::MessageLoopProxy> io_loop)
       shutdown_event_(true, false) {
   // The first FD (based on NACL_CHROME_DESC_BASE) is the IPC channel to the

[Mark Seaborn, 2014/02/19 17:00:30]

Please remove this comment now since the reference to NACL_CHROME_DESC_BASE is
outdated

[hidehiko, 2014/02/20 18:50:01]

Done.

   // browser.
+  DCHECK_NE(g_nacl_ipc_browser_fd, -1)
+      << "g_nacl_ipc_browser_fd must be initialized before the plugin starts";
   IPC::ChannelHandle channel_handle(
-      "NaCl IPC", base::FileDescriptor(NACL_CHROME_DESC_BASE, false));
+      "NaCl IPC", base::FileDescriptor(g_nacl_ipc_browser_fd, false));
   // We don't have/need a PID since handle sharing happens outside of the
   // NaCl sandbox.
   InitWithChannel(this, base::kNullProcessId, channel_handle,
@@ -231,15 +252,15 @@ void PpapiDispatcher::OnMsgInitializeNaClDispatcher(
   // The channel handle's true name is not revealed here.
   // The second FD (based on NACL_CHROME_DESC_BASE) is the IPC channel to the

[Mark Seaborn, 2014/02/19 17:00:30]

Same for this sentence

[hidehiko, 2014/02/20 18:50:01]

Done.

   // renderer.
+  DCHECK_NE(g_nacl_ipc_renderer_fd, -1)
+      << "g_nacl_ipc_renderer_fd must be initialized before the plugin starts";
   IPC::ChannelHandle channel_handle(
-      "nacl", base::FileDescriptor(NACL_CHROME_DESC_BASE + 1, false));
+      "nacl", base::FileDescriptor(g_nacl_ipc_renderer_fd, false));
   if (!dispatcher->InitPluginWithChannel(this, base::kNullProcessId,
                                          channel_handle, false)) {
     delete dispatcher;
     return;
   }
-  // From here, the dispatcher will manage its own lifetime according to the

[Mark Seaborn, 2014/02/19 17:00:30]

Why remove this?

[hidehiko, 2014/02/20 18:50:01]

Oops.. I somehow accidentally removed this. Reverted.

-  // lifetime of the attached channel.
 }
 
 void PpapiDispatcher::OnPluginDispatcherMessageReceived(
@@ -271,24 +292,43 @@ void PpapiDispatcher::SetPpapiKeepAliveThrottleFromCommandLine() {
 
 }  // namespace
 
+void SetIPCFileDescriptors(int ipc_browser_fd, int ipc_renderer_fd) {
+  // For non-SFI mode, the browser_ipc_fd and renderer_ipc_fd are different
+  // from the hard-coded numbers. So, this overwrites the values directly.

[Mark Seaborn, 2014/02/19 17:00:30]

With the "#if defined(__native_client__)" above, this isn't overwriting
hard-coded numbers.  You could just remove this comment since it's pretty
self-explanatory.

[hidehiko, 2014/02/20 18:50:01]

Done.

+  g_nacl_ipc_browser_fd = ipc_browser_fd;
+  g_nacl_ipc_renderer_fd = ipc_renderer_fd;
+}
+
 void PpapiPluginRegisterThreadCreator(
     const struct PP_ThreadFunctions* thread_functions) {
+#if defined(__native_client__)
+  // TODO(hidehiko): The thread creation for the PPB_Audio is not yet
+  // implemented on non-SFI mode. Support this. Now, this function invocation
+  // is just ignored.
+
   // Initialize all classes that need to create threads that call back into
   // user code.
   ppapi::PPB_Audio_Shared::SetThreadFunctions(thread_functions);
+#endif
 }
 
 int PpapiPluginMain() {
   // Though it isn't referenced here, we must instantiate an AtExitManager.
   base::AtExitManager exit_manager;
   base::MessageLoop loop;
+#if defined(IPC_MESSAGE_LOG_ENABLED)
   IPC::Logging::set_log_function_map(&g_log_function_mapping);
+#endif
   ppapi::proxy::PluginGlobals plugin_globals;
   base::Thread io_thread("Chrome_NaClIOThread");
   base::Thread::Options options;
   options.message_loop_type = base::MessageLoop::TYPE_IO;
   io_thread.StartWithOptions(options);
 
+#if defined(__native_client__)
+  // Currently on non-SFI mode, we don't use SRPC server on plugin.
+  // TODO(hidehiko): Make sure this SRPC is actually used on SFI-mode.
+
   // Start up the SRPC server on another thread. Otherwise, when it blocks
   // on an RPC, the PPAPI proxy will hang. Do this before we initialize the
   // module and start the PPAPI proxy so that the NaCl plugin can continue
@@ -297,6 +337,7 @@ int PpapiPluginMain() {
   if (!NaClSrpcAcceptClientOnThread(srpc_methods)) {
     return 1;
   }
+#endif
 
   PpapiDispatcher ppapi_dispatcher(io_thread.message_loop_proxy());
   plugin_globals.set_plugin_proxy_delegate(&ppapi_dispatcher);