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

base/synchronization/waitable_event_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/synchronization/waitable_event.h"
 
 #include "base/compiler_specific.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 55 matching lines @@
 
   ev[0]->Signal();
   EXPECT_EQ(WaitableEvent::WaitMany(ev, 5), 0u);
 
   for (unsigned i = 0; i < 5; ++i)
     delete ev[i];
 }
 
 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);
 
   ev->Wait();
   delete ev;
 
   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);
 
   size_t index = WaitableEvent::WaitMany(ev, 5);
 
   for (unsigned i = 0; i < 5; ++i)
     delete ev[i];
 
   PlatformThread::Join(thread);
   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