Refactor: Simplify WaitableEventWatcher.

This change uses a callback instead of a delegate for specifying what
should be called when a WaitableEvent occurs.

This simplifies the class and gets rid of a workaround internal to the
class to prevent name collision on "Delegate" inner classes.

Tested (linux and windows):
  out/Debug/base_unittests --gtest_filter=*WaitableEventWatcherTest*
  out/Release/ipc_tests with valgrind and leak-check=yes

Previously reverted at:
  https://codereview.chromium.org/12087120/


BUG=


Review URL: https://chromiumcodereview.appspot.com/12094106

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@180450 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 64 insertions, 127 deletions

diff --git a/base/synchronization/waitable_event_watcher.h b/base/synchronization/waitable_event_watcher.h
index f5c1510..f2f5f04 100644
--- a/base/synchronization/waitable_event_watcher.h
+++ b/base/synchronization/waitable_event_watcher.h
@@ -23,7 +23,6 @@ class AsyncWaiter;
 class AsyncCallbackTask;
 class WaitableEvent;
 
-// -----------------------------------------------------------------------------
 // This class provides a way to wait on a WaitableEvent asynchronously.
 //
 // Each instance of this object can be waiting on a single WaitableEvent. When
@@ -32,15 +31,16 @@ class WaitableEvent;
 //
 // Typical usage:
 //
-//   class MyClass : public base::WaitableEventWatcher::Delegate {
+//   class MyClass {
 //    public:
 //     void DoStuffWhenSignaled(WaitableEvent *waitable_event) {
-//       watcher_.StartWatching(waitable_event, this);
+//       watcher_.StartWatching(waitable_event,
+//           base::Bind(&MyClass::OnWaitableEventSignaled, this);
 //     }
-//     virtual void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
+//    private:
+//     void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
 //       // OK, time to do stuff!
 //     }
-//    private:
 //     base::WaitableEventWatcher watcher_;
 //   };
 //
@@ -57,106 +57,56 @@ class WaitableEvent;
 //
 // NOTE: you /are/ allowed to delete the WaitableEvent while still waiting on
 // it with a Watcher. It will act as if the event was never signaled.
-// -----------------------------------------------------------------------------
 
 class BASE_EXPORT WaitableEventWatcher
