Broker out PPAPI handle duplication

BUG=127449
Review URL: https://chromiumcodereview.appspot.com/10378057

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136686 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 64 insertions, 47 deletions

diff --git a/ppapi/proxy/broker_dispatcher.cc b/ppapi/proxy/broker_dispatcher.cc
index c3f3b47..97fca5c 100644
--- a/ppapi/proxy/broker_dispatcher.cc
+++ b/ppapi/proxy/broker_dispatcher.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -12,10 +12,8 @@
 namespace ppapi {
 namespace proxy {
 
-BrokerDispatcher::BrokerDispatcher(base::ProcessHandle remote_process_handle,
-                                   PP_ConnectInstance_Func connect_instance)
-    : ProxyChannel(remote_process_handle),
-      connect_instance_(connect_instance) {
+BrokerDispatcher::BrokerDispatcher(PP_ConnectInstance_Func connect_instance)
+    : connect_instance_(connect_instance) {
 }
 
 BrokerDispatcher::~BrokerDispatcher() {
@@ -65,9 +63,8 @@ void BrokerDispatcher::OnMsgConnectToPlugin(
   }
 }
 
-BrokerHostDispatcher::BrokerHostDispatcher(
-    base::ProcessHandle remote_process_handle)
-    : BrokerDispatcher(remote_process_handle, NULL) {
+BrokerHostDispatcher::BrokerHostDispatcher()
+    : BrokerDispatcher(NULL) {
 }
 
 void BrokerHostDispatcher::OnChannelError() {
@@ -80,9 +77,8 @@ void BrokerHostDispatcher::OnChannelError() {
 }
 
 BrokerSideDispatcher::BrokerSideDispatcher(
-    base::ProcessHandle remote_process_handle,
     PP_ConnectInstance_Func connect_instance)
-    : BrokerDispatcher(remote_process_handle, connect_instance) {
+    : BrokerDispatcher(connect_instance) {
 }
 
 void BrokerSideDispatcher::OnChannelError() {
diff --git a/ppapi/proxy/broker_dispatcher.h b/ppapi/proxy/broker_dispatcher.h
index f325858..5354632 100644
--- a/ppapi/proxy/broker_dispatcher.h
+++ b/ppapi/proxy/broker_dispatcher.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -29,8 +29,7 @@ class PPAPI_PROXY_EXPORT BrokerDispatcher : public ProxyChannel {
 
  protected:
   // You must call InitBrokerWithChannel after the constructor.
-  BrokerDispatcher(base::ProcessHandle remote_process_handle,
-                   PP_ConnectInstance_Func connect_instance);
+  explicit BrokerDispatcher(PP_ConnectInstance_Func connect_instance);
 
   void OnMsgConnectToPlugin(PP_Instance instance,
                             IPC::PlatformFileForTransit handle,
@@ -45,7 +44,7 @@ class PPAPI_PROXY_EXPORT BrokerDispatcher : public ProxyChannel {
 // The dispatcher for the browser side of the broker channel.
 class PPAPI_PROXY_EXPORT BrokerHostDispatcher : public BrokerDispatcher {
  public:
-  BrokerHostDispatcher(base::ProcessHandle remote_process_handle);
+  BrokerHostDispatcher();
 
   // IPC::Channel::Listener implementation.
   virtual void OnChannelError() OVERRIDE;
@@ -54,8 +53,7 @@ class PPAPI_PROXY_EXPORT BrokerHostDispatcher : public BrokerDispatcher {
 // The dispatcher for the broker side of the broker channel.
 class PPAPI_PROXY_EXPORT BrokerSideDispatcher : public BrokerDispatcher {
  public:
-  BrokerSideDispatcher(base::ProcessHandle remote_process_handle,
-                       PP_ConnectInstance_Func connect_instance);
+  explicit BrokerSideDispatcher(PP_ConnectInstance_Func connect_instance);
 
   // IPC::Channel::Listener implementation.
   virtual void OnChannelError() OVERRIDE;
diff --git a/ppapi/proxy/dispatcher.cc b/ppapi/proxy/dispatcher.cc
index a646643..7ecd2c1 100644
--- a/ppapi/proxy/dispatcher.cc
+++ b/ppapi/proxy/dispatcher.cc
@@ -17,10 +17,8 @@
 namespace ppapi {
 namespace proxy {
 
-Dispatcher::Dispatcher(base::ProcessHandle remote_process_handle,
-                       PP_GetInterface_Func local_get_interface)
-    : ProxyChannel(remote_process_handle),
-      disallow_trusted_interfaces_(false),  // TODO(brettw) make this settable.
+Dispatcher::Dispatcher(PP_GetInterface_Func local_get_interface)
+    : disallow_trusted_interfaces_(false),  // TODO(brettw) make this settable.
       local_get_interface_(local_get_interface) {
 }
 
diff --git a/ppapi/proxy/dispatcher.h b/ppapi/proxy/dispatcher.h
index f5cc468..797fa08 100644
--- a/ppapi/proxy/dispatcher.h
+++ b/ppapi/proxy/dispatcher.h
@@ -81,8 +81,7 @@ class PPAPI_PROXY_EXPORT Dispatcher : public ProxyChannel {
   }
 
  protected:
-  Dispatcher(base::ProcessHandle remote_process_handle,
-             PP_GetInterface_Func local_get_interface);
+  explicit Dispatcher(PP_GetInterface_Func local_get_interface);
 
   // Setter for the derived classes to set the appropriate var serialization.
   // Takes one reference of the given pointer, which must be on the heap.
diff --git a/ppapi/proxy/host_dispatcher.cc b/ppapi/proxy/host_dispatcher.cc
index 8cac0ef..f9d3471 100644
--- a/ppapi/proxy/host_dispatcher.cc
+++ b/ppapi/proxy/host_dispatcher.cc
@@ -61,11 +61,10 @@ class BoolRestorer {
 
 }  // namespace
 
-HostDispatcher::HostDispatcher(base::ProcessHandle remote_process_handle,
-                               PP_Module module,
+HostDispatcher::HostDispatcher(PP_Module module,
                                PP_GetInterface_Func local_get_interface,
                                SyncMessageStatusReceiver* sync_status)
-    : Dispatcher(remote_process_handle, local_get_interface),
+    : Dispatcher(local_get_interface),
       sync_status_(sync_status),
       pp_module_(module),
       ppb_proxy_(NULL),
diff --git a/ppapi/proxy/host_dispatcher.h b/ppapi/proxy/host_dispatcher.h
index e7b5135..b43d305 100644
--- a/ppapi/proxy/host_dispatcher.h
+++ b/ppapi/proxy/host_dispatcher.h
@@ -47,8 +47,7 @@ class PPAPI_PROXY_EXPORT HostDispatcher : public Dispatcher {
   // SyncMessageStatusReceiver.
   //
   // You must call InitHostWithChannel after the constructor.
-  HostDispatcher(base::ProcessHandle host_process_handle,
-                 PP_Module module,
+  HostDispatcher(PP_Module module,
                  PP_GetInterface_Func local_get_interface,
                  SyncMessageStatusReceiver* sync_status);
   ~HostDispatcher();
diff --git a/ppapi/proxy/plugin_dispatcher.cc b/ppapi/proxy/plugin_dispatcher.cc
index 1e3fc25..ad8c620 100644
--- a/ppapi/proxy/plugin_dispatcher.cc
+++ b/ppapi/proxy/plugin_dispatcher.cc
@@ -58,10 +58,9 @@ InstanceData::~InstanceData() {
   }
 }
 
-PluginDispatcher::PluginDispatcher(base::ProcessHandle remote_process_handle,
-                                   PP_GetInterface_Func get_interface,
+PluginDispatcher::PluginDispatcher(PP_GetInterface_Func get_interface,
                                    bool incognito)
-    : Dispatcher(remote_process_handle, get_interface),
+    : Dispatcher(get_interface),
       plugin_delegate_(NULL),
       received_preferences_(false),
       plugin_dispatcher_id_(0),
diff --git a/ppapi/proxy/plugin_dispatcher.h b/ppapi/proxy/plugin_dispatcher.h
index 3e43bf2..5d344e7 100644
--- a/ppapi/proxy/plugin_dispatcher.h
+++ b/ppapi/proxy/plugin_dispatcher.h
@@ -70,8 +70,7 @@ class PPAPI_PROXY_EXPORT PluginDispatcher
   // module ID will be set upon receipt of the InitializeModule message.
   //
   // You must call InitPluginWithChannel after the constructor.
-  PluginDispatcher(base::ProcessHandle remote_process_handle,
-                   PP_GetInterface_Func get_interface,
+  PluginDispatcher(PP_GetInterface_Func get_interface,
                    bool incognito);
   virtual ~PluginDispatcher();
 
diff --git a/ppapi/proxy/ppapi_messages.h b/ppapi/proxy/ppapi_messages.h
index b40d9a8..493352d6 100644
--- a/ppapi/proxy/ppapi_messages.h
+++ b/ppapi/proxy/ppapi_messages.h
@@ -209,8 +209,7 @@ IPC_MESSAGE_CONTROL1(PpapiMsg_LoadPlugin, FilePath /* path */)
 
 // Creates a channel to talk to a renderer. The plugin will respond with
 // PpapiHostMsg_ChannelCreated.
-IPC_MESSAGE_CONTROL3(PpapiMsg_CreateChannel,
-                     base::ProcessHandle /* host_process_handle */,
+IPC_MESSAGE_CONTROL2(PpapiMsg_CreateChannel,
                      int /* renderer_id */,
                      bool /* incognito */)
 
diff --git a/ppapi/proxy/ppapi_proxy_test.cc b/ppapi/proxy/ppapi_proxy_test.cc
index e1fb6e7..445f84b 100644
--- a/ppapi/proxy/ppapi_proxy_test.cc
+++ b/ppapi/proxy/ppapi_proxy_test.cc
@@ -157,7 +157,6 @@ void PluginProxyTestHarness::SetUpHarness() {
   resource_tracker().DidCreateInstance(pp_instance());
 
   plugin_dispatcher_.reset(new PluginDispatcher(
-      base::Process::Current().handle(),
       &MockGetInterface,
       false));
   plugin_dispatcher_->InitWithTestSink(&sink());
@@ -175,7 +174,6 @@ void PluginProxyTestHarness::SetUpHarnessWithChannel(
   plugin_delegate_mock_.Init(ipc_message_loop, shutdown_event);
 
   plugin_dispatcher_.reset(new PluginDispatcher(
-      base::Process::Current().handle(),
       &MockGetInterface,
       false));
   plugin_dispatcher_->InitPluginWithChannel(&plugin_delegate_mock_,
@@ -201,6 +199,16 @@ PluginProxyTestHarness::PluginDelegateMock::GetShutdownEvent() {
   return shutdown_event_;
 }
 
+IPC::PlatformFileForTransit
+PluginProxyTestHarness::PluginDelegateMock::ShareHandleWithRemote(
+    base::PlatformFile handle,
+    const IPC::SyncChannel& /* channel */,
+    bool should_close_source) {
+  return IPC::GetFileHandleForProcess(handle,
+                                      base::Process::Current().handle(),
+                                      should_close_source);
+}
+
 std::set<PP_Instance>*
 PluginProxyTestHarness::PluginDelegateMock::GetGloballySeenInstanceIDSet() {
   return &instance_id_set_;
@@ -266,7 +274,6 @@ void HostProxyTestHarness::SetUpHarness() {
   // These must be first since the dispatcher set-up uses them.
   PpapiGlobals::SetPpapiGlobalsOnThreadForTest(GetGlobals());
   host_dispatcher_.reset(new HostDispatcher(
-      base::Process::Current().handle(),
       pp_module(),
       &MockGetInterface,
       status_receiver_.get()));
@@ -284,7 +291,6 @@ void HostProxyTestHarness::SetUpHarnessWithChannel(
   delegate_mock_.Init(ipc_message_loop, shutdown_event);
 
   host_dispatcher_.reset(new HostDispatcher(
-      base::Process::Current().handle(),
       pp_module(),
       &MockGetInterface,
       status_receiver_.get()));
@@ -308,6 +314,16 @@ base::WaitableEvent* HostProxyTestHarness::DelegateMock::GetShutdownEvent() {
   return shutdown_event_;
 }
 
+IPC::PlatformFileForTransit
+HostProxyTestHarness::DelegateMock::ShareHandleWithRemote(
+    base::PlatformFile handle,
+    const IPC::SyncChannel& /* channel */,
+    bool should_close_source) {
+  return IPC::GetFileHandleForProcess(handle,
+                                      base::Process::Current().handle(),
+                                      should_close_source);
+}
+
 
 // HostProxyTest ---------------------------------------------------------------
 
diff --git a/ppapi/proxy/ppapi_proxy_test.h b/ppapi/proxy/ppapi_proxy_test.h
index b2a3f36..59a5671 100644
--- a/ppapi/proxy/ppapi_proxy_test.h
+++ b/ppapi/proxy/ppapi_proxy_test.h
@@ -114,6 +114,10 @@ class PluginProxyTestHarness : public ProxyTestHarnessBase {
     // ProxyChannel::Delegate implementation.
     virtual base::MessageLoopProxy* GetIPCMessageLoop() OVERRIDE;
     virtual base::WaitableEvent* GetShutdownEvent() OVERRIDE;
+    virtual IPC::PlatformFileForTransit ShareHandleWithRemote(
+        base::PlatformFile handle,
+        const IPC::SyncChannel& /* channel */,
+        bool should_close_source) OVERRIDE;
 
     // PluginDispatcher::PluginDelegate implementation.
     virtual std::set<PP_Instance>* GetGloballySeenInstanceIDSet() OVERRIDE;
@@ -189,6 +193,10 @@ class HostProxyTestHarness : public ProxyTestHarnessBase {
     // ProxyChannel::Delegate implementation.
     virtual base::MessageLoopProxy* GetIPCMessageLoop();
     virtual base::WaitableEvent* GetShutdownEvent();
+    virtual IPC::PlatformFileForTransit ShareHandleWithRemote(
+        base::PlatformFile handle,
+        const IPC::SyncChannel& /* channel */,
+        bool should_close_source) OVERRIDE;
 
    private:
     base::MessageLoopProxy* ipc_message_loop_;  // Weak
diff --git a/ppapi/proxy/proxy_channel.cc b/ppapi/proxy/proxy_channel.cc
index fd5937f..c67d83c 100644
--- a/ppapi/proxy/proxy_channel.cc
+++ b/ppapi/proxy/proxy_channel.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -10,9 +10,8 @@
 namespace ppapi {
 namespace proxy {
 
-ProxyChannel::ProxyChannel(base::ProcessHandle remote_process_handle)
+ProxyChannel::ProxyChannel()
     : delegate_(NULL),
-      remote_process_handle_(remote_process_handle),
       test_sink_(NULL) {
 }
 
@@ -51,8 +50,8 @@ int ProxyChannel::TakeRendererFD() {
 IPC::PlatformFileForTransit ProxyChannel::ShareHandleWithRemote(
       base::PlatformFile handle,
       bool should_close_source) {
-  return IPC::GetFileHandleForProcess(handle, remote_process_handle_,
-                                      should_close_source);
+  return delegate_->ShareHandleWithRemote(handle, *channel_,
+                                          should_close_source);
 }
 
 bool ProxyChannel::Send(IPC::Message* msg) {
diff --git a/ppapi/proxy/proxy_channel.h b/ppapi/proxy/proxy_channel.h
index f32d470..424da94 100644
--- a/ppapi/proxy/proxy_channel.h
+++ b/ppapi/proxy/proxy_channel.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -38,6 +38,16 @@ class PPAPI_PROXY_EXPORT ProxyChannel
     // Returns the event object that becomes signalled when the main thread's
     // message loop exits.
     virtual base::WaitableEvent* GetShutdownEvent() = 0;
+
+    // Duplicates a handle to the provided object, returning one that is valid
+    // on the other side of the channel. This is part of the delegate interface
+    // because both sides of the channel may not have sufficient permission to
+    // duplicate handles directly. The implementation must provide the same
+    // guarantees as ProxyChannel::ShareHandleWithRemote below.
+    virtual IPC::PlatformFileForTransit ShareHandleWithRemote(
+        base::PlatformFile handle,
+        const IPC::SyncChannel& channel,
+        bool should_close_source) = 0;
   };
 
   virtual ~ProxyChannel();
@@ -73,7 +83,7 @@ class PPAPI_PROXY_EXPORT ProxyChannel
 #endif
 
  protected:
-  explicit ProxyChannel(base::ProcessHandle remote_process_handle);
+  explicit ProxyChannel();
 
   // You must call this function before anything else. Returns true on success.
   // The delegate pointer must outlive this class, ownership is not
@@ -90,8 +100,6 @@ class PPAPI_PROXY_EXPORT ProxyChannel
   // Non-owning pointer. Guaranteed non-NULL after init is called.
   ProxyChannel::Delegate* delegate_;
 
-  base::ProcessHandle remote_process_handle_;  // See getter above.
-
   // When we're unit testing, this will indicate the sink for the messages to
   // be deposited so they can be inspected by the test. When non-NULL, this
   // indicates that the channel should not be used.