Implement open_resource in non-SFI mode.

components/nacl/renderer/manifest_service_channel.cc

Index: components/nacl/renderer/manifest_service_channel.cc
diff --git a/components/nacl/renderer/manifest_service_channel.cc b/components/nacl/renderer/manifest_service_channel.cc
index f0f4adb8f940a261a4116c1f6fe9b1f292e0e4e2..32f1d5f84eb4d754822e210e0f684a1eb3c045e3 100644
--- a/components/nacl/renderer/manifest_service_channel.cc
+++ b/components/nacl/renderer/manifest_service_channel.cc
@@ -4,8 +4,10 @@
 
 #include "components/nacl/renderer/manifest_service_channel.h"
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
+#include "content/common/sandbox_util.h"
 #include "content/public/renderer/render_thread.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_sync_channel.h"
@@ -24,20 +26,29 @@ ManifestServiceChannel::ManifestServiceChannel(
       channel_(new IPC::SyncChannel(
           handle, IPC::Channel::MODE_CLIENT, this,
           content::RenderThread::Get()->GetIOMessageLoopProxy(),
-          true, waitable_event)) {
+          true, waitable_event)),
+      peer_handle_(base::kNullProcessHandle),
+      weak_ptr_factory_(this) {
 }
 
 ManifestServiceChannel::~ManifestServiceChannel() {
   if (!connected_callback_.is_null())
     base::ResetAndReturn(&connected_callback_).Run(PP_ERROR_FAILED);
+  if (peer_handle_ != base::kNullProcessHandle)
+    base::CloseProcessHandle(peer_handle_);
+}
+
+void ManifestServiceChannel::Send(IPC::Message* message) {
+  channel_->Send(message);
 }
 
 bool ManifestServiceChannel::OnMessageReceived(const IPC::Message& message) {
-  // TODO(hidehiko): Implement OpenResource.
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(ManifestServiceChannel, message)
       IPC_MESSAGE_HANDLER(PpapiHostMsg_StartupInitializationComplete,
                           OnStartupInitializationComplete)
+      IPC_MESSAGE_HANDLER_DELAY_REPLY(PpapiHostMsg_OpenResource,
+                                      OnOpenResource)
       IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -57,4 +68,31 @@ void ManifestServiceChannel::OnStartupInitializationComplete() {
   delegate_->StartupInitializationComplete();
 }
 
+void ManifestServiceChannel::OnOpenResource(
+    const std::string& key, IPC::Message* reply) {
+  delegate_->OpenResource(key,
+                          base::Bind(&ManifestServiceChannel::DidOpenResource,
+                                     weak_ptr_factory_.GetWeakPtr(), reply));
+}
+
+void ManifestServiceChannel::DidOpenResource(
+    IPC::Message* reply, int32_t pp_error,

[dmichael (off chromium), 2014/04/25 21:06:58]

style nit: Parameters should be either all on one line or each have their own
line in a function declaration or definition.

[hidehiko, 2014/04/28 08:44:27]

Done.

+    const base::PlatformFile& platform_file) {
+  PpapiHostMsg_OpenResource::WriteReplyParams(
+      reply,
+      ppapi::proxy::SerializedHandle(
+          ppapi::proxy::SerializedHandle::FILE,
+          content::BrokerGetFileHandleForProcess(
+              platform_file, PeerHandle(), true)));

[Mark Seaborn, 2014/04/26 00:18:59]

On Unix, the PeerHandle() arg is ignored...  You could use
base::FileDescriptor() instead of BrokerGetFileHandleForProcess().

This doesn't compile on Windows.  BrokerGetFileHandleForProcess() expects a
Windows process ID, not a Windows process handle.

[hidehiko, 2014/04/28 08:44:27]

Ok, then, let's ifdef.
I just tried to keep the sane platform-neutral manner in this class rather than
supporting on Windows.

If ifdef is OK, I'm fine with it. Let's revisit here, later.

+  Send(reply);
+}
+
+base::ProcessHandle ManifestServiceChannel::PeerHandle() {
+  // This function is only called from DidOpenResource(), which always runs on
+  // the same thread.
+  if (peer_handle_ == base::kNullProcessHandle)
+    base::OpenPrivilegedProcessHandle(channel_->peer_pid(), &peer_handle_);

[Mark Seaborn, 2014/04/26 00:18:59]

This only does something interesting on Windows, so I wonder if this is left
over from an earlier version of your code for SFI NaCl, or if you added it after
preparing this code for Non-SFI NaCl?

I ask because I doubt this call works in a Windows sandboxed renderer process.

peer_handle_ shouldn't be needed on Unix.

Can you remove any of the complications for building this on Windows, please? 
Since we don't need this on non-Linux systems, you can simplify the code for
now.  You can put in #ifs if necessary.

[hidehiko, 2014/04/28 08:44:27]

Removed.

+  return peer_handle_;
+}
+
 }  // namespace nacl