-#if !defined(OS_WIN)
-    : public MessageLoop::DestructionObserver
+#if defined(OS_WIN)
+    : public win::ObjectWatcher::Delegate {
+#else
+    : public MessageLoop::DestructionObserver {
 #endif
-{
  public:
-
+  typedef Callback<void(WaitableEvent*)> EventCallback;
   WaitableEventWatcher();
   virtual ~WaitableEventWatcher();
 
-  class BASE_EXPORT Delegate {
-   public:
-    // -------------------------------------------------------------------------
-    // This is called on the MessageLoop thread when WaitableEvent has been
-    // signaled.
-    //
-    // Note: the event may not be signaled by the time that this function is
-    // called. This indicates only that it has been signaled at some point in
-    // the past.
-    // -------------------------------------------------------------------------
-    virtual void OnWaitableEventSignaled(WaitableEvent* waitable_event) = 0;
-
-   protected:
-    virtual ~Delegate() { }
-  };
-
-  // ---------------------------------------------------------------------------
-  // When @event is signaled, the given delegate is called on the thread of the
-  // current message loop when StartWatching is called. The delegate is not
-  // deleted.
-  // ---------------------------------------------------------------------------
-  bool StartWatching(WaitableEvent* event, Delegate* delegate);
-
-  // ---------------------------------------------------------------------------
+  // When @event is signaled, the given callback is called on the thread of the
+  // current message loop when StartWatching is called.
+  bool StartWatching(WaitableEvent* event, const EventCallback& callback);
+
   // Cancel the current watch. Must be called from the same thread which
   // started the watch.
   //
   // Does nothing if no event is being watched, nor if the watch has completed.
-  // The delegate will *not* be called for the current watch after this
-  // function returns. Since the delegate runs on the same thread as this
+  // The callback will *not* be called for the current watch after this
+  // function returns. Since the callback runs on the same thread as this
   // function, it cannot be called during this function either.
-  // ---------------------------------------------------------------------------
   void StopWatching();
 
-  // ---------------------------------------------------------------------------
   // Return the currently watched event, or NULL if no object is currently being
   // watched.
-  // ---------------------------------------------------------------------------
   WaitableEvent* GetWatchedEvent();
 
-  // ---------------------------------------------------------------------------
-  // Return the delegate, or NULL if there is no delegate.
-  // ---------------------------------------------------------------------------
-  Delegate* delegate() {
-    return delegate_;
-  }
+  // Return the callback that will be invoked when the event is
+  // signaled.
+  const EventCallback& callback() const { return callback_; }
 
  private:
 #if defined(OS_WIN)
-  // ---------------------------------------------------------------------------
-  // The helper class exists because, if WaitableEventWatcher were to inherit
-  // from ObjectWatcher::Delegate, then it couldn't also have an inner class
-  // called Delegate (at least on Windows). Thus this object exists to proxy
-  // the callback function
-  // ---------------------------------------------------------------------------
-  class ObjectWatcherHelper : public win::ObjectWatcher::Delegate {
-   public:
-    ObjectWatcherHelper(WaitableEventWatcher* watcher);
-
-    // -------------------------------------------------------------------------
-    // Implementation of ObjectWatcher::Delegate
-    // -------------------------------------------------------------------------
-    void OnObjectSignaled(HANDLE h);
-
-   private:
-    WaitableEventWatcher *const watcher_;
-  };
-
-  void OnObjectSignaled();
-
-  ObjectWatcherHelper helper_;
+  virtual void OnObjectSignaled(HANDLE h) OVERRIDE;
   win::ObjectWatcher watcher_;
 #else
-  // ---------------------------------------------------------------------------
   // Implementation of MessageLoop::DestructionObserver
-  // ---------------------------------------------------------------------------
   virtual void WillDestroyCurrentMessageLoop() OVERRIDE;
 
   MessageLoop* message_loop_;
   scoped_refptr<Flag> cancel_flag_;
   AsyncWaiter* waiter_;
-  base::Closure callback_;
+  base::Closure internal_callback_;
   scoped_refptr<WaitableEvent::WaitableEventKernel> kernel_;
 #endif
 
   WaitableEvent* event_;
-
-  Delegate* delegate_;
+  EventCallback callback_;
 };
 
 }  // namespace base
diff --git a/base/synchronization/waitable_event_watcher_posix.cc b/base/synchronization/waitable_event_watcher_posix.cc
index 3180b4b..cc2fbc3 100644
--- a/base/synchronization/waitable_event_watcher_posix.cc
+++ b/base/synchronization/waitable_event_watcher_posix.cc
@@ -97,14 +97,14 @@ class AsyncWaiter : public WaitableEvent::Waiter {
 // the event is canceled.
 // -----------------------------------------------------------------------------
 void AsyncCallbackHelper(Flag* flag,
-                         WaitableEventWatcher::Delegate* delegate,
+                         const WaitableEventWatcher::EventCallback& callback,
                          WaitableEvent* event) {
   // Runs in MessageLoop thread.
   if (!flag->value()) {
     // This is to let the WaitableEventWatcher know that the event has occured
     // because it needs to be able to return NULL from GetWatchedObject
     flag->Set();
-    delegate->OnWaitableEventSignaled(event);
+    callback.Run(event);
   }
 }
 
@@ -112,8 +112,7 @@ WaitableEventWatcher::WaitableEventWatcher()
     : message_loop_(NULL),
       cancel_flag_(NULL),
       waiter_(NULL),
-      event_(NULL),
-      delegate_(NULL) {
+      event_(NULL) {
 }
 
 WaitableEventWatcher::~WaitableEventWatcher() {
@@ -124,8 +123,9 @@ WaitableEventWatcher::~WaitableEventWatcher() {
 // The Handle is how the user cancels a wait. After deleting the Handle we
 // insure that the delegate cannot be called.
 // -----------------------------------------------------------------------------
-bool WaitableEventWatcher::StartWatching
-    (WaitableEvent* event, WaitableEventWatcher::Delegate* delegate) {
+bool WaitableEventWatcher::StartWatching(
+    WaitableEvent* event,
+    const EventCallback& callback) {
   MessageLoop *const current_ml = MessageLoop::current();
   DCHECK(current_ml) << "Cannot create WaitableEventWatcher without a "
                         "current MessageLoop";
@@ -145,12 +145,13 @@ bool WaitableEventWatcher::StartWatching
   DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching";
 
   cancel_flag_ = new Flag;
-  callback_ = base::Bind(&AsyncCallbackHelper, cancel_flag_, delegate, event);
+  callback_ = callback;
+  internal_callback_ =
+      base::Bind(&AsyncCallbackHelper, cancel_flag_, callback_, event);
   WaitableEvent::WaitableEventKernel* kernel = event->kernel_.get();
 
   AutoLock locked(kernel->lock_);
 
-  delegate_ = delegate;
   event_ = event;
 
   if (kernel->signaled_) {
@@ -159,7 +160,7 @@ bool WaitableEventWatcher::StartWatching
 
     // No hairpinning - we can't call the delegate directly here. We have to
     // enqueue a task on the MessageLoop as normal.
-    current_ml->PostTask(FROM_HERE, callback_);
+    current_ml->PostTask(FROM_HERE, internal_callback_);
     return true;
   }
 
@@ -167,14 +168,14 @@ bool WaitableEventWatcher::StartWatching
   current_ml->AddDestructionObserver(this);
 
   kernel_ = kernel;
-  waiter_ = new AsyncWaiter(current_ml, callback_, cancel_flag_);
+  waiter_ = new AsyncWaiter(current_ml, internal_callback_, cancel_flag_);
   event->Enqueue(waiter_);
 
   return true;
 }
 
 void WaitableEventWatcher::StopWatching() {
-  delegate_ = NULL;
+  callback_.Reset();
 
   if (message_loop_) {
     message_loop_->RemoveDestructionObserver(this);
@@ -227,7 +228,7 @@ void WaitableEventWatcher::StopWatching() {
     // have been enqueued with the MessageLoop because the waiter was never
     // signaled)
     delete waiter_;
-    callback_.Reset();
+    internal_callback_.Reset();
     cancel_flag_ = NULL;
     return;
   }
diff --git a/base/synchronization/waitable_event_watcher_unittest.cc b/base/synchronization/waitable_event_watcher_unittest.cc
index 03ab800..a35c7c9 100644
--- a/base/synchronization/waitable_event_watcher_unittest.cc
+++ b/base/synchronization/waitable_event_watcher_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/message_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/synchronization/waitable_event_watcher.h"
@@ -23,18 +25,15 @@ const MessageLoop::Type testing_message_loops[] = {
 
 const int kNumTestingMessageLoops = arraysize(testing_message_loops);
 
-class QuitDelegate : public WaitableEventWatcher::Delegate {
- public:
-  virtual void OnWaitableEventSignaled(WaitableEvent* event) OVERRIDE {
-    MessageLoop::current()->QuitWhenIdle();
-  }
-};
+void QuitWhenSignaled(WaitableEvent* event) {
+  MessageLoop::current()->QuitWhenIdle();
+}
 
-class DecrementCountDelegate : public WaitableEventWatcher::Delegate {
+class DecrementCountContainer {
  public:
-  explicit DecrementCountDelegate(int* counter) : counter_(counter) {
+  explicit DecrementCountContainer(int* counter) : counter_(counter) {
   }
-  virtual void OnWaitableEventSignaled(WaitableEvent* object) OVERRIDE {
+  void OnWaitableEventSignaled(WaitableEvent* object) {
     --(*counter_);
   }
  private:
@@ -50,8 +49,7 @@ void RunTest_BasicSignal(MessageLoop::Type message_loop_type) {
   WaitableEventWatcher watcher;
   EXPECT_TRUE(watcher.GetWatchedEvent() == NULL);
 
-  QuitDelegate delegate;
-  watcher.StartWatching(&event, &delegate);
+  watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
   EXPECT_EQ(&event, watcher.GetWatchedEvent());
 
   event.Signal();
@@ -69,8 +67,7 @@ void RunTest_BasicCancel(MessageLoop::Type message_loop_type) {
 
   WaitableEventWatcher watcher;
 
-  QuitDelegate delegate;
-  watcher.StartWatching(&event, &delegate);
+  watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
 
   watcher.StopWatching();
 }
@@ -84,9 +81,11 @@ void RunTest_CancelAfterSet(MessageLoop::Type message_loop_type) {
   WaitableEventWatcher watcher;
 
   int counter = 1;
-  DecrementCountDelegate delegate(&counter);
-
-  watcher.StartWatching(&event, &delegate);
+  DecrementCountContainer delegate(&counter);
+  WaitableEventWatcher::EventCallback callback =
+      Bind(&DecrementCountContainer::OnWaitableEventSignaled,
+           Unretained(&delegate));
+  watcher.StartWatching(&event, callback);
 
   event.Signal();
 
@@ -111,8 +110,7 @@ void RunTest_OutlivesMessageLoop(MessageLoop::Type message_loop_type) {
     {
       MessageLoop message_loop(message_loop_type);
 
-      QuitDelegate delegate;
-      watcher.StartWatching(&event, &delegate);
+      watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
     }
   }
 }
@@ -127,8 +125,8 @@ void RunTest_DeleteUnder(MessageLoop::Type message_loop_type) {
     WaitableEventWatcher watcher;
 
     WaitableEvent* event = new WaitableEvent(false, false);
-    QuitDelegate delegate;
-    watcher.StartWatching(event, &delegate);
+
+    watcher.StartWatching(event, Bind(&QuitWhenSignaled));
     delete event;
   }
 }
diff --git a/base/synchronization/waitable_event_watcher_win.cc b/base/synchronization/waitable_event_watcher_win.cc
index 43e3c47..46d47ac 100644
--- a/base/synchronization/waitable_event_watcher_win.cc
+++ b/base/synchronization/waitable_event_watcher_win.cc
@@ -10,35 +10,23 @@
 
 namespace base {
 
-WaitableEventWatcher::ObjectWatcherHelper::ObjectWatcherHelper(
-    WaitableEventWatcher* watcher)
-    : watcher_(watcher) {
-};
-
-void WaitableEventWatcher::ObjectWatcherHelper::OnObjectSignaled(HANDLE h) {
-  watcher_->OnObjectSignaled();
-}
-
-
 WaitableEventWatcher::WaitableEventWatcher()
-    : ALLOW_THIS_IN_INITIALIZER_LIST(helper_(this)),
-      event_(NULL),
-      delegate_(NULL) {
+    : event_(NULL) {
 }
 
 WaitableEventWatcher::~WaitableEventWatcher() {
 }
 
-bool WaitableEventWatcher::StartWatching(WaitableEvent* event,
-                                         Delegate* delegate) {
-  delegate_ = delegate;
+bool WaitableEventWatcher::StartWatching(
+    WaitableEvent* event,
+    const EventCallback& callback) {
+  callback_ = callback;
   event_ = event;
-
-  return watcher_.StartWatching(event->handle(), &helper_);
+  return watcher_.StartWatching(event->handle(), this);
 }
 
 void WaitableEventWatcher::StopWatching() {
-  delegate_ = NULL;
+  callback_.Reset();
   event_ = NULL;
   watcher_.StopWatching();
 }
@@ -47,14 +35,14 @@ WaitableEvent* WaitableEventWatcher::GetWatchedEvent() {
   return event_;
 }
 
-void WaitableEventWatcher::OnObjectSignaled() {
+void WaitableEventWatcher::OnObjectSignaled(HANDLE h) {
   WaitableEvent* event = event_;
-  Delegate* delegate = delegate_;
+  EventCallback callback = callback_;
   event_ = NULL;
-  delegate_ = NULL;
+  callback_.Reset();
   DCHECK(event);
 
-  delegate->OnWaitableEventSignaled(event);
+  callback.Run(event);
 }
 
 }  // namespace base