Use WaitableEvents to wake up sync IPC waiting

ipc/ipc_sync_message_filter.cc

Index: ipc/ipc_sync_message_filter.cc
diff --git a/ipc/ipc_sync_message_filter.cc b/ipc/ipc_sync_message_filter.cc
index ffccff3d50889271ac0b4c0926b10f62eb9e4ea8..b75876ac8afe826dc3d6bf6ed6e61c49468134d0 100644
--- a/ipc/ipc_sync_message_filter.cc
+++ b/ipc/ipc_sync_message_filter.cc
@@ -7,16 +7,12 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
-#include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
-#include "base/message_loop/message_loop.h"
-#include "base/single_thread_task_runner.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_sync_message.h"
-#include "ipc/mojo_event.h"
 #include "mojo/public/cpp/bindings/sync_handle_registry.h"
 
 namespace IPC {
@@ -24,77 +20,13 @@ namespace IPC {
 namespace {
 
 // A generic callback used when watching handles synchronously. Sets |*signal|
-// to true. Also sets |*error| to true in case of an error.
-void OnSyncHandleReady(bool* signal, bool* error, MojoResult result) {
+// to true.
+void OnEventReady(bool* signal) {
   *signal = true;
-  *error = result != MOJO_RESULT_OK;
 }
 
 }  // namespace
 
-// A helper class created by SyncMessageFilter to watch the lifetime of the IO
-// MessageLoop. This holds a weak ref to the SyncMessageFilter and notifies it
-// on its own thread if the SyncMessageFilter is still alive at the time of
-// IO MessageLoop destruction.
-class SyncMessageFilter::IOMessageLoopObserver
-    : public base::MessageLoop::DestructionObserver,
-      public base::RefCountedThreadSafe<IOMessageLoopObserver> {
- public:
-  IOMessageLoopObserver(
-      base::WeakPtr<SyncMessageFilter> weak_filter,
-      scoped_refptr<base::SingleThreadTaskRunner> filter_task_runner)
-      : weak_filter_(weak_filter), filter_task_runner_(filter_task_runner) {}
-
-  void StartOnIOThread() {
-    DCHECK(!watching_);
-    watching_ = true;
-    io_task_runner_ = base::ThreadTaskRunnerHandle::Get();
-    base::MessageLoop::current()->AddDestructionObserver(this);
-  }
-
-  void Stop() {
-    if (!io_task_runner_)
-      return;
-
-    if (io_task_runner_->BelongsToCurrentThread()) {
-      StopOnIOThread();
-    } else {
-      io_task_runner_->PostTask(
-          FROM_HERE, base::Bind(&IOMessageLoopObserver::StopOnIOThread, this));
-    }
-  }
-
- private:
-  void StopOnIOThread() {
-    DCHECK(io_task_runner_->BelongsToCurrentThread());
-    if (!watching_)
-      return;
-    watching_ = false;
-    base::MessageLoop::current()->RemoveDestructionObserver(this);
-  }
-
-  // base::MessageLoop::DestructionObserver:
-  void WillDestroyCurrentMessageLoop() override {
-    DCHECK(io_task_runner_ && io_task_runner_->BelongsToCurrentThread());
-    DCHECK(watching_);
-    StopOnIOThread();
-    filter_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&SyncMessageFilter::OnIOMessageLoopDestroyed, weak_filter_));
-  }
-
-  friend class base::RefCountedThreadSafe<IOMessageLoopObserver>;
-
-  ~IOMessageLoopObserver() override {}
-
-  bool watching_ = false;
-  base::WeakPtr<SyncMessageFilter> weak_filter_;
-  scoped_refptr<base::SingleThreadTaskRunner> filter_task_runner_;
-  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(IOMessageLoopObserver);
-};
-
 bool SyncMessageFilter::Send(Message* message) {
   if (!message->is_sync()) {
     {
@@ -110,7 +42,9 @@ bool SyncMessageFilter::Send(Message* message) {
     return true;
   }
 
-  MojoEvent done_event;
+  base::WaitableEvent done_event(
+      base::WaitableEvent::ResetPolicy::MANUAL,
+      base::WaitableEvent::InitialState::NOT_SIGNALED);
   PendingSyncMsg pending_message(
       SyncMessage::GetMessageId(*message),
       static_cast<SyncMessage*>(message)->GetReplyDeserializer(),
@@ -137,26 +71,21 @@ bool SyncMessageFilter::Send(Message* message) {
 
   bool done = false;
   bool shutdown = false;
-  bool error = false;
   scoped_refptr<mojo::SyncHandleRegistry> registry =
       mojo::SyncHandleRegistry::current();
-  registry->RegisterHandle(shutdown_mojo_event_.GetHandle(),
-                           MOJO_HANDLE_SIGNAL_READABLE,
-                           base::Bind(&OnSyncHandleReady, &shutdown, &error));
-  registry->RegisterHandle(done_event.GetHandle(),
-                           MOJO_HANDLE_SIGNAL_READABLE,
-                           base::Bind(&OnSyncHandleReady, &done, &error));
+  registry->RegisterEvent(shutdown_event_,
+                          base::Bind(&OnEventReady, &shutdown));
+  registry->RegisterEvent(&done_event, base::Bind(&OnEventReady, &done));
 
   const bool* stop_flags[] = { &done, &shutdown };
-  registry->WatchAllHandles(stop_flags, 2);
-  DCHECK(!error);
-
+  registry->Wait(stop_flags, 2);
   if (done) {
     TRACE_EVENT_FLOW_END0(TRACE_DISABLED_BY_DEFAULT("ipc.flow"),
                           "SyncMessageFilter::Send", &done_event);
   }
-  registry->UnregisterHandle(shutdown_mojo_event_.GetHandle());
-  registry->UnregisterHandle(done_event.GetHandle());
+
+  registry->UnregisterEvent(shutdown_event_);
+  registry->UnregisterEvent(&done_event);
 
   {
     base::AutoLock auto_lock(lock_);
@@ -174,10 +103,6 @@ void SyncMessageFilter::OnFilterAdded(Channel* channel) {
     channel_ = channel;
 
     io_task_runner_ = base::ThreadTaskRunnerHandle::Get();
-    shutdown_watcher_.StartWatching(
-        shutdown_event_,
-        base::Bind(&SyncMessageFilter::OnShutdownEventSignaled, this));
-    io_message_loop_observer_->StartOnIOThread();
     std::swap(pending_messages_, pending_messages);
   }
   for (auto& msg : pending_messages)
@@ -187,14 +112,12 @@ void SyncMessageFilter::OnFilterAdded(Channel* channel) {
 void SyncMessageFilter::OnChannelError() {
   base::AutoLock auto_lock(lock_);
   channel_ = nullptr;
-  shutdown_watcher_.StopWatching();
   SignalAllEvents();
 }
 
 void SyncMessageFilter::OnChannelClosing() {
   base::AutoLock auto_lock(lock_);
   channel_ = nullptr;
-  shutdown_watcher_.StopWatching();
   SignalAllEvents();
 }
 
@@ -221,15 +144,9 @@ bool SyncMessageFilter::OnMessageReceived(const Message& message) {
 SyncMessageFilter::SyncMessageFilter(base::WaitableEvent* shutdown_event)
     : channel_(nullptr),
       listener_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      shutdown_event_(shutdown_event),
-      weak_factory_(this) {
-  io_message_loop_observer_ = new IOMessageLoopObserver(
-      weak_factory_.GetWeakPtr(), listener_task_runner_);
-}
+      shutdown_event_(shutdown_event) {}
 
-SyncMessageFilter::~SyncMessageFilter() {
-  io_message_loop_observer_->Stop();
-}
+SyncMessageFilter::~SyncMessageFilter() {}
 
 void SyncMessageFilter::SendOnIOThread(Message* message) {
   if (channel_) {
@@ -258,19 +175,6 @@ void SyncMessageFilter::SignalAllEvents() {
   }
 }
 
-void SyncMessageFilter::OnShutdownEventSignaled(base::WaitableEvent* event) {
-  DCHECK_EQ(event, shutdown_event_);
-  shutdown_mojo_event_.Signal();
-}
-
-void SyncMessageFilter::OnIOMessageLoopDestroyed() {
-  // Since we use an async WaitableEventWatcher to watch the shutdown event
-  // from the IO thread, we can't forward the shutdown signal after the IO
-  // message loop is destroyed. Since that destruction indicates shutdown
-  // anyway, we manually signal the shutdown event in this case.
-  shutdown_mojo_event_.Signal();
-}
-
 void SyncMessageFilter::GetGenericRemoteAssociatedInterface(
     const std::string& interface_name,
     mojo::ScopedInterfaceEndpointHandle handle) {