Modify WaitableEvent::Wait() to return void

Currently, WaitableEvent::Wait() returns bool. However, the Windows
implementation DCHECKs that the return value is true and the POSIX
implementation can never return false. Also, all call sites that use the return
value simply DCHECK that it's true.

This change modifies the method to return void, adds a DCHECK in the POSIX
implementation and updates call sites.


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

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

4 files changed, 21 insertions, 22 deletions

diff --git a/base/synchronization/waitable_event.h b/base/synchronization/waitable_event.h
index 62712ed..6c91701 100644
--- a/base/synchronization/waitable_event.h
+++ b/base/synchronization/waitable_event.h
@@ -73,9 +73,8 @@ class BASE_EXPORT WaitableEvent {
   // is not a manual reset event, then this test will cause a reset.
   bool IsSignaled();
 
-  // Wait indefinitely for the event to be signaled.  Returns true if the event
-  // was signaled, else false is returned to indicate that waiting failed.
-  bool Wait();
+  // Wait indefinitely for the event to be signaled.
+  void Wait();
 
   // Wait up until max_time has passed for the event to be signaled.  Returns
   // true if the event was signaled.  If this method returns false, then it
diff --git a/base/synchronization/waitable_event_posix.cc b/base/synchronization/waitable_event_posix.cc
index ae03ead..87567c8 100644
--- a/base/synchronization/waitable_event_posix.cc
+++ b/base/synchronization/waitable_event_posix.cc
@@ -149,8 +149,9 @@ class SyncWaiter : public WaitableEvent::Waiter {
   base::ConditionVariable cv_;
 };
 
-bool WaitableEvent::Wait() {
-  return TimedWait(TimeDelta::FromSeconds(-1));
+void WaitableEvent::Wait() {
+  bool result = TimedWait(TimeDelta::FromSeconds(-1));
+  DCHECK(result) << "TimedWait() should never fail with infinite timeout";
 }
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
@@ -158,21 +159,21 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   const bool finite_time = max_time.ToInternalValue() >= 0;
 
   kernel_->lock_.Acquire();
-    if (kernel_->signaled_) {
-      if (!kernel_->manual_reset_) {
-        // In this case we were signaled when we had no waiters. Now that
-        // someone has waited upon us, we can automatically reset.
-        kernel_->signaled_ = false;
-      }
-
-      kernel_->lock_.Release();
-      return true;
+  if (kernel_->signaled_) {
+    if (!kernel_->manual_reset_) {
+      // In this case we were signaled when we had no waiters. Now that
+      // someone has waited upon us, we can automatically reset.
+      kernel_->signaled_ = false;
     }
 
-    SyncWaiter sw;
-    sw.lock()->Acquire();
+    kernel_->lock_.Release();
+    return true;
+  }
+
+  SyncWaiter sw;
+  sw.lock()->Acquire();
 
-    Enqueue(&sw);
+  Enqueue(&sw);
   kernel_->lock_.Release();
   // We are violating locking order here by holding the SyncWaiter lock but not
   // the WaitableEvent lock. However, this is safe because we don't lock @lock_
@@ -193,7 +194,7 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
       sw.lock()->Release();
 
       kernel_->lock_.Acquire();
-        kernel_->Dequeue(&sw, &sw);
+      kernel_->Dequeue(&sw, &sw);
       kernel_->lock_.Release();
 
       return return_value;
diff --git a/base/synchronization/waitable_event_unittest.cc b/base/synchronization/waitable_event_unittest.cc
index 47e7ff7..ad86d14 100644
--- a/base/synchronization/waitable_event_unittest.cc
+++ b/base/synchronization/waitable_event_unittest.cc
@@ -23,7 +23,7 @@ TEST(WaitableEventTest, ManualBasics) {
   EXPECT_FALSE(event.TimedWait(TimeDelta::FromMilliseconds(10)));
 
   event.Signal();
-  EXPECT_TRUE(event.Wait());
+  event.Wait();
   EXPECT_TRUE(event.TimedWait(TimeDelta::FromMilliseconds(10)));
 }
 
@@ -41,7 +41,7 @@ TEST(WaitableEventTest, AutoBasics) {
   EXPECT_FALSE(event.TimedWait(TimeDelta::FromMilliseconds(10)));
 
   event.Signal();
-  EXPECT_TRUE(event.Wait());
+  event.Wait();
   EXPECT_FALSE(event.TimedWait(TimeDelta::FromMilliseconds(10)));
 
   event.Signal();
diff --git a/base/synchronization/waitable_event_win.cc b/base/synchronization/waitable_event_win.cc
index a0e39c1..31932bd 100644
--- a/base/synchronization/waitable_event_win.cc
+++ b/base/synchronization/waitable_event_win.cc
@@ -46,12 +46,11 @@ bool WaitableEvent::IsSignaled() {
   return TimedWait(TimeDelta::FromMilliseconds(0));
 }
 
-bool WaitableEvent::Wait() {
+void WaitableEvent::Wait() {
   DWORD result = WaitForSingleObject(handle_, INFINITE);
   // It is most unexpected that this should ever fail.  Help consumers learn
   // about it if it should ever fail.
   DCHECK_EQ(WAIT_OBJECT_0, result) << "WaitForSingleObject failed";
-  return result == WAIT_OBJECT_0;
 }
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {