summaryrefslogtreecommitdiffstats
path: root/ipc
diff options
context:
space:
mode:
authormorrita <morrita@chromium.org>2015-04-08 16:42:31 -0700
committerCommit bot <commit-bot@chromium.org>2015-04-08 23:43:02 +0000
commit808706e71c213c916815e967e3156cae64d12c00 (patch)
tree3d07537ac9fe66cac5162559cfedb47e0baea1ee /ipc
parent378f8be1fc36b0878810f2cfeb9da8436cf95313 (diff)
downloadchromium_src-808706e71c213c916815e967e3156cae64d12c00.zip
chromium_src-808706e71c213c916815e967e3156cae64d12c00.tar.gz
chromium_src-808706e71c213c916815e967e3156cae64d12c00.tar.bz2
ChannelMojo: Ensure that it always has ScopedIPCSupport
ChannelMojo has ScopedIPCSupport, but it is instantiated only in in-process mode. This CL lets it always instantiate to make it clear that ChannelInfo is protected by the ScopedIPCSupport. It simplifies the relationship between the support object and the channel, and makes the lifecycle invariant reasonable. With this change, we no longer need to protect ChannelMojo with ScopedIPCSupport on its client side. Now it's built-in. Note that this is a speculative fix of fuzzer generated crash, where Mojo channel related globals are gone before when channel mojo is being destroyed. BUG=473438 R=viettrungluu@chromium.org, rockot@chromium.org Review URL: https://codereview.chromium.org/1054253005 Cr-Commit-Position: refs/heads/master@{#324308}
Diffstat (limited to 'ipc')
-rw-r--r--ipc/mojo/ipc_channel_mojo.cc63
-rw-r--r--ipc/mojo/ipc_channel_mojo.h16
-rw-r--r--ipc/mojo/ipc_channel_mojo_host.cc6
-rw-r--r--ipc/mojo/ipc_channel_mojo_unittest.cc19
-rw-r--r--ipc/mojo/ipc_mojo_perftest.cc17
5 files changed, 57 insertions, 64 deletions
diff --git a/ipc/mojo/ipc_channel_mojo.cc b/ipc/mojo/ipc_channel_mojo.cc
index f338c6f..4689e5f 100644
--- a/ipc/mojo/ipc_channel_mojo.cc
+++ b/ipc/mojo/ipc_channel_mojo.cc
@@ -28,20 +28,26 @@ namespace {
class MojoChannelFactory : public ChannelFactory {
public:
MojoChannelFactory(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
ChannelHandle channel_handle,
Channel::Mode mode)
- : delegate_(delegate), channel_handle_(channel_handle), mode_(mode) {}
+ : delegate_(delegate),
+ io_runner_(io_runner),
+ channel_handle_(channel_handle),
+ mode_(mode) {}
std::string GetName() const override {
return channel_handle_.name;
}
scoped_ptr<Channel> BuildChannel(Listener* listener) override {
- return ChannelMojo::Create(delegate_, channel_handle_, mode_, listener);
+ return ChannelMojo::Create(delegate_, io_runner_, channel_handle_, mode_,
+ listener);
}
private:
ChannelMojo::Delegate* delegate_;
+ scoped_refptr<base::TaskRunner> io_runner_;
ChannelHandle channel_handle_;
Channel::Mode mode_;
};
@@ -53,6 +59,7 @@ class ClientChannelMojo : public ChannelMojo,
public mojo::ErrorHandler {
public:
ClientChannelMojo(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& handle,
Listener* listener);
~ClientChannelMojo() override;
@@ -73,9 +80,10 @@ class ClientChannelMojo : public ChannelMojo,
};
ClientChannelMojo::ClientChannelMojo(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& handle,
Listener* listener)
- : ChannelMojo(delegate, handle, Channel::MODE_CLIENT, listener),
+ : ChannelMojo(delegate, io_runner, handle, Channel::MODE_CLIENT, listener),
binding_(this) {
}
@@ -104,6 +112,7 @@ void ClientChannelMojo::Init(
class ServerChannelMojo : public ChannelMojo, public mojo::ErrorHandler {
public:
ServerChannelMojo(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& handle,
Listener* listener);
~ServerChannelMojo() override;
@@ -126,9 +135,10 @@ class ServerChannelMojo : public ChannelMojo, public mojo::ErrorHandler {
};
ServerChannelMojo::ServerChannelMojo(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& handle,
Listener* listener)
- : ChannelMojo(delegate, handle, Channel::MODE_SERVER, listener) {
+ : ChannelMojo(delegate, io_runner, handle, Channel::MODE_SERVER, listener) {
}
ServerChannelMojo::~ServerChannelMojo() {
@@ -196,17 +206,19 @@ bool ChannelMojo::ShouldBeUsed() {
}
// static
-scoped_ptr<ChannelMojo> ChannelMojo::Create(ChannelMojo::Delegate* delegate,
- const ChannelHandle& channel_handle,
- Mode mode,
- Listener* listener) {
+scoped_ptr<ChannelMojo> ChannelMojo::Create(
+ ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
+ const ChannelHandle& channel_handle,
+ Mode mode,
+ Listener* listener) {
switch (mode) {
case Channel::MODE_CLIENT:
return make_scoped_ptr(
- new ClientChannelMojo(delegate, channel_handle, listener));
+ new ClientChannelMojo(delegate, io_runner, channel_handle, listener));
case Channel::MODE_SERVER:
return make_scoped_ptr(
- new ServerChannelMojo(delegate, channel_handle, listener));
+ new ServerChannelMojo(delegate, io_runner, channel_handle, listener));
default:
NOTREACHED();
return nullptr;
@@ -216,20 +228,23 @@ scoped_ptr<ChannelMojo> ChannelMojo::Create(ChannelMojo::Delegate* delegate,
// static
scoped_ptr<ChannelFactory> ChannelMojo::CreateServerFactory(
ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& channel_handle) {
- return make_scoped_ptr(
- new MojoChannelFactory(delegate, channel_handle, Channel::MODE_SERVER));
+ return make_scoped_ptr(new MojoChannelFactory(
+ delegate, io_runner, channel_handle, Channel::MODE_SERVER));
}
// static
scoped_ptr<ChannelFactory> ChannelMojo::CreateClientFactory(
ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& channel_handle) {
- return make_scoped_ptr(
- new MojoChannelFactory(delegate, channel_handle, Channel::MODE_CLIENT));
+ return make_scoped_ptr(new MojoChannelFactory(
+ delegate, io_runner, channel_handle, Channel::MODE_CLIENT));
}
ChannelMojo::ChannelMojo(ChannelMojo::Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& handle,
Mode mode,
Listener* listener)
@@ -240,16 +255,12 @@ ChannelMojo::ChannelMojo(ChannelMojo::Delegate* delegate,
// Create MojoBootstrap after all members are set as it touches
// ChannelMojo from a different thread.
bootstrap_ = MojoBootstrap::Create(handle, mode, this);
- if (delegate) {
- if (delegate->GetIOTaskRunner() ==
- base::MessageLoop::current()->message_loop_proxy()) {
- InitDelegate(delegate);
- } else {
- delegate->GetIOTaskRunner()->PostTask(
- FROM_HERE,
- base::Bind(
- &ChannelMojo::InitDelegate, base::Unretained(this), delegate));
- }
+ if (io_runner == base::MessageLoop::current()->message_loop_proxy()) {
+ InitOnIOThread(delegate);
+ } else {
+ io_runner->PostTask(FROM_HERE,
+ base::Bind(&ChannelMojo::InitOnIOThread,
+ base::Unretained(this), delegate));
}
}
@@ -257,9 +268,11 @@ ChannelMojo::~ChannelMojo() {
Close();
}
-void ChannelMojo::InitDelegate(ChannelMojo::Delegate* delegate) {
+void ChannelMojo::InitOnIOThread(ChannelMojo::Delegate* delegate) {
ipc_support_.reset(
new ScopedIPCSupport(base::MessageLoop::current()->task_runner()));
+ if (!delegate)
+ return;
delegate_ = delegate->ToWeakPtr();
delegate_->OnChannelCreated(weak_factory_.GetWeakPtr());
}
diff --git a/ipc/mojo/ipc_channel_mojo.h b/ipc/mojo/ipc_channel_mojo.h
index 1959e0f..40bc256 100644
--- a/ipc/mojo/ipc_channel_mojo.h
+++ b/ipc/mojo/ipc_channel_mojo.h
@@ -54,7 +54,6 @@ class IPC_MOJO_EXPORT ChannelMojo
public:
virtual ~Delegate() {}
virtual base::WeakPtr<Delegate> ToWeakPtr() = 0;
- virtual scoped_refptr<base::TaskRunner> GetIOTaskRunner() = 0;
virtual void OnChannelCreated(base::WeakPtr<ChannelMojo> channel) = 0;
};
@@ -63,20 +62,24 @@ class IPC_MOJO_EXPORT ChannelMojo
// Create ChannelMojo. A bootstrap channel is created as well.
// |host| must not be null for server channels.
- static scoped_ptr<ChannelMojo> Create(Delegate* delegate,
- const ChannelHandle& channel_handle,
- Mode mode,
- Listener* listener);
+ static scoped_ptr<ChannelMojo> Create(
+ Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
+ const ChannelHandle& channel_handle,
+ Mode mode,
+ Listener* listener);
// Create a factory object for ChannelMojo.
// The factory is used to create Mojo-based ChannelProxy family.
// |host| must not be null.
static scoped_ptr<ChannelFactory> CreateServerFactory(
Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& channel_handle);
static scoped_ptr<ChannelFactory> CreateClientFactory(
Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& channel_handle);
~ChannelMojo() override;
@@ -115,6 +118,7 @@ class IPC_MOJO_EXPORT ChannelMojo
protected:
ChannelMojo(Delegate* delegate,
+ scoped_refptr<base::TaskRunner> io_runner,
const ChannelHandle& channel_handle,
Mode mode,
Listener* listener);
@@ -136,7 +140,7 @@ class IPC_MOJO_EXPORT ChannelMojo
// notifications invoked by them.
typedef internal::MessagePipeReader::DelayedDeleter ReaderDeleter;
- void InitDelegate(ChannelMojo::Delegate* delegate);
+ void InitOnIOThread(ChannelMojo::Delegate* delegate);
scoped_ptr<MojoBootstrap> bootstrap_;
base::WeakPtr<Delegate> delegate_;
diff --git a/ipc/mojo/ipc_channel_mojo_host.cc b/ipc/mojo/ipc_channel_mojo_host.cc
index 7e1bff9..28067cf 100644
--- a/ipc/mojo/ipc_channel_mojo_host.cc
+++ b/ipc/mojo/ipc_channel_mojo_host.cc
@@ -30,7 +30,6 @@ class ChannelMojoHost::ChannelDelegate
// ChannelMojo::Delegate
base::WeakPtr<Delegate> ToWeakPtr() override;
void OnChannelCreated(base::WeakPtr<ChannelMojo> channel) override;
- scoped_refptr<base::TaskRunner> GetIOTaskRunner() override;
// Returns an weak ptr of ChannelDelegate instead of Delegate
base::WeakPtr<ChannelDelegate> GetWeakPtr();
@@ -73,11 +72,6 @@ void ChannelMojoHost::ChannelDelegate::OnChannelCreated(
channel_ = channel;
}
-scoped_refptr<base::TaskRunner>
-ChannelMojoHost::ChannelDelegate::GetIOTaskRunner() {
- return io_task_runner_;
-}
-
void ChannelMojoHost::ChannelDelegate::OnClientLaunched(
base::ProcessHandle process) {
if (channel_)
diff --git a/ipc/mojo/ipc_channel_mojo_unittest.cc b/ipc/mojo/ipc_channel_mojo_unittest.cc
index 356d3b3..bf70b7f 100644
--- a/ipc/mojo/ipc_channel_mojo_unittest.cc
+++ b/ipc/mojo/ipc_channel_mojo_unittest.cc
@@ -66,12 +66,9 @@ class ListenerThatExpectsOK : public IPC::Listener {
class ChannelClient {
public:
explicit ChannelClient(IPC::Listener* listener, const char* name) {
- ipc_support_.reset(
- new IPC::ScopedIPCSupport(main_message_loop_.task_runner()));
- channel_ = IPC::ChannelMojo::Create(NULL,
+ channel_ = IPC::ChannelMojo::Create(NULL, main_message_loop_.task_runner(),
IPCTestBase::GetChannelName(name),
- IPC::Channel::MODE_CLIENT,
- listener);
+ IPC::Channel::MODE_CLIENT, listener);
}
void Connect() {
@@ -90,7 +87,6 @@ class ChannelClient {
private:
base::MessageLoopForIO main_message_loop_;
- scoped_ptr<IPC::ScopedIPCSupport> ipc_support_;
scoped_ptr<IPC::ChannelMojo> channel_;
};
@@ -98,22 +94,17 @@ class IPCChannelMojoTestBase : public IPCTestBase {
public:
void InitWithMojo(const std::string& test_client_name) {
Init(test_client_name);
- ipc_support_.reset(new IPC::ScopedIPCSupport(task_runner()));
}
void TearDown() override {
// Make sure Mojo IPC support is properly shutdown on the I/O loop before
// TearDown continues.
- ipc_support_.reset();
base::RunLoop run_loop;
task_runner()->PostTask(FROM_HERE, run_loop.QuitClosure());
run_loop.Run();
IPCTestBase::TearDown();
}
-
- private:
- scoped_ptr<IPC::ScopedIPCSupport> ipc_support_;
};
class IPCChannelMojoTest : public IPCChannelMojoTestBase {
@@ -123,7 +114,7 @@ class IPCChannelMojoTest : public IPCChannelMojoTestBase {
base::SequencedTaskRunner* runner) override {
host_.reset(new IPC::ChannelMojoHost(task_runner()));
return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(),
- handle);
+ task_runner(), handle);
}
bool DidStartClient() override {
@@ -231,7 +222,7 @@ class IPCChannelMojoErrorTest : public IPCChannelMojoTestBase {
base::SequencedTaskRunner* runner) override {
host_.reset(new IPC::ChannelMojoHost(task_runner()));
return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(),
- handle);
+ task_runner(), handle);
}
bool DidStartClient() override {
@@ -565,7 +556,7 @@ class IPCChannelMojoDeadHandleTest : public IPCChannelMojoTestBase {
base::SequencedTaskRunner* runner) override {
host_.reset(new IPC::ChannelMojoHost(task_runner()));
return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(),
- handle);
+ task_runner(), handle);
}
virtual bool DidStartClient() override {
diff --git a/ipc/mojo/ipc_mojo_perftest.cc b/ipc/mojo/ipc_mojo_perftest.cc
index 9f063ab..dc96dd1 100644
--- a/ipc/mojo/ipc_mojo_perftest.cc
+++ b/ipc/mojo/ipc_mojo_perftest.cc
@@ -31,17 +31,15 @@ public:
MojoChannelPerfTest();
void TearDown() override {
- ipc_support_.reset();
IPC::test::IPCChannelPerfTestBase::TearDown();
}
scoped_ptr<IPC::ChannelFactory> CreateChannelFactory(
const IPC::ChannelHandle& handle,
base::SequencedTaskRunner* runner) override {
- ipc_support_.reset(new IPC::ScopedIPCSupport(runner));
host_.reset(new IPC::ChannelMojoHost(runner));
return IPC::ChannelMojo::CreateServerFactory(host_->channel_delegate(),
- handle);
+ runner, handle);
}
bool DidStartClient() override {
@@ -52,7 +50,6 @@ public:
}
private:
- scoped_ptr<IPC::ScopedIPCSupport> ipc_support_;
scoped_ptr<IPC::ChannelMojoHost> host_;
};
@@ -82,9 +79,6 @@ class MojoTestClient : public IPC::test::PingPongTestClient {
MojoTestClient();
scoped_ptr<IPC::Channel> CreateChannel(IPC::Listener* listener) override;
-
- private:
- scoped_ptr<IPC::ScopedIPCSupport> ipc_support_;
};
MojoTestClient::MojoTestClient() {
@@ -93,12 +87,9 @@ MojoTestClient::MojoTestClient() {
scoped_ptr<IPC::Channel> MojoTestClient::CreateChannel(
IPC::Listener* listener) {
- ipc_support_.reset(new IPC::ScopedIPCSupport(task_runner()));
- return scoped_ptr<IPC::Channel>(
- IPC::ChannelMojo::Create(NULL,
- IPCTestBase::GetChannelName("PerformanceClient"),
- IPC::Channel::MODE_CLIENT,
- listener));
+ return scoped_ptr<IPC::Channel>(IPC::ChannelMojo::Create(
+ NULL, task_runner(), IPCTestBase::GetChannelName("PerformanceClient"),
+ IPC::Channel::MODE_CLIENT, listener));
}
MULTIPROCESS_IPC_TEST_CLIENT_MAIN(PerformanceClient) {