From f0ecb550248afddff9a28c8bf645e689f59a6e3e Mon Sep 17 00:00:00 2001 From: "jschuh@chromium.org" Date: Fri, 11 May 2012 22:09:11 +0000 Subject: 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 --- ppapi/proxy/broker_dispatcher.cc | 16 ++++++---------- ppapi/proxy/broker_dispatcher.h | 10 ++++------ ppapi/proxy/dispatcher.cc | 6 ++---- ppapi/proxy/dispatcher.h | 3 +-- ppapi/proxy/host_dispatcher.cc | 5 ++--- ppapi/proxy/host_dispatcher.h | 3 +-- ppapi/proxy/plugin_dispatcher.cc | 5 ++--- ppapi/proxy/plugin_dispatcher.h | 3 +-- ppapi/proxy/ppapi_messages.h | 3 +-- ppapi/proxy/ppapi_proxy_test.cc | 24 ++++++++++++++++++++---- ppapi/proxy/ppapi_proxy_test.h | 8 ++++++++ ppapi/proxy/proxy_channel.cc | 9 ++++----- ppapi/proxy/proxy_channel.h | 16 ++++++++++++---- 13 files changed, 64 insertions(+), 47 deletions(-) (limited to 'ppapi') 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* 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* 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. -- cgit v1.1