Stop using WaitMessage in MessagePumpForUI::WaitForWork

base/message_loop/message_pump_win.cc

Index: base/message_loop/message_pump_win.cc
diff --git a/base/message_loop/message_pump_win.cc b/base/message_loop/message_pump_win.cc
index cb68a24d74a2979ece7ef57076a80e27ac3d788c..9729e4d651b3c8ee097c6c8b2ca26019f3e9fab1 100644
--- a/base/message_loop/message_pump_win.cc
+++ b/base/message_loop/message_pump_win.cc
@@ -220,37 +220,46 @@ void MessagePumpForUI::InitMessageWnd() {
 void MessagePumpForUI::WaitForWork() {
   // Wait until a message is available, up to the time needed by the timer
   // manager to fire the next set of timers.
-  int delay = GetCurrentDelay();
-  if (delay < 0)  // Negative value means no timers waiting.
-    delay = INFINITE;
-
-  DWORD result;
-  result = MsgWaitForMultipleObjectsEx(0, NULL, delay, QS_ALLINPUT,
-                                       MWMO_INPUTAVAILABLE);
-
-  if (WAIT_OBJECT_0 == result) {
-    // A WM_* message is available.
-    // If a parent child relationship exists between windows across threads
-    // then their thread inputs are implicitly attached.
-    // This causes the MsgWaitForMultipleObjectsEx API to return indicating
-    // that messages are ready for processing (Specifically, mouse messages
-    // intended for the child window may appear if the child window has
-    // capture).
-    // The subsequent PeekMessages call may fail to return any messages thus
-    // causing us to enter a tight loop at times.
-    // The WaitMessage call below is a workaround to give the child window
-    // some time to process its input messages.
-    MSG msg = {0};
-    bool has_pending_sent_message =
-        (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
-    if (!has_pending_sent_message &&
-        !PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE)) {
-      WaitMessage();
+  int delay;
+  DWORD wait_flags = MWMO_INPUTAVAILABLE;
+
+  while ((delay = GetCurrentDelay()) != 0) {

[danakj, 2016/05/11 01:13:36]

The old code when delay was 0 would call MsgWaitForMultipleObjectsEx still and
just return immediately regardless, which would cause |result| to be set, and if
WAIT_OBJECT_0 was the result, it would wait for a new message.

The new code will return if delay is 0. Is this intentional, why?

[stanisc, 2016/05/11 01:46:41]

You are right. This is a suble difference with the previous implementation that
I forgot to mention.
If the delay is 0 MsgWaitForMultupleObjectsEx would return promptly with either
WAIT_OBJECT_0 or WAIT_TIMEOUT. The zero delay means that a pending delayed task
is overdue. The right way to handle that is to exit WaitForWork and iterate
through the loop allowing the delayed task to be done. The previous
implementation could theoretically enter WaitMessage and get stuck there for a
while (if there was a pending message belonging to another thread) which in my
opinion isn't the right thing to do.

Another difference of this version with the previous version is that it would
also enter MsgWaitForMultupleObjectsEx repeatedly if a previous call to
MsgWaitForMultupleObjectsEx ended with WAIT_TIMEOUT too early. In this situation
we know for sure that there is nothing to do - no messages to dispatch, no new
work, and no delayed work due. So going back to wait saves the trouble of
reloading the work queue.

+    if (delay < 0)  // Negative value means no timers waiting.
+      delay = INFINITE;
+
+    DWORD result =
+        MsgWaitForMultipleObjectsEx(0, NULL, delay, QS_ALLINPUT, wait_flags);
+
+    if (WAIT_OBJECT_0 == result) {
+      // A WM_* message is available.
+      // If a parent child relationship exists between windows across threads
+      // then their thread inputs are implicitly attached.
+      // This causes the MsgWaitForMultipleObjectsEx API to return indicating
+      // that messages are ready for processing (Specifically, mouse messages
+      // intended for the child window may appear if the child window has
+      // capture).
+      // The subsequent PeekMessages call may fail to return any messages thus
+      // causing us to enter a tight loop at times.
+      // The code below is a workaround to give the child window
+      // some time to process its input messages by looping back to
+      // MsgWaitForMultipleObjectsEx above when there are no messages for the
+      // current thread.
+      MSG msg = {0};
+      bool has_pending_sent_message =
+          (HIWORD(GetQueueStatus(QS_SENDMESSAGE)) & QS_SENDMESSAGE) != 0;
+      if (has_pending_sent_message ||
+          PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE)) {
+        return;
+      }
+
+      // We know there are no more messages for this thread because PeekMessage
+      // has returned false. Reset |wait_flags| so that we wait for a *new*
+      // message.
+      wait_flags = 0;
     }
-    return;
-  }
 
-  DCHECK_NE(WAIT_FAILED, result) << GetLastError();
+    DCHECK_NE(WAIT_FAILED, result) << GetLastError();
+  }
 }
 
 void MessagePumpForUI::HandleWorkMessage() {