Skip to content

Commit

Permalink
Revert "Pepper: Fix channel init in ProxyChannel."
Browse files Browse the repository at this point in the history
This reverts:
https://src.chromium.org/viewvc/chrome?view=rev&revision=253408
because it introduces a data race. See:
https://code.google.com/p/chromium/issues/detail?id=244383

This CL might still be a good change, but there will need to
be changes to IPC::ChannelProxy first.

BUG=343768
TBR=teravest

Review URL: https://codereview.chromium.org/179873019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253978 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
dmichael@chromium.org committed Feb 27, 2014
1 parent 44d8e4f commit f7b7eb7
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 26 deletions.
5 changes: 2 additions & 3 deletions ppapi/proxy/broker_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ bool BrokerDispatcher::InitBrokerWithChannel(
base::ProcessId peer_pid,
const IPC::ChannelHandle& channel_handle,
bool is_client) {
InitWithChannel(delegate, peer_pid);
ConnectChannel(channel_handle, is_client);
return true;
return ProxyChannel::InitWithChannel(delegate, peer_pid, channel_handle,
is_client);
}

bool BrokerDispatcher::OnMessageReceived(const IPC::Message& msg) {
Expand Down
5 changes: 3 additions & 2 deletions ppapi/proxy/host_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ bool HostDispatcher::InitHostWithChannel(
const IPC::ChannelHandle& channel_handle,
bool is_client,
const ppapi::Preferences& preferences) {
InitWithChannel(delegate, peer_pid);
if (!Dispatcher::InitWithChannel(delegate, peer_pid, channel_handle,
is_client))
return false;
AddIOThreadMessageFilter(sync_status_.get());
ConnectChannel(channel_handle, is_client);

Send(new PpapiMsg_SetPreferences(preferences));
return true;
Expand Down
5 changes: 3 additions & 2 deletions ppapi/proxy/plugin_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ bool PluginDispatcher::InitPluginWithChannel(
base::ProcessId peer_pid,
const IPC::ChannelHandle& channel_handle,
bool is_client) {
InitWithChannel(delegate, peer_pid);
if (!Dispatcher::InitWithChannel(delegate, peer_pid, channel_handle,
is_client))
return false;
plugin_delegate_ = delegate;
plugin_dispatcher_id_ = plugin_delegate_->Register(this);

Expand All @@ -174,7 +176,6 @@ bool PluginDispatcher::InitPluginWithChannel(
new PluginMessageFilter(
delegate->GetGloballySeenInstanceIDSet(),
PluginGlobals::Get()->resource_reply_thread_registrar()));
ConnectChannel(channel_handle, is_client);
return true;
}

Expand Down
17 changes: 8 additions & 9 deletions ppapi/proxy/proxy_channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,18 @@ ProxyChannel::~ProxyChannel() {
DVLOG(1) << "ProxyChannel::~ProxyChannel()";
}

void ProxyChannel::InitWithChannel(Delegate* delegate,
base::ProcessId peer_pid) {
bool ProxyChannel::InitWithChannel(Delegate* delegate,
base::ProcessId peer_pid,
const IPC::ChannelHandle& channel_handle,
bool is_client) {
delegate_ = delegate;
peer_pid_ = peer_pid;
channel_.reset(new IPC::SyncChannel(this, delegate->GetIPCMessageLoop(),
delegate->GetShutdownEvent()));
}

void ProxyChannel::ConnectChannel(const IPC::ChannelHandle& channel_handle,
bool is_client) {
IPC::Channel::Mode mode = is_client ? IPC::Channel::MODE_CLIENT
: IPC::Channel::MODE_SERVER;
channel_->Init(channel_handle, mode, true);
channel_.reset(new IPC::SyncChannel(channel_handle, mode, this,
delegate->GetIPCMessageLoop(), true,
delegate->GetShutdownEvent()));
return true;
}

void ProxyChannel::InitWithTestSink(IPC::TestSink* test_sink) {
Expand Down
19 changes: 9 additions & 10 deletions ppapi/proxy/proxy_channel.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ class PPAPI_PROXY_EXPORT ProxyChannel

virtual ~ProxyChannel();

// Alternative to InitWithChannel()/ConnectChannel() for unit tests that want
// to send all messages sent via this channel to the given test sink. The
// test sink must outlive this class. In this case, the peer PID will be the
// current process ID.
// Alternative to InitWithChannel() for unit tests that want to send all
// messages sent via this channel to the given test sink. The test sink
// must outlive this class. In this case, the peer PID will be the current
// process ID.
void InitWithTestSink(IPC::TestSink* test_sink);

// Shares a file handle (HANDLE / file descriptor) with the remote side. It
Expand Down Expand Up @@ -87,14 +87,13 @@ class PPAPI_PROXY_EXPORT ProxyChannel
protected:
explicit ProxyChannel();

// You must call this function before anything else.
// You must call this function before anything else. Returns true on success.
// The delegate pointer must outlive this class, ownership is not
// transferred.
void InitWithChannel(Delegate* delegate, base::ProcessId peer_pid);

// You must call this function after InitWithChannel(), and after adding any
// desired filters to the underlying channel, but before anything else.
void ConnectChannel(const IPC::ChannelHandle& channel_handle, bool is_client);
virtual bool InitWithChannel(Delegate* delegate,
base::ProcessId peer_pid,
const IPC::ChannelHandle& channel_handle,
bool is_client);

ProxyChannel::Delegate* delegate() const {
return delegate_;
Expand Down

0 comments on commit f7b7eb7

Please sign in to comment.