Stop using WaitMessage in MessagePumpForUI::WaitForWork

base/message_loop/message_pump_win.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/message_loop/message_pump_win.h"
 
 #include <math.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 202 matching lines @@
   DCHECK(atom_);
 
   message_hwnd_ = CreateWindow(MAKEINTATOM(atom_), 0, 0, 0, 0, 0, 0,
                                HWND_MESSAGE, 0, instance, 0);
   DCHECK(message_hwnd_);
 }
 
 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;
+  int delay;
+  DWORD wait_flags = MWMO_INPUTAVAILABLE;
 
-  DWORD result;
-  result = MsgWaitForMultipleObjectsEx(0, NULL, delay, QS_ALLINPUT,
-                                       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;
 
-  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();
+    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() {
   // If we are being called outside of the context of Run, then don't try to do
   // any work.  This could correspond to a MessageBox call or something of that
   // sort.
   if (!state_) {
     // Since we handled a kMsgHaveWork message, we must still update this flag.
     InterlockedExchange(&have_work_, 0);
     return;
@@ skipping 377 matching lines @@
 
 // static
 MessagePumpForIO::IOHandler* MessagePumpForIO::KeyToHandler(
     ULONG_PTR key,
     bool* has_valid_io_context) {
   *has_valid_io_context = ((key & 1) == 0);
   return reinterpret_cast<IOHandler*>(key & ~static_cast<ULONG_PTR>(1));
 }
 
 }  // namespace base