Skip to content

Commit

Permalink
Refactor: Simplify WaitableEventWatcher.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
teravest@chromium.org committed Feb 4, 2013
1 parent b02d89c commit 329be05
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 160 deletions.
94 changes: 22 additions & 72 deletions base/synchronization/waitable_event_watcher.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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_;
// };
//
Expand All @@ -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
Expand Down
25 changes: 13 additions & 12 deletions base/synchronization/waitable_event_watcher_posix.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,23 +97,22 @@ 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);
}
}

WaitableEventWatcher::WaitableEventWatcher()
: message_loop_(NULL),
cancel_flag_(NULL),
waiter_(NULL),
event_(NULL),
delegate_(NULL) {
event_(NULL) {
}

WaitableEventWatcher::~WaitableEventWatcher() {
Expand All @@ -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";
Expand All @@ -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_) {
Expand All @@ -159,22 +160,22 @@ 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;
}

message_loop_ = current_ml;
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);
Expand Down Expand Up @@ -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;
}
Expand Down
38 changes: 18 additions & 20 deletions base/synchronization/waitable_event_watcher_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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:
Expand All @@ -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();
Expand All @@ -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();
}
Expand All @@ -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();

Expand All @@ -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));
}
}
}
Expand All @@ -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;
}
}
Expand Down
34 changes: 11 additions & 23 deletions base/synchronization/waitable_event_watcher_win.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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
Loading

0 comments on commit 329be05

Please sign in to comment.