Refactor: Simplify WaitableEventWatcher.

base/synchronization/waitable_event_watcher.h

Index: base/synchronization/waitable_event_watcher.h
diff --git a/base/synchronization/waitable_event_watcher.h b/base/synchronization/waitable_event_watcher.h
index f5c1510b8f7ddc8ebcf2e7cbc33e7ca534552145..22016e3b787273129368c1613e3cbf98bb173427 100644
--- a/base/synchronization/waitable_event_watcher.h
+++ b/base/synchronization/waitable_event_watcher.h
@@ -32,16 +32,19 @@ class WaitableEvent;
 //
 // Typical usage:
 //
-//   class MyClass : public base::WaitableEventWatcher::Delegate {
+//   class MyClass {
 //    public:
 //     void DoStuffWhenSignaled(WaitableEvent *waitable_event) {
-//       watcher_.StartWatching(waitable_event, this);
+//       watcher_callback_ =
+//           base::Bind(&MyClass::OnWaitableEventSignaled, this);
+//       watcher_.StartWatching(waitable_event, &watcher_callback_);
 //     }
 //     virtual void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
 //       // OK, time to do stuff!
 //     }
 //    private:
 //     base::WaitableEventWatcher watcher_;
+//     base::Callback watcher_callback_;
 //   };
 //
 // In the above example, MyClass wants to "do stuff" when waitable_event
@@ -60,45 +63,30 @@ class WaitableEvent;
 // -----------------------------------------------------------------------------
 
 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:
 
   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.
+  // 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, Delegate* delegate);
+  bool StartWatching(WaitableEvent* event,
+                     Callback<void(WaitableEvent*)>* callback);

[dmichael (off chromium), 2013/01/25 21:28:11]

const Callback<...>&?

[tfarina, 2013/01/26 15:24:18]

a typedef would be helpful too. It would increase the readability.

[dmichael (off chromium), 2013/01/28 16:54:11]

I personally prefer not using a typedef when it's short like this, so you don't
have to go look up the typedef to figure out what it is. I would leave it alone
and maybe ask about it during OWNERS review.

[teravest, 2013/01/28 17:10:55]

We'll see what brett has to say. I changed this to use a typedef, but I'm happy
to change it back.

 
   // ---------------------------------------------------------------------------
   // 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();
@@ -110,36 +98,14 @@ class BASE_EXPORT WaitableEventWatcher
   WaitableEvent* GetWatchedEvent();
 
   // ---------------------------------------------------------------------------
-  // Return the delegate, or NULL if there is no delegate.
+  // Return the callback that will be invoked when the event is
+  // signaled.
   // ---------------------------------------------------------------------------
-  Delegate* delegate() {
-    return delegate_;
-  }
+  Callback<void(WaitableEvent*)>* callback() { return callback_; }

[dmichael (off chromium), 2013/01/25 21:28:11]

I would return by const&

[teravest, 2013/01/28 17:10:55]

Done.

 
  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_;
+  void OnObjectSignaled(HANDLE h);

[dmichael (off chromium), 2013/01/25 21:28:11]

mark virtual, and OVERRIDE

[teravest, 2013/01/28 17:10:55]

Done.

   win::ObjectWatcher watcher_;
 #else
   // ---------------------------------------------------------------------------
@@ -150,13 +116,12 @@ class BASE_EXPORT WaitableEventWatcher
   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_;
+  Callback<void(WaitableEvent*)>* callback_;

[dmichael (off chromium), 2013/01/25 21:28:11]

This should probably be by value.

[teravest, 2013/01/28 17:10:55]

Done.

 };
 
 }  // namespace base