summaryrefslogtreecommitdiffstats
path: root/ipc
diff options
context:
space:
mode:
authorkkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-11-16 01:06:46 +0000
committerkkania@chromium.org <kkania@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-11-16 01:06:46 +0000
commit952394afc0a0dc5e3e6a1f3c2e1c0fa99b7b681f (patch)
tree85d8246efdada076c5dda1b0a561d6c3322f37d1 /ipc
parent8bbba87862f0ce45234380cebf60e3ded5d88147 (diff)
downloadchromium_src-952394afc0a0dc5e3e6a1f3c2e1c0fa99b7b681f.zip
chromium_src-952394afc0a0dc5e3e6a1f3c2e1c0fa99b7b681f.tar.gz
chromium_src-952394afc0a0dc5e3e6a1f3c2e1c0fa99b7b681f.tar.bz2
Allow proxy channels to be created without initializing the underlying channel.
This fixes a bug where a client needed to guarantee a message filter was in place before any messages were received. It also follows the style of not having constructors that do complex initialization. BUG=102894 TEST=none Review URL: http://codereview.chromium.org/8417054 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@110229 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'ipc')
-rw-r--r--ipc/ipc_channel_proxy.cc20
-rw-r--r--ipc/ipc_channel_proxy.h28
-rw-r--r--ipc/ipc_sync_channel.cc33
-rw-r--r--ipc/ipc_sync_channel.h13
-rw-r--r--ipc/ipc_sync_channel_unittest.cc79
5 files changed, 139 insertions, 34 deletions
diff --git a/ipc/ipc_channel_proxy.cc b/ipc/ipc_channel_proxy.cc
index 26d285a..fb365d6 100644
--- a/ipc/ipc_channel_proxy.cc
+++ b/ipc/ipc_channel_proxy.cc
@@ -285,18 +285,15 @@ ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
Channel::Listener* listener,
base::MessageLoopProxy* ipc_thread)
: context_(new Context(listener, ipc_thread)),
- outgoing_message_filter_(NULL) {
- Init(channel_handle, mode, ipc_thread, true);
+ outgoing_message_filter_(NULL),
+ did_init_(false) {
+ Init(channel_handle, mode, true);
}
-ChannelProxy::ChannelProxy(const IPC::ChannelHandle& channel_handle,
- Channel::Mode mode,
- base::MessageLoopProxy* ipc_thread,
- Context* context,
- bool create_pipe_now)
+ChannelProxy::ChannelProxy(Context* context)
: context_(context),
- outgoing_message_filter_(NULL) {
- Init(channel_handle, mode, ipc_thread, create_pipe_now);
+ outgoing_message_filter_(NULL),
+ did_init_(false) {
}
ChannelProxy::~ChannelProxy() {
@@ -305,8 +302,8 @@ ChannelProxy::~ChannelProxy() {
void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
Channel::Mode mode,
- base::MessageLoopProxy* ipc_thread_loop,
bool create_pipe_now) {
+ DCHECK(!did_init_);
#if defined(OS_POSIX)
// When we are creating a server on POSIX, we need its file descriptor
// to be created immediately so that it can be accessed and passed
@@ -332,6 +329,8 @@ void ChannelProxy::Init(const IPC::ChannelHandle& channel_handle,
// complete initialization on the background thread
context_->ipc_message_loop()->PostTask(
FROM_HERE, base::Bind(&Context::OnChannelOpened, context_.get()));
+
+ did_init_ = true;
}
void ChannelProxy::Close() {
@@ -347,6 +346,7 @@ void ChannelProxy::Close() {
}
bool ChannelProxy::Send(Message* message) {
+ DCHECK(did_init_);
if (outgoing_message_filter())
message = outgoing_message_filter()->Rewrite(message);
diff --git a/ipc/ipc_channel_proxy.h b/ipc/ipc_channel_proxy.h
index 4e2ebbf..521389f 100644
--- a/ipc/ipc_channel_proxy.h
+++ b/ipc/ipc_channel_proxy.h
@@ -120,8 +120,21 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
Channel::Listener* listener,
base::MessageLoopProxy* ipc_thread_loop);
+ // Creates an uninitialized channel proxy. Init must be called to receive
+ // or send any messages. This two-step setup allows message filters to be
+ // added before any messages are sent or received.
+ ChannelProxy(Channel::Listener* listener,
+ base::MessageLoopProxy* ipc_thread_loop);
+
virtual ~ChannelProxy();
+ // Initializes the channel proxy. Only call this once to initialize a channel
+ // proxy that was not initialized in its constructor. If create_pipe_now is
+ // true, the pipe is created synchronously. Otherwise it's created on the IO
+ // thread.
+ void Init(const IPC::ChannelHandle& channel_handle, Channel::Mode mode,
+ bool create_pipe_now);
+
// Close the IPC::Channel. This operation completes asynchronously, once the
// background thread processes the command to close the channel. It is ok to
// call this method multiple times. Redundant calls are ignored.
@@ -165,13 +178,8 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
protected:
class Context;
// A subclass uses this constructor if it needs to add more information
- // to the internal state. If create_pipe_now is true, the pipe is created
- // immediately. Otherwise it's created on the IO thread.
- ChannelProxy(const IPC::ChannelHandle& channel_handle,
- Channel::Mode mode,
- base::MessageLoopProxy* ipc_thread_loop,
- Context* context,
- bool create_pipe_now);
+ // to the internal state.
+ ChannelProxy(Context* context);
// Used internally to hold state that is referenced on the IPC thread.
class Context : public base::RefCountedThreadSafe<Context>,
@@ -257,15 +265,15 @@ class IPC_EXPORT ChannelProxy : public Message::Sender {
private:
friend class SendTask;
- void Init(const IPC::ChannelHandle& channel_handle, Channel::Mode mode,
- base::MessageLoopProxy* ipc_thread_loop, bool create_pipe_now);
-
// By maintaining this indirection (ref-counted) to our internal state, we
// can safely be destroyed while the background thread continues to do stuff
// that involves this data.
scoped_refptr<Context> context_;
OutgoingMessageFilter* outgoing_message_filter_;
+
+ // Whether the channel has been initialized.
+ bool did_init_;
};
} // namespace IPC
diff --git a/ipc/ipc_sync_channel.cc b/ipc/ipc_sync_channel.cc
index 163c7ca..f3c5384 100644
--- a/ipc/ipc_sync_channel.cc
+++ b/ipc/ipc_sync_channel.cc
@@ -378,18 +378,19 @@ SyncChannel::SyncChannel(
base::MessageLoopProxy* ipc_message_loop,
bool create_pipe_now,
WaitableEvent* shutdown_event)
- : ChannelProxy(
- channel_handle, mode, ipc_message_loop,
- new SyncContext(listener, ipc_message_loop, shutdown_event),
- create_pipe_now),
+ : ChannelProxy(new SyncContext(listener, ipc_message_loop, shutdown_event)),
sync_messages_with_no_timeout_allowed_(true) {
- // Ideally we only want to watch this object when running a nested message
- // loop. However, we don't know when it exits if there's another nested
- // message loop running under it or not, so we wouldn't know whether to
- // stop or keep watching. So we always watch it, and create the event as
- // manual reset since the object watcher might otherwise reset the event
- // when we're doing a WaitMany.
- dispatch_watcher_.StartWatching(sync_context()->GetDispatchEvent(), this);
+ ChannelProxy::Init(channel_handle, mode, create_pipe_now);
+ StartWatching();
+}
+
+SyncChannel::SyncChannel(
+ Channel::Listener* listener,
+ base::MessageLoopProxy* ipc_message_loop,
+ WaitableEvent* shutdown_event)
+ : ChannelProxy(new SyncContext(listener, ipc_message_loop, shutdown_event)),
+ sync_messages_with_no_timeout_allowed_(true) {
+ StartWatching();
}
SyncChannel::~SyncChannel() {
@@ -514,4 +515,14 @@ void SyncChannel::OnWaitableEventSignaled(WaitableEvent* event) {
sync_context()->DispatchMessages();
}
+void SyncChannel::StartWatching() {
+ // Ideally we only want to watch this object when running a nested message
+ // loop. However, we don't know when it exits if there's another nested
+ // message loop running under it or not, so we wouldn't know whether to
+ // stop or keep watching. So we always watch it, and create the event as
+ // manual reset since the object watcher might otherwise reset the event
+ // when we're doing a WaitMany.
+ dispatch_watcher_.StartWatching(sync_context()->GetDispatchEvent(), this);
+}
+
} // namespace IPC
diff --git a/ipc/ipc_sync_channel.h b/ipc/ipc_sync_channel.h
index e43933e..ae9a765 100644
--- a/ipc/ipc_sync_channel.h
+++ b/ipc/ipc_sync_channel.h
@@ -62,12 +62,22 @@ class SyncMessage;
class IPC_EXPORT SyncChannel : public ChannelProxy,
public base::WaitableEventWatcher::Delegate {
public:
+ // Creates and initializes a sync channel. If create_pipe_now is specified,
+ // the channel will be initialized synchronously.
SyncChannel(const IPC::ChannelHandle& channel_handle,
Channel::Mode mode,
Channel::Listener* listener,
base::MessageLoopProxy* ipc_message_loop,
bool create_pipe_now,
base::WaitableEvent* shutdown_event);
+
+ // Creates an uninitialized sync channel. Call ChannelProxy::Init to
+ // initialize the channel. This two-step setup allows message filters to be
+ // added before any messages are sent or received.
+ SyncChannel(Channel::Listener* listener,
+ base::MessageLoopProxy* ipc_message_loop,
+ base::WaitableEvent* shutdown_event);
+
virtual ~SyncChannel();
virtual bool Send(Message* message);
@@ -186,6 +196,9 @@ class IPC_EXPORT SyncChannel : public ChannelProxy,
// shuts down.
static void WaitForReplyWithNestedMessageLoop(SyncContext* context);
+ // Starts the dispatch watcher.
+ void StartWatching();
+
bool sync_messages_with_no_timeout_allowed_;
// Used to signal events between the IPC and listener threads.
diff --git a/ipc/ipc_sync_channel_unittest.cc b/ipc/ipc_sync_channel_unittest.cc
index 029e49c..6982b28 100644
--- a/ipc/ipc_sync_channel_unittest.cc
+++ b/ipc/ipc_sync_channel_unittest.cc
@@ -120,6 +120,7 @@ class Worker : public Channel::Listener, public Message::Sender {
DCHECK_EQ(answer, (succeed ? 10 : 0));
return result;
}
+ const std::string& channel_name() { return channel_name_; }
Channel::Mode mode() { return mode_; }
WaitableEvent* done_event() { return done_.get(); }
WaitableEvent* shutdown_event() { return &shutdown_event_; }
@@ -156,6 +157,12 @@ class Worker : public Channel::Listener, public Message::Sender {
NOTREACHED();
}
+ virtual SyncChannel* CreateChannel() {
+ return new SyncChannel(
+ channel_name_, mode_, this, ipc_thread_.message_loop_proxy(), true,
+ &shutdown_event_);
+ }
+
base::Thread* ListenerThread() {
return overrided_thread_ ? overrided_thread_ : &listener_thread_;
}
@@ -167,9 +174,7 @@ class Worker : public Channel::Listener, public Message::Sender {
void OnStart() {
// Link ipc_thread_, listener_thread_ and channel_ altogether.
StartThread(&ipc_thread_, MessageLoop::TYPE_IO);
- channel_.reset(new SyncChannel(
- channel_name_, mode_, this, ipc_thread_.message_loop_proxy(), true,
- &shutdown_event_));
+ channel_.reset(CreateChannel());
channel_created_->Signal();
Run();
}
@@ -311,6 +316,74 @@ TEST_F(IPCSyncChannelTest, Simple) {
namespace {
+// Worker classes which override how the sync channel is created to use the
+// two-step initialization (calling the lightweight constructor and then
+// ChannelProxy::Init separately) process.
+class TwoStepServer : public Worker {
+ public:
+ explicit TwoStepServer(bool create_pipe_now)
+ : Worker(Channel::MODE_SERVER, "simpler_server"),
+ create_pipe_now_(create_pipe_now) { }
+
+ void Run() {
+ SendAnswerToLife(false, base::kNoTimeout, true);
+ Done();
+ }
+
+ virtual SyncChannel* CreateChannel() {
+ SyncChannel* channel = new SyncChannel(
+ this, ipc_thread().message_loop_proxy(), shutdown_event());
+ channel->Init(channel_name(), mode(), create_pipe_now_);
+ return channel;
+ }
+
+ bool create_pipe_now_;
+};
+
+class TwoStepClient : public Worker {
+ public:
+ TwoStepClient(bool create_pipe_now)
+ : Worker(Channel::MODE_CLIENT, "simple_client"),
+ create_pipe_now_(create_pipe_now) { }
+
+ void OnAnswer(int* answer) {
+ *answer = 42;
+ Done();
+ }
+
+ virtual SyncChannel* CreateChannel() {
+ SyncChannel* channel = new SyncChannel(
+ this, ipc_thread().message_loop_proxy(), shutdown_event());
+ channel->Init(channel_name(), mode(), create_pipe_now_);
+ return channel;
+ }
+
+ bool create_pipe_now_;
+};
+
+void TwoStep(bool create_server_pipe_now, bool create_client_pipe_now) {
+ std::vector<Worker*> workers;
+ workers.push_back(new TwoStepServer(create_server_pipe_now));
+ workers.push_back(new TwoStepClient(create_client_pipe_now));
+ RunTest(workers);
+}
+
+} // namespace
+
+// Tests basic two-step initialization, where you call the lightweight
+// constructor then Init.
+TEST_F(IPCSyncChannelTest, TwoStepInitialization) {
+ TwoStep(false, false);
+ TwoStep(false, true);
+ TwoStep(true, false);
+ TwoStep(true, true);
+}
+
+
+//-----------------------------------------------------------------------------
+
+namespace {
+
class DelayClient : public Worker {
public:
DelayClient() : Worker(Channel::MODE_CLIENT, "delay_client") { }