Revert 179987

Caused memory leak:
Leak_DefinitelyLost
4,196 (1,600 direct, 2,596 indirect) bytes in 4 blocks are definitely lost in loss record 608 of 639
  operator new(unsigned long) (m_replacemalloc/vg_replace_malloc.c:1140)
  IPC::SyncChannel::SyncChannel(IPC::ChannelHandle const&, IPC::Channel::Mode, IPC::Listener*, base::SingleThreadTaskRunner*, bool, base::WaitableEvent*) (ipc/ipc_sync_channel.cc:410)
  (anonymous namespace)::RestrictedDispatchPipeWorker::Run() (ipc/ipc_sync_channel_unittest.cc:1636)
  (anonymous namespace)::Worker::OnStart() (ipc/ipc_sync_channel_unittest.cc:176)
Suppression (error hash=#2A77226DFEFF6041#):
  For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports
{
   <insert_a_suppression_name_here>
   Memcheck:Leak
   fun:_Znw*
   fun:_ZN3IPC11SyncChannelC1ERKNS_13ChannelHandleENS_7Channel4ModeEPNS_8ListenerEPN4base22SingleThreadTaskRunnerEbPNS8_13WaitableEventE
   fun:_ZN12_GLOBAL__N_128RestrictedDispatchPipeWorker3RunEv
   fun:_ZN12_GLOBAL__N_16Worker7OnStartEv
}

15:02:51 memcheck_a
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28valgrind%29%282%29/builds/22101

> 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 (on linux and windows):
>   ninja -C out/Debug chrome
>   out/Debug/base_unittests --gtest_filter=*WaitableEventWatcherTest*
> 
> BUG=
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11953112

TBR=teravest@chromium.org
Review URL: https://codereview.chromium.org/12087120

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

4 files changed, 126 insertions, 63 deletions

diff --git a/base/synchronization/waitable_event_watcher.h b/base/synchronization/waitable_event_watcher.h
index fe61f96..f5c1510 100644
--- a/base/synchronization/waitable_event_watcher.h
+++ b/base/synchronization/waitable_event_watcher.h
@@ -23,6 +23,7 @@ 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
@@ -31,13 +32,12 @@ class WaitableEvent;
 //
 // Typical usage:
 //
-//   class MyClass {
+//   class MyClass : public base::WaitableEventWatcher::Delegate {
 //    public:
 //     void DoStuffWhenSignaled(WaitableEvent *waitable_event) {
-//       watcher_.StartWatching(waitable_event,
-//           base::Bind(&MyClass::OnWaitableEventSignaled, this);
+//       watcher_.StartWatching(waitable_event, this);
 //     }
-//     void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
+//     virtual void OnWaitableEventSignaled(WaitableEvent* waitable_event) {
 //       // OK, time to do stuff!
 //     }
 //    private:
@@ -57,56 +57,106 @@ 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 win::ObjectWatcher::Delegate {
-#else
-    : public MessageLoop::DestructionObserver {
+#if !defined(OS_WIN)
+    : public MessageLoop::DestructionObserver
 #endif
+{
  public:
-  typedef Callback<void(WaitableEvent*)> EventCallback;
+
   WaitableEventWatcher();
   virtual ~WaitableEventWatcher();
 
-  // 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);
-
+  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);
+
+  // ---------------------------------------------------------------------------
   // 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 callback will *not* be called for the current watch after this
-  // function returns. Since the callback runs on the same thread as this
+  // The delegate will *not* be called for the current watch after this
+  // function returns. Since the delegate 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 callback that will be invoked when the event is
-  // signaled.
-  const EventCallback& callback() const { return callback_; }
+  // ---------------------------------------------------------------------------
+  // Return the delegate, or NULL if there is no delegate.
+  // ---------------------------------------------------------------------------
+  Delegate* delegate() {
+    return delegate_;
+  }
 
  private:
 #if defined(OS_WIN)
-  virtual void OnObjectSignaled(HANDLE h) OVERRIDE;
+  // ---------------------------------------------------------------------------
+  // 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_;
   win::ObjectWatcher watcher_;
 #else
+  // ---------------------------------------------------------------------------
   // Implementation of MessageLoop::DestructionObserver
+  // ---------------------------------------------------------------------------
   virtual void WillDestroyCurrentMessageLoop() OVERRIDE;
 
   MessageLoop* message_loop_;
   scoped_refptr<Flag> cancel_flag_;
   AsyncWaiter* waiter_;
-  base::Closure internal_callback_;
+  base::Closure callback_;
   scoped_refptr<WaitableEvent::WaitableEventKernel> kernel_;
 #endif
 
   WaitableEvent* event_;
-  EventCallback callback_;
+
+  Delegate* delegate_;
 };
 
 }  // namespace base
diff --git a/base/synchronization/waitable_event_watcher_posix.cc b/base/synchronization/waitable_event_watcher_posix.cc
index cc2fbc3..3180b4b 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,
-                         const WaitableEventWatcher::EventCallback& callback,
+                         WaitableEventWatcher::Delegate* delegate,
                          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();
-    callback.Run(event);
+    delegate->OnWaitableEventSignaled(event);
   }
 }
 
