Mac/Linux test fixes: hopefully this will fix some flakey tests

Review URL: http://codereview.chromium.org/18298


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

2 files changed, 54 insertions, 30 deletions

diff --git a/base/waitable_event_posix.cc b/base/waitable_event_posix.cc
index 335f13e..09e2868 100644
--- a/base/waitable_event_posix.cc
+++ b/base/waitable_event_posix.cc
@@ -35,13 +35,16 @@ namespace base {
 // This is just an abstract base class for waking the two types of waiters
 // -----------------------------------------------------------------------------
 WaitableEvent::WaitableEvent(bool manual_reset, bool initially_signaled)
-    : signaled_(false),
+    : signaled_(initially_signaled),
       manual_reset_(manual_reset) {
-  DCHECK(!initially_signaled) << "Not implemented";
 }
 
 WaitableEvent::~WaitableEvent() {
-  DCHECK(waiters_.empty()) << "Deleting WaitableEvent with listeners!";
+  if (!waiters_.empty()) {
+    LOG(ERROR) << "Destroying a WaitableEvent (" << this << ") with "
+               << waiters_.size() << " waiters";
+    NOTREACHED() << "Aborting.";
+  }
 }
 
 void WaitableEvent::Reset() {
@@ -88,7 +91,8 @@ class SyncWaiter : public WaitableEvent::Waiter {
       : fired_(false),
         cv_(cv),
         lock_(lock),
-        signaling_event_(NULL) { }
+        signaling_event_(NULL) {
+  }
 
   bool Fire(WaitableEvent *signaling_event) {
     lock_->Acquire();
@@ -144,6 +148,7 @@ class SyncWaiter : public WaitableEvent::Waiter {
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   const Time end_time(Time::Now() + max_time);
+  const bool finite_time = max_time.ToInternalValue() >= 0;
 
   lock_.Acquire();
     if (signaled_) {
@@ -169,32 +174,31 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   // again before unlocking it.
 
   for (;;) {
-    if (sw.fired()) {
+    const Time current_time(Time::Now());
+
+    if (sw.fired() || (finite_time && current_time >= end_time)) {
+      const bool return_value = sw.fired();
+
+      // We can't acquire @lock_ before releasing @lock (because of locking
+      // order), however, inbetween the two a signal could be fired and @sw
+      // would accept it, however we will still return false, so the signal
+      // would be lost on an auto-reset WaitableEvent. Thus we call Disable
+      // which makes sw::Fire return false.
+      sw.Disable();
       lock.Release();
-      return true;
+
+      lock_.Acquire();
+        Dequeue(&sw, &sw);
+      lock_.Release();
+
+      return return_value;
     }
 
-    if (max_time.ToInternalValue() < 0) {
-      cv.Wait();
-    } else {
-      const Time current_time(Time::Now());
-      if (current_time >= end_time) {
-        // We can't acquire @lock_ before releasing @lock (because of locking
-        // order), however, inbetween the two a signal could be fired and @sw
-        // would accept it, however we will still return false, so the signal
-        // would be lost on an auto-reset WaitableEvent. Thus we call Disable
-        // which makes sw::Fire return false.
-        sw.Disable();
-        lock.Release();
-
-        lock_.Acquire();
-          Dequeue(&sw, &sw);
-        lock_.Release();
-        return false;
-      }
+    if (finite_time) {
       const TimeDelta max_wait(end_time - current_time);
-
       cv.TimedWait(max_wait);
+    } else {
+      cv.Wait();
     }
   }
 }
diff --git a/base/waitable_event_watcher_posix.cc b/base/waitable_event_watcher_posix.cc
index e4e1a7f..10d47b7 100644
--- a/base/waitable_event_watcher_posix.cc
+++ b/base/waitable_event_watcher_posix.cc
@@ -80,6 +80,7 @@ class AsyncWaiter : public WaitableEvent::Waiter {
     return tag == flag_.get();
   }
 
+ private:
   MessageLoop *const message_loop_;
   Task *const cb_task_;
   scoped_refptr<Flag> flag_;
@@ -101,12 +102,12 @@ class AsyncCallbackTask : public Task {
 
   void Run() {
     // Runs in MessageLoop thread.
-    if (!flag_->value())
+    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_);
-
-    // This is to let the WaitableEventWatcher know that the event has occured
-    // because it needs to be able to return NULL from GetWatchedEvent
-    flag_->Set();
+    }
 
     // We are deleted by the MessageLoop
   }
@@ -138,6 +139,18 @@ bool WaitableEventWatcher::StartWatching
   DCHECK(current_ml) << "Cannot create WaitableEventWatcher without a "
                         "current MessageLoop";
 
+  // A user may call StartWatching from within the callback function. In this
+  // case, we won't know that we have finished watching, expect that the Flag
+  // will have been set in AsyncCallbackTask::Run()
+  if (cancel_flag_.get() && cancel_flag_->value()) {
+    if (message_loop_) {
+      message_loop_->RemoveDestructionObserver(this);
+      message_loop_ = NULL;
+    }
+
+    cancel_flag_ = NULL;
+  }
+
   DCHECK(!cancel_flag_.get()) << "StartWatching called while still watching";
 
   cancel_flag_ = new Flag;
@@ -174,6 +187,13 @@ void WaitableEventWatcher::StopWatching() {
   if (!cancel_flag_.get())  // if not currently watching...
     return;
 
+  if (cancel_flag_->value()) {
+    // In this case, the event has fired, but we haven't figured that out yet.
+    // The WaitableEvent may have been deleted too.
+    cancel_flag_ = NULL;
+    return;
+  }
+
   if (!event_) {
     // We have no WaitableEvent. This means that we never enqueued a Waiter on
     // an event because the event was already signaled when StartWatching was