Move OS_WIN specific logic in MessagePumpDefault::Run into waitable_event_win.cc

base/synchronization/waitable_event_win.cc

Index: base/synchronization/waitable_event_win.cc
diff --git a/base/synchronization/waitable_event_win.cc b/base/synchronization/waitable_event_win.cc
index d80cabb3ff3ae864fb25f54448885d2c252c5f2b..db511d4cc1a4616d018d80b8e0a90bca66b66b06 100644
--- a/base/synchronization/waitable_event_win.cc
+++ b/base/synchronization/waitable_event_win.cc
@@ -7,6 +7,7 @@
 #include <windows.h>
 #include <stddef.h>
 
+#include <algorithm>
 #include <utility>
 
 #include "base/debug/activity_tracker.h"
@@ -44,54 +45,94 @@ void WaitableEvent::Signal() {
 }
 
 bool WaitableEvent::IsSignaled() {
-  return TimedWait(TimeDelta());
+  DWORD result = WaitForSingleObject(handle_.Get(), 0);
+  DCHECK(result == WAIT_OBJECT_0 || result == WAIT_TIMEOUT)
+      << "Unexpected WaitForSingleObject result " << result;
+  return result == WAIT_OBJECT_0;
 }
 
 void WaitableEvent::Wait() {
+  base::ThreadRestrictions::AssertWaitAllowed();
   // Record the event that this thread is blocking upon (for hang diagnosis).
   base::debug::ScopedEventWaitActivity event_activity(this);
 
-  base::ThreadRestrictions::AssertWaitAllowed();
   DWORD result = WaitForSingleObject(handle_.Get(), 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";
 }
 
-bool WaitableEvent::TimedWait(const TimeDelta& max_time) {
+namespace {
+// Helper function called from TimedWait and TimedWaitUntil.
+bool WaitUntil(HANDLE handle, const TimeTicks& now, const TimeTicks& end_time) {
+  TimeDelta delta = end_time - now;
+  DCHECK_GT(delta, TimeDelta());
+
+  do {
+    // On Windows, waiting for less than 1 ms results in WaitForSingleObject
+    // returning promptly which may result in the caller code spinning.
+    // We need to ensure that we specify at least the minimally possible 1 ms
+    // delay unless the initial timeout was exactly zero.
+    delta = std::max(delta, TimeDelta::FromMilliseconds(1));
+    // Truncate the timeout to milliseconds.
+    DWORD timeout_ms = saturated_cast<DWORD>(delta.InMilliseconds());
+    DWORD result = WaitForSingleObject(handle, timeout_ms);
+    DCHECK(result == WAIT_OBJECT_0 || result == WAIT_TIMEOUT)
+        << "Unexpected WaitForSingleObject result " << result;
+    switch (result) {
+      case WAIT_OBJECT_0:
+        return true;
+      case WAIT_TIMEOUT:
+        // TimedWait can time out earlier than the specified |timeout| on
+        // Windows. To make this consistent with the posix implementation we
+        // should guarantee that TimedWait doesn't return earlier than the
+        // specified |max_time| and wait again for the remaining time.
+        delta = end_time - TimeTicks::Now();
+        break;
+    }
+  } while (delta > TimeDelta());
+  return false;
+}
+}  // namespace

[danakj, 2016/11/29 02:46:45]

nit: whitespace lines around the namespace { and }

[stanisc, 2016/11/29 22:33:21]

Done.

+
+bool WaitableEvent::TimedWait(const TimeDelta& max_delta) {

[danakj, 2016/11/29 02:46:45]

same name nit

[stanisc, 2016/11/29 22:33:21]

Done.

+  DCHECK_GE(max_delta, TimeDelta());
+  if (max_delta.is_zero())
+    return IsSignaled();
+
+  base::ThreadRestrictions::AssertWaitAllowed();
   // Record the event that this thread is blocking upon (for hang diagnosis).
   base::debug::ScopedEventWaitActivity event_activity(this);
 
-  DCHECK_GE(max_time, TimeDelta());
-  if (!max_time.is_zero())
-    base::ThreadRestrictions::AssertWaitAllowed();
-
-  // Truncate the timeout to milliseconds. The API specifies that this method
-  // can return in less than |max_time| (when returning false), as the argument
-  // is the maximum time that a caller is willing to wait.
-  DWORD timeout = saturated_cast<DWORD>(max_time.InMilliseconds());
-
-  DWORD result = WaitForSingleObject(handle_.Get(), timeout);
-  switch (result) {
-    case WAIT_OBJECT_0:
-      return true;
-    case WAIT_TIMEOUT:
-      return false;
-  }
-  // It is most unexpected that this should ever fail.  Help consumers learn
-  // about it if it should ever fail.
-  NOTREACHED() << "WaitForSingleObject failed";
-  return false;
+  TimeTicks now(TimeTicks::Now());
+  // TimeTicks takes care of overflow including the cases when max_delta
+  // is a maximum value.
+  return WaitUntil(handle_.Get(), now, now + max_delta);
+}
+
+bool WaitableEvent::TimedWaitUntil(const TimeTicks& end_time) {
+  if (end_time.is_null())
+    return IsSignaled();
+
+  base::ThreadRestrictions::AssertWaitAllowed();
+  // Record the event that this thread is blocking upon (for hang diagnosis).
+  base::debug::ScopedEventWaitActivity event_activity(this);
+
+  TimeTicks now(TimeTicks::Now());
+  if (end_time <= now)
+    return IsSignaled();
+
+  return WaitUntil(handle_.Get(), now, end_time);
 }
 
 // static
 size_t WaitableEvent::WaitMany(WaitableEvent** events, size_t count) {
   DCHECK(count) << "Cannot wait on no events";
 
+  base::ThreadRestrictions::AssertWaitAllowed();
   // Record an event (the first) that this thread is blocking upon.
   base::debug::ScopedEventWaitActivity event_activity(events[0]);
 
-  base::ThreadRestrictions::AssertWaitAllowed();
   HANDLE handles[MAXIMUM_WAIT_OBJECTS];
   CHECK_LE(count, static_cast<size_t>(MAXIMUM_WAIT_OBJECTS))
       << "Can only wait on " << MAXIMUM_WAIT_OBJECTS << " with WaitMany";