@@ -112,7 +112,8 @@ WaitableEventWatcher::WaitableEventWatcher()
     : message_loop_(NULL),
       cancel_flag_(NULL),
       waiter_(NULL),
-      event_(NULL) {
+      event_(NULL),
+      delegate_(NULL) {
 }
 
 WaitableEventWatcher::~WaitableEventWatcher() {
@@ -123,9 +124,8 @@ 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,
-    const EventCallback& callback) {
+bool WaitableEventWatcher::StartWatching
+    (WaitableEvent* event, WaitableEventWatcher::Delegate* delegate) {
   MessageLoop *const current_ml = MessageLoop::current();
   DCHECK(current_ml) << "Cannot create WaitableEventWatcher without a "
                         "current MessageLoop";
@@ -145,13 +145,12 @@ bool WaitableEventWatcher::StartWatching(
   DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching";
 
   cancel_flag_ = new Flag;
-  callback_ = callback;
-  internal_callback_ =
-      base::Bind(&AsyncCallbackHelper, cancel_flag_, callback_, event);
+  callback_ = base::Bind(&AsyncCallbackHelper, cancel_flag_, delegate, event);
   WaitableEvent::WaitableEventKernel* kernel = event->kernel_.get();
 
   AutoLock locked(kernel->lock_);
 
+  delegate_ = delegate;
   event_ = event;
 
   if (kernel->signaled_) {
@@ -160,7 +159,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, internal_callback_);
+    current_ml->PostTask(FROM_HERE, callback_);
     return true;
   }
 
@@ -168,14 +167,14 @@ bool WaitableEventWatcher::StartWatching(
   current_ml->AddDestructionObserver(this);
 
   kernel_ = kernel;
-  waiter_ = new AsyncWaiter(current_ml, internal_callback_, cancel_flag_);
+  waiter_ = new AsyncWaiter(current_ml, callback_, cancel_flag_);
   event->Enqueue(waiter_);
 
   return true;
 }
 
 void WaitableEventWatcher::StopWatching() {
-  callback_.Reset();
+  delegate_ = NULL;
 
   if (message_loop_) {
     message_loop_->RemoveDestructionObserver(this);
@@ -228,7 +227,7 @@ void WaitableEventWatcher::StopWatching() {
     // have been enqueued with the MessageLoop because the waiter was never
     // signaled)
     delete waiter_;
-    internal_callback_.Reset();
+    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 a35c7c9..03ab800 100644
--- a/base/synchronization/waitable_event_watcher_unittest.cc
+++ b/base/synchronization/waitable_event_watcher_unittest.cc
@@ -2,8 +2,6 @@
 // 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"
@@ -25,15 +23,18 @@ const MessageLoop::Type testing_message_loops[] = {
 
 const int kNumTestingMessageLoops = arraysize(testing_message_loops);
 
-void QuitWhenSignaled(WaitableEvent* event) {
-  MessageLoop::current()->QuitWhenIdle();
-}
+class QuitDelegate : public WaitableEventWatcher::Delegate {
+ public:
+  virtual void OnWaitableEventSignaled(WaitableEvent* event) OVERRIDE {
+    MessageLoop::current()->QuitWhenIdle();
+  }
+};
 
-class DecrementCountContainer {
+class DecrementCountDelegate : public WaitableEventWatcher::Delegate {
  public:
-  explicit DecrementCountContainer(int* counter) : counter_(counter) {
+  explicit DecrementCountDelegate(int* counter) : counter_(counter) {
   }
-  void OnWaitableEventSignaled(WaitableEvent* object) {
+  virtual void OnWaitableEventSignaled(WaitableEvent* object) OVERRIDE {
     --(*counter_);
   }
  private:
@@ -49,7 +50,8 @@ void RunTest_BasicSignal(MessageLoop::Type message_loop_type) {
   WaitableEventWatcher watcher;
   EXPECT_TRUE(watcher.GetWatchedEvent() == NULL);
 
-  watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
+  QuitDelegate delegate;
+  watcher.StartWatching(&event, &delegate);
   EXPECT_EQ(&event, watcher.GetWatchedEvent());
 
   event.Signal();
@@ -67,7 +69,8 @@ void RunTest_BasicCancel(MessageLoop::Type message_loop_type) {
 
   WaitableEventWatcher watcher;
 
-  watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
+  QuitDelegate delegate;
+  watcher.StartWatching(&event, &delegate);
 
   watcher.StopWatching();
 }
@@ -81,11 +84,9 @@ void RunTest_CancelAfterSet(MessageLoop::Type message_loop_type) {
   WaitableEventWatcher watcher;
 
   int counter = 1;
-  DecrementCountContainer delegate(&counter);
-  WaitableEventWatcher::EventCallback callback =
-      Bind(&DecrementCountContainer::OnWaitableEventSignaled,
-           Unretained(&delegate));
-  watcher.StartWatching(&event, callback);
+  DecrementCountDelegate delegate(&counter);
+
+  watcher.StartWatching(&event, &delegate);
 
   event.Signal();
 
@@ -110,7 +111,8 @@ void RunTest_OutlivesMessageLoop(MessageLoop::Type message_loop_type) {
     {
       MessageLoop message_loop(message_loop_type);
 
-      watcher.StartWatching(&event, Bind(&QuitWhenSignaled));
+      QuitDelegate delegate;
+      watcher.StartWatching(&event, &delegate);
     }
   }
 }
@@ -125,8 +127,8 @@ void RunTest_DeleteUnder(MessageLoop::Type message_loop_type) {
     WaitableEventWatcher watcher;
 
     WaitableEvent* event = new WaitableEvent(false, false);
-
-    watcher.StartWatching(event, Bind(&QuitWhenSignaled));
+    QuitDelegate delegate;
+    watcher.StartWatching(event, &delegate);
     delete event;
   }
 }
diff --git a/base/synchronization/waitable_event_watcher_win.cc b/base/synchronization/waitable_event_watcher_win.cc
index 46d47ac..43e3c47 100644
--- a/base/synchronization/waitable_event_watcher_win.cc
+++ b/base/synchronization/waitable_event_watcher_win.cc
@@ -10,23 +10,35 @@
 
 namespace base {
 
+WaitableEventWatcher::ObjectWatcherHelper::ObjectWatcherHelper(
+    WaitableEventWatcher* watcher)
+    : watcher_(watcher) {
+};
+
+void WaitableEventWatcher::ObjectWatcherHelper::OnObjectSignaled(HANDLE h) {
+  watcher_->OnObjectSignaled();
+}
+
+
 WaitableEventWatcher::WaitableEventWatcher()
-    : event_(NULL) {
+    : ALLOW_THIS_IN_INITIALIZER_LIST(helper_(this)),
+      event_(NULL),
+      delegate_(NULL) {
 }
 
 WaitableEventWatcher::~WaitableEventWatcher() {
 }
 
-bool WaitableEventWatcher::StartWatching(
-    WaitableEvent* event,
-    const EventCallback& callback) {
-  callback_ = callback;
+bool WaitableEventWatcher::StartWatching(WaitableEvent* event,
+                                         Delegate* delegate) {
+  delegate_ = delegate;
   event_ = event;
-  return watcher_.StartWatching(event->handle(), this);
+
+  return watcher_.StartWatching(event->handle(), &helper_);
 }
 
 void WaitableEventWatcher::StopWatching() {
-  callback_.Reset();
+  delegate_ = NULL;
   event_ = NULL;
   watcher_.StopWatching();
 }
@@ -35,14 +47,14 @@ WaitableEvent* WaitableEventWatcher::GetWatchedEvent() {
   return event_;
 }
 
-void WaitableEventWatcher::OnObjectSignaled(HANDLE h) {
+void WaitableEventWatcher::OnObjectSignaled() {
   WaitableEvent* event = event_;
-  EventCallback callback = callback_;
+  Delegate* delegate = delegate_;
   event_ = NULL;
-  callback_.Reset();
+  delegate_ = NULL;
   DCHECK(event);
 
-  callback.Run(event);
+  delegate->OnWaitableEventSignaled(event);
 }
 
 }  // namespace base