Commit b97e3d3f authored by rockot's avatar rockot Committed by Commit bot

IPC: Delete thread-safe send support

We ended up not using it, and there are no plans to use it in
the future. It adds some unnecessary complexity to
SyncMessageFilter which I don't want to have to maintain as
I add some extra support for associated interfaces.

BUG=612500
R=jam@chromium.org

Review-Url: https://codereview.chromium.org/2343033002
Cr-Commit-Position: refs/heads/master@{#419205}
parent 69895c81
...@@ -54,8 +54,7 @@ class MockDispatcher : public IndexedDBDispatcher { ...@@ -54,8 +54,7 @@ class MockDispatcher : public IndexedDBDispatcher {
class MockSyncMessageFilter : public IPC::SyncMessageFilter { class MockSyncMessageFilter : public IPC::SyncMessageFilter {
public: public:
MockSyncMessageFilter() MockSyncMessageFilter() : SyncMessageFilter(nullptr) {}
: SyncMessageFilter(nullptr, false /* is_channel_send_thread_safe */) {}
private: private:
~MockSyncMessageFilter() override {} ~MockSyncMessageFilter() override {}
......
...@@ -124,8 +124,7 @@ class MockContinueCallbacks : public StrictMock<MockWebIDBCallbacks> { ...@@ -124,8 +124,7 @@ class MockContinueCallbacks : public StrictMock<MockWebIDBCallbacks> {
class MockSyncMessageFilter : public IPC::SyncMessageFilter { class MockSyncMessageFilter : public IPC::SyncMessageFilter {
public: public:
MockSyncMessageFilter() MockSyncMessageFilter() : SyncMessageFilter(nullptr) {}
: SyncMessageFilter(nullptr, false /* is_channel_send_thread_safe */) {}
private: private:
~MockSyncMessageFilter() override {} ~MockSyncMessageFilter() override {}
......
...@@ -35,7 +35,7 @@ class TestRenderWidget : public RenderWidget { ...@@ -35,7 +35,7 @@ class TestRenderWidget : public RenderWidget {
class TestSyncMessageFilter : public IPC::SyncMessageFilter { class TestSyncMessageFilter : public IPC::SyncMessageFilter {
public: public:
TestSyncMessageFilter() : IPC::SyncMessageFilter(NULL, false) {} TestSyncMessageFilter() : IPC::SyncMessageFilter(nullptr) {}
bool Send(IPC::Message* message) override { bool Send(IPC::Message* message) override {
messages_.push_back(base::WrapUnique(message)); messages_.push_back(base::WrapUnique(message));
......
...@@ -266,10 +266,6 @@ class IPC_EXPORT Channel : public Endpoint { ...@@ -266,10 +266,6 @@ class IPC_EXPORT Channel : public Endpoint {
// deleted once the contents of the Message have been sent. // deleted once the contents of the Message have been sent.
bool Send(Message* message) override = 0; bool Send(Message* message) override = 0;
// IsSendThreadSafe returns true iff it's safe to call |Send| from non-IO
// threads. This is constant for the lifetime of the |Channel|.
virtual bool IsSendThreadSafe() const;
// NaCl in Non-SFI mode runs on Linux directly, and the following functions // NaCl in Non-SFI mode runs on Linux directly, and the following functions
// compiled on Linux are also needed. Please see also comments in // compiled on Linux are also needed. Please see also comments in
// components/nacl_nonsfi.gyp for more details. // components/nacl_nonsfi.gyp for more details.
......
...@@ -86,10 +86,6 @@ void Channel::Unpause(bool flush) { NOTREACHED(); } ...@@ -86,10 +86,6 @@ void Channel::Unpause(bool flush) { NOTREACHED(); }
void Channel::Flush() { NOTREACHED(); } void Channel::Flush() { NOTREACHED(); }
bool Channel::IsSendThreadSafe() const {
return false;
}
void Channel::OnSetAttachmentBrokerEndpoint() { void Channel::OnSetAttachmentBrokerEndpoint() {
CHECK(!did_start_connect_); CHECK(!did_start_connect_);
} }
......
...@@ -370,10 +370,6 @@ bool ChannelMojo::Send(Message* message) { ...@@ -370,10 +370,6 @@ bool ChannelMojo::Send(Message* message) {
return message_reader_->Send(std::move(scoped_message)); return message_reader_->Send(std::move(scoped_message));
} }
bool ChannelMojo::IsSendThreadSafe() const {
return false;
}
base::ProcessId ChannelMojo::GetPeerPID() const { base::ProcessId ChannelMojo::GetPeerPID() const {
if (!message_reader_) if (!message_reader_)
return base::kNullProcessId; return base::kNullProcessId;
......
...@@ -73,7 +73,6 @@ class IPC_EXPORT ChannelMojo ...@@ -73,7 +73,6 @@ class IPC_EXPORT ChannelMojo
void Flush() override; void Flush() override;
void Close() override; void Close() override;
bool Send(Message* message) override; bool Send(Message* message) override;
bool IsSendThreadSafe() const override;
base::ProcessId GetPeerPID() const override; base::ProcessId GetPeerPID() const override;
base::ProcessId GetSelfPID() const override; base::ProcessId GetSelfPID() const override;
Channel::AssociatedInterfaceSupport* GetAssociatedInterfaceSupport() override; Channel::AssociatedInterfaceSupport* GetAssociatedInterfaceSupport() override;
......
...@@ -47,7 +47,6 @@ ChannelProxy::Context::Context( ...@@ -47,7 +47,6 @@ ChannelProxy::Context::Context(
listener_(listener), listener_(listener),
ipc_task_runner_(ipc_task_runner), ipc_task_runner_(ipc_task_runner),
channel_connected_called_(false), channel_connected_called_(false),
channel_send_thread_safe_(false),
message_filter_router_(new MessageFilterRouter()), message_filter_router_(new MessageFilterRouter()),
peer_pid_(base::kNullProcessId), peer_pid_(base::kNullProcessId),
attachment_broker_endpoint_(false) { attachment_broker_endpoint_(false) {
...@@ -77,7 +76,6 @@ void ChannelProxy::Context::CreateChannel( ...@@ -77,7 +76,6 @@ void ChannelProxy::Context::CreateChannel(
DCHECK_EQ(factory->GetIPCTaskRunner(), ipc_task_runner_); DCHECK_EQ(factory->GetIPCTaskRunner(), ipc_task_runner_);
channel_id_ = factory->GetName(); channel_id_ = factory->GetName();
channel_ = factory->BuildChannel(this); channel_ = factory->BuildChannel(this);
channel_send_thread_safe_ = channel_->IsSendThreadSafe();
channel_->SetAttachmentBrokerEndpoint(attachment_broker_endpoint_); channel_->SetAttachmentBrokerEndpoint(attachment_broker_endpoint_);
Channel::AssociatedInterfaceSupport* support = Channel::AssociatedInterfaceSupport* support =
...@@ -412,29 +410,12 @@ void ChannelProxy::Context::AddGenericAssociatedInterfaceForIOThread( ...@@ -412,29 +410,12 @@ void ChannelProxy::Context::AddGenericAssociatedInterfaceForIOThread(
support->AddGenericAssociatedInterface(name, factory); support->AddGenericAssociatedInterface(name, factory);
} }
void ChannelProxy::Context::SendFromThisThread(Message* message) {
base::AutoLock l(channel_lifetime_lock_);
if (!channel_)
return;
DCHECK(channel_->IsSendThreadSafe());
channel_->Send(message);
}
void ChannelProxy::Context::Send(Message* message) { void ChannelProxy::Context::Send(Message* message) {
if (channel_send_thread_safe_) {
SendFromThisThread(message);
return;
}
ipc_task_runner()->PostTask( ipc_task_runner()->PostTask(
FROM_HERE, base::Bind(&ChannelProxy::Context::OnSendMessage, this, FROM_HERE, base::Bind(&ChannelProxy::Context::OnSendMessage, this,
base::Passed(base::WrapUnique(message)))); base::Passed(base::WrapUnique(message))));
} }
bool ChannelProxy::Context::IsChannelSendThreadSafe() const {
return channel_send_thread_safe_;
}
// Called on the IPC::Channel thread // Called on the IPC::Channel thread
void ChannelProxy::Context::GetRemoteAssociatedInterface( void ChannelProxy::Context::GetRemoteAssociatedInterface(
const std::string& name, const std::string& name,
......
...@@ -269,9 +269,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe { ...@@ -269,9 +269,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe {
// Sends |message| from appropriate thread. // Sends |message| from appropriate thread.
void Send(Message* message); void Send(Message* message);
// Indicates if the underlying channel's Send is thread-safe.
bool IsChannelSendThreadSafe() const;
// Requests a remote associated interface on the IPC thread. // Requests a remote associated interface on the IPC thread.
void GetRemoteAssociatedInterface( void GetRemoteAssociatedInterface(
const std::string& name, const std::string& name,
...@@ -336,7 +333,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe { ...@@ -336,7 +333,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe {
const std::string& interface_name, const std::string& interface_name,
mojo::ScopedInterfaceEndpointHandle handle); mojo::ScopedInterfaceEndpointHandle handle);
void SendFromThisThread(Message* message);
void ClearChannel(); void ClearChannel();
mojo::AssociatedGroup* associated_group() { return &associated_group_; } mojo::AssociatedGroup* associated_group() { return &associated_group_; }
...@@ -366,9 +362,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe { ...@@ -366,9 +362,6 @@ class IPC_EXPORT ChannelProxy : public Endpoint, public base::NonThreadSafe {
// Lock for |channel_| value. This is only relevant in the context of // Lock for |channel_| value. This is only relevant in the context of
// thread-safe send. // thread-safe send.
base::Lock channel_lifetime_lock_; base::Lock channel_lifetime_lock_;
// Indicates the thread-safe send availability. This is constant once
// |channel_| is set.
bool channel_send_thread_safe_;
// Routes a given message to a proper subset of |filters_|, depending // Routes a given message to a proper subset of |filters_|, depending
// on which message classes a filter might support. // on which message classes a filter might support.
......
...@@ -542,8 +542,7 @@ void SyncChannel::SetRestrictDispatchChannelGroup(int group) { ...@@ -542,8 +542,7 @@ void SyncChannel::SetRestrictDispatchChannelGroup(int group) {
scoped_refptr<SyncMessageFilter> SyncChannel::CreateSyncMessageFilter() { scoped_refptr<SyncMessageFilter> SyncChannel::CreateSyncMessageFilter() {
scoped_refptr<SyncMessageFilter> filter = new SyncMessageFilter( scoped_refptr<SyncMessageFilter> filter = new SyncMessageFilter(
sync_context()->shutdown_event(), sync_context()->shutdown_event());
sync_context()->IsChannelSendThreadSafe());
AddFilter(filter.get()); AddFilter(filter.get());
if (!did_init()) if (!did_init())
pre_init_sync_message_filters_.push_back(filter); pre_init_sync_message_filters_.push_back(filter);
...@@ -699,10 +698,6 @@ void SyncChannel::StartWatching() { ...@@ -699,10 +698,6 @@ void SyncChannel::StartWatching() {
} }
void SyncChannel::OnChannelInit() { void SyncChannel::OnChannelInit() {
for (const auto& filter : pre_init_sync_message_filters_) {
filter->set_is_channel_send_thread_safe(
context()->IsChannelSendThreadSafe());
}
pre_init_sync_message_filters_.clear(); pre_init_sync_message_filters_.clear();
} }
......
...@@ -991,7 +991,7 @@ class TestSyncMessageFilter : public SyncMessageFilter { ...@@ -991,7 +991,7 @@ class TestSyncMessageFilter : public SyncMessageFilter {
base::WaitableEvent* shutdown_event, base::WaitableEvent* shutdown_event,
Worker* worker, Worker* worker,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) scoped_refptr<base::SingleThreadTaskRunner> task_runner)
: SyncMessageFilter(shutdown_event, false), : SyncMessageFilter(shutdown_event),
worker_(worker), worker_(worker),
task_runner_(task_runner) {} task_runner_(task_runner) {}
......
...@@ -99,10 +99,7 @@ bool SyncMessageFilter::Send(Message* message) { ...@@ -99,10 +99,7 @@ bool SyncMessageFilter::Send(Message* message) {
if (!message->is_sync()) { if (!message->is_sync()) {
{ {
base::AutoLock auto_lock(lock_); base::AutoLock auto_lock(lock_);
if (sender_ && is_channel_send_thread_safe_) { if (!io_task_runner_.get()) {
sender_->Send(message);
return true;
} else if (!io_task_runner_.get()) {
pending_messages_.emplace_back(base::WrapUnique(message)); pending_messages_.emplace_back(base::WrapUnique(message));
return true; return true;
} }
...@@ -220,10 +217,8 @@ bool SyncMessageFilter::OnMessageReceived(const Message& message) { ...@@ -220,10 +217,8 @@ bool SyncMessageFilter::OnMessageReceived(const Message& message) {
return false; return false;
} }
SyncMessageFilter::SyncMessageFilter(base::WaitableEvent* shutdown_event, SyncMessageFilter::SyncMessageFilter(base::WaitableEvent* shutdown_event)
bool is_channel_send_thread_safe)
: sender_(NULL), : sender_(NULL),
is_channel_send_thread_safe_(is_channel_send_thread_safe),
listener_task_runner_(base::ThreadTaskRunnerHandle::Get()), listener_task_runner_(base::ThreadTaskRunnerHandle::Get()),
shutdown_event_(shutdown_event), shutdown_event_(shutdown_event),
weak_factory_(this) { weak_factory_(this) {
......
...@@ -43,9 +43,7 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender { ...@@ -43,9 +43,7 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender {
bool OnMessageReceived(const Message& message) override; bool OnMessageReceived(const Message& message) override;
protected: protected:
SyncMessageFilter(base::WaitableEvent* shutdown_event, explicit SyncMessageFilter(base::WaitableEvent* shutdown_event);
bool is_channel_send_thread_safe);
~SyncMessageFilter() override; ~SyncMessageFilter() override;
private: private:
...@@ -54,10 +52,6 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender { ...@@ -54,10 +52,6 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender {
friend class SyncChannel; friend class SyncChannel;
friend class IOMessageLoopObserver; friend class IOMessageLoopObserver;
void set_is_channel_send_thread_safe(bool is_channel_send_thread_safe) {
is_channel_send_thread_safe_ = is_channel_send_thread_safe;
}
void SendOnIOThread(Message* message); void SendOnIOThread(Message* message);
// Signal all the pending sends as done, used in an error condition. // Signal all the pending sends as done, used in an error condition.
void SignalAllEvents(); void SignalAllEvents();
...@@ -68,9 +62,6 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender { ...@@ -68,9 +62,6 @@ class IPC_EXPORT SyncMessageFilter : public MessageFilter, public Sender {
// The channel to which this filter was added. // The channel to which this filter was added.
Sender* sender_; Sender* sender_;
// Indicates if |sender_|'s Send method is thread-safe.
bool is_channel_send_thread_safe_;
// The process's main thread. // The process's main thread.
scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner_; scoped_refptr<base::SingleThreadTaskRunner> listener_task_runner_;
......
...@@ -32,8 +32,7 @@ namespace ppapi { ...@@ -32,8 +32,7 @@ namespace ppapi {
class ManifestMessageFilter : public IPC::SyncMessageFilter { class ManifestMessageFilter : public IPC::SyncMessageFilter {
public: public:
ManifestMessageFilter(base::WaitableEvent* shutdown_event) ManifestMessageFilter(base::WaitableEvent* shutdown_event)
: SyncMessageFilter(shutdown_event, : SyncMessageFilter(shutdown_event),
false /* is_channel_send_thread_safe */),
connected_event_(base::WaitableEvent::ResetPolicy::MANUAL, connected_event_(base::WaitableEvent::ResetPolicy::MANUAL,
base::WaitableEvent::InitialState::NOT_SIGNALED) {} base::WaitableEvent::InitialState::NOT_SIGNALED) {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment