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..35d56ca00461d52775c4cfe1332c7ca0d8cf3b1f 100644
--- a/base/synchronization/waitable_event_watcher.h
+++ b/base/synchronization/waitable_event_watcher.h
@@ -23,6 +23,8 @@ class AsyncWaiter;
 class AsyncCallbackTask;
 class WaitableEvent;
 
+typedef Callback<void(WaitableEvent*)> WaitableEventCallback;

[brettw, 2013/01/28 20:56:59]

I think the typedef is fine. I usually try to keep stuff out of the global (or
base in this case) namespace when possible, so in this case I'd probably do it
inside the class. Maybe we can come up with a shorter name ("EventCallback"?).

[teravest, 2013/01/28 21:25:09]

Done.

+
 // -----------------------------------------------------------------------------
 // This class provides a way to wait on a WaitableEvent asynchronously.
 //
@@ -32,12 +34,14 @@ class WaitableEvent;
 //
 // Typical usage:
 //
-//   class MyClass : public base::WaitableEventWatcher::Delegate {
+//   class MyClass {
 //    public:
 //     void DoStuffWhenSignaled(WaitableEvent *waitable_event) {
-//       watcher_.StartWatching(waitable_event, this);
+//       base::WaitableEventCallback watcher_callback =
+//           base::Bind(&MyClass::OnWaitableEventSignaled, this);
+//       watcher_.StartWatching(waitable_event, watcher_callback);
 //     }
-//     virtual void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
+//     void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
 //       // OK, time to do stuff!
 //     }
 //    private:
@@ -60,45 +64,30 @@ class WaitableEvent;
 // -----------------------------------------------------------------------------
 
 class BASE_EXPORT WaitableEventWatcher
-#if !defined(OS_WIN)
-    : public MessageLoop::DestructionObserver
+#if defined(OS_WIN)

[tfarina, 2013/01/28 21:06:10]

for a possible future CL. Just an idea. We should try hard to avoid ifdefs. So
instead of ifdefing here, we could have a WaitableEventWatcherWin and
WaitableEventWatcherLinux and with gyp compile only the class necessary for each
platform.

[dmichael (off chromium), 2013/01/28 21:47:27]

That's what we have, but the header is shared. That seems to be the most common
way to do platform-specific classes, AFAICT. IPC::Channel gets around it by
using a pImpl class that's platform specific, so I could see doing it that way
in a future CL if it's important. But IMHO it's not worth the trouble here.

+    : 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() { }
-  };
-
   // ---------------------------------------------------------------------------

[brettw, 2013/01/28 20:56:59]

Whoever wrote this kind of went crazy with the horizontal lines. Would you mind
removing them while you're here? It doesn't really match our style.

[teravest, 2013/01/28 21:25:09]

Done.

-  // 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,
+                     const WaitableEventCallback& 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();
@@ -110,36 +99,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_;
-  }
+  const WaitableEventCallback& 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
   // ---------------------------------------------------------------------------
@@ -150,13 +117,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_;
+  WaitableEventCallback callback_;
 };
 
 }  // namespace base