Revert "Make sure that WaitableEvent::TimedWait works fine with large…

… timeouts."

This reverts commit ac3c756.

BUG=471426, 465948
TBR=thestig@chromium.org

Review URL: https://codereview.chromium.org/1048883002

Cr-Commit-Position: refs/heads/master@{#322829}

base/synchronization/waitable_event.h

@@ -81,8 +81,6 @@ class BASE_EXPORT WaitableEvent {
   // does not necessarily mean that max_time was exceeded.
   //
   // TimedWait can synchronise its own destruction like |Wait|.
-  //
-  // Passing a negative |max_time| is not supported.
   bool TimedWait(const TimeDelta& max_time);
 
 #if defined(OS_WIN)

base/synchronization/waitable_event_posix.cc

@@ -151,14 +151,14 @@ class SyncWaiter : public WaitableEvent::Waiter {
 };
 
 void WaitableEvent::Wait() {
-  bool result = TimedWait(TimeDelta::Max());
+  bool result = TimedWait(TimeDelta::FromSeconds(-1));
   DCHECK(result) << "TimedWait() should never fail with infinite timeout";
 }
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
-  DCHECK_GE(max_time, TimeDelta());
   base::ThreadRestrictions::AssertWaitAllowed();
   const TimeTicks end_time(TimeTicks::Now() + max_time);
+  const bool finite_time = max_time.ToInternalValue() >= 0;
 
   kernel_->lock_.Acquire();
   if (kernel_->signaled_) {
@@ -184,7 +184,7 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   for (;;) {
     const TimeTicks current_time(TimeTicks::Now());
 
-    if (sw.fired() || current_time >= end_time) {
+    if (sw.fired() || (finite_time && current_time >= end_time)) {
       const bool return_value = sw.fired();
 
       // We can't acquire @lock_ before releasing the SyncWaiter lock (because
@@ -207,7 +207,12 @@ bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
       return return_value;
     }
 
-    sw.cv()->TimedWait(end_time - current_time);
+    if (finite_time) {
+      const TimeDelta max_wait(end_time - current_time);
+      sw.cv()->TimedWait(max_wait);
+    } else {
+      sw.cv()->Wait();
+    }
   }
 }
 

base/synchronization/waitable_event_unittest.cc

@@ -73,27 +73,29 @@ TEST(WaitableEventTest, WaitManyShortcut) {
 
 class WaitableEventSignaler : public PlatformThread::Delegate {
  public:
-  WaitableEventSignaler(TimeDelta delay, WaitableEvent* event)
-      : delay_(delay),
-        event_(event) {
+  WaitableEventSignaler(double seconds, WaitableEvent* ev)
+      : seconds_(seconds),
+        ev_(ev) {
   }
 
   void ThreadMain() override {
-    PlatformThread::Sleep(delay_);
-    event_->Signal();
+    PlatformThread::Sleep(TimeDelta::FromSecondsD(seconds_));
+    ev_->Signal();
   }
 
  private:
-  const TimeDelta delay_;
-  WaitableEvent* event_;
+  const double seconds_;
+  WaitableEvent *const ev_;
 };
 
-// Tests that a WaitableEvent can be safely deleted when |Wait| is done without
-// additional synchronization.
 TEST(WaitableEventTest, WaitAndDelete) {
+  // This test tests that if a WaitableEvent can be safely deleted
+  // when |Wait| is done without additional synchrnization.
+  // If this test crashes, it is a bug.
+
   WaitableEvent* ev = new WaitableEvent(false, false);
 
-  WaitableEventSignaler signaler(TimeDelta::FromMilliseconds(10), ev);
+  WaitableEventSignaler signaler(0.01, ev);
   PlatformThreadHandle thread;
   PlatformThread::Create(0, &signaler, &thread);
 
@@ -103,14 +105,16 @@ TEST(WaitableEventTest, WaitAndDelete) {
   PlatformThread::Join(thread);
 }
 
-// Tests that a WaitableEvent can be safely deleted when |WaitMany| is done
-// without additional synchronization.
 TEST(WaitableEventTest, WaitMany) {
+  // This test tests that if a WaitableEvent can be safely deleted
+  // when |WaitMany| is done without additional synchrnization.
+  // If this test crashes, it is a bug.
+
   WaitableEvent* ev[5];
   for (unsigned i = 0; i < 5; ++i)
     ev[i] = new WaitableEvent(false, false);
 
-  WaitableEventSignaler signaler(TimeDelta::FromMilliseconds(10), ev[2]);
+  WaitableEventSignaler signaler(0.01, ev[2]);
   PlatformThreadHandle thread;
   PlatformThread::Create(0, &signaler, &thread);
 
@@ -123,22 +127,4 @@ TEST(WaitableEventTest, WaitMany) {
   EXPECT_EQ(2u, index);
 }
 
-// Tests that using TimeDelta::Max() on TimedWait() is not the same as passing
-// a timeout of 0. (crbug.com/465948)
-TEST(WaitableEventTest, TimedWait) {
-  WaitableEvent* ev = new WaitableEvent(false, false);
-
-  TimeDelta thread_delay = TimeDelta::FromMilliseconds(10);
-  WaitableEventSignaler signaler(thread_delay, ev);
-  PlatformThreadHandle thread;
-  TimeTicks start = TimeTicks::Now();
-  PlatformThread::Create(0, &signaler, &thread);
-
-  ev->TimedWait(TimeDelta::Max());
-  EXPECT_GE(TimeTicks::Now() - start, thread_delay);
-  delete ev;
-
-  PlatformThread::Join(thread);
-}
-
 }  // namespace base

base/synchronization/waitable_event_win.cc

@@ -4,10 +4,10 @@
 
 #include "base/synchronization/waitable_event.h"
 
+#include <math.h>
 #include <windows.h>
 
 #include "base/logging.h"
-#include "base/numerics/safe_conversions.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 
@@ -37,7 +37,7 @@ void WaitableEvent::Signal() {
 }
 
 bool WaitableEvent::IsSignaled() {
-  return TimedWait(TimeDelta());
+  return TimedWait(TimeDelta::FromMilliseconds(0));
 }
 
 void WaitableEvent::Wait() {
@@ -50,13 +50,13 @@ void WaitableEvent::Wait() {
 
 bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
   base::ThreadRestrictions::AssertWaitAllowed();
-  DCHECK_GE(max_time, TimeDelta());
+  DCHECK(max_time >= TimeDelta::FromMicroseconds(0));
   // Be careful here.  TimeDelta has a precision of microseconds, but this API
   // is in milliseconds.  If there are 5.5ms left, should the delay be 5 or 6?
   // It should be 6 to avoid returning too early.
-  DWORD timeout = saturated_cast<DWORD>(max_time.InMillisecondsRoundedUp());
-
-  DWORD result = WaitForSingleObject(handle_.Get(), timeout);
+  double timeout = ceil(max_time.InMillisecondsF());
+  DWORD result = WaitForSingleObject(handle_.Get(),
+                                     static_cast<DWORD>(timeout));
   switch (result) {
     case WAIT_OBJECT_0:
       return true;