Make sure that WaitableEvent::TimedWait works fine with large timeouts.

base/synchronization/waitable_event_unittest.cc

Index: base/synchronization/waitable_event_unittest.cc
diff --git a/base/synchronization/waitable_event_unittest.cc b/base/synchronization/waitable_event_unittest.cc
index abba9356bb17a2e967863a2e06e0a6a4dfaed092..7d8e419a18c3a21a38b075a994863a7625e761ba 100644
--- a/base/synchronization/waitable_event_unittest.cc
+++ b/base/synchronization/waitable_event_unittest.cc
@@ -73,29 +73,27 @@ TEST(WaitableEventTest, WaitManyShortcut) {
 
 class WaitableEventSignaler : public PlatformThread::Delegate {
  public:
-  WaitableEventSignaler(double seconds, WaitableEvent* ev)
-      : seconds_(seconds),
-        ev_(ev) {
+  WaitableEventSignaler(TimeDelta delay, WaitableEvent* event)
+      : delay_(delay),
+        event_(event) {
   }
 
   void ThreadMain() override {
-    PlatformThread::Sleep(TimeDelta::FromSecondsD(seconds_));
-    ev_->Signal();
+    PlatformThread::Sleep(delay_);
+    event_->Signal();
   }
 
  private:
-  const double seconds_;
-  WaitableEvent *const ev_;
+  TimeDelta delay_;

[Lei Zhang, 2015/03/25 01:36:27]

nit: can this stay const since it doesn't change?

[rvargas (doing something else), 2015/03/26 19:37:28]

Done.

+  WaitableEvent* event_;
 };
 
+// Tests that a WaitableEvent can be safely deleted when |Wait| is done without
+// additional synchrnization.

[Lei Zhang, 2015/03/25 01:36:27]

nit: typo (cut+pasted), ditto below.

[rvargas (doing something else), 2015/03/26 19:37:28]

Done.

 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(0.01, ev);
+  WaitableEventSignaler signaler(TimeDelta::FromMilliseconds(10), ev);
   PlatformThreadHandle thread;
   PlatformThread::Create(0, &signaler, &thread);
 
@@ -105,16 +103,14 @@ TEST(WaitableEventTest, WaitAndDelete) {
   PlatformThread::Join(thread);
 }
 
+// Tests that a WaitableEvent can be safely deleted when |WaitMany| is done
+// without additional synchrnization.
 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(0.01, ev[2]);
+  WaitableEventSignaler signaler(TimeDelta::FromMilliseconds(10), ev[2]);
   PlatformThreadHandle thread;
   PlatformThread::Create(0, &signaler, &thread);
 
@@ -127,4 +123,25 @@ 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);
+
+  TimeTicks end_time(TimeTicks::Now() + TimeDelta::Max());

[Lei Zhang, 2015/03/25 01:36:27]

I can't tell what part this plays in the test. Should it be in
base/time/time_unittest.cc instead?

[rvargas (doing something else), 2015/03/26 19:37:28]

Sorry, this was a quick test I did weeks ago before filing bug 466445 and forgot
it was here :(

Removed.

+  EXPECT_LT(TimeTicks(), end_time);
+}
+
 }  // namespace base