Ignore WM_QUIT messages handled by MessagePumpForUI.

WM_QUIT messages are intended to be used internally by applications to
exit message-pumping loops built using the GetMessage() API.

Chromium MessageLoop/RunLoop have a common cross-platform Quit()
mechanism, and much of our code assumes that loops will run until we
explicitly ask them to Quit(), so we have no need to process WM_QUIT.

We might see WM_QUIT messages injected by other applications, for some
reason, and some code we run will use a GetMessage() loop and exit that
via PostQuitMessage(). If a nested GetMessage() loop triggers multiple
PostQuitMessage() calls before returning then we may end up with our
MessagePumpForUI receiving WM_QUIT unintentionally.

Bug: 720078
Change-Id: I11df406ab4fd1d0a3b56899bf1a710ef51c048e9
Reviewed-on: https://chromium-review.googlesource.com/1020452
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553653}

base/message_loop/message_loop_unittest.cc

@@ -24,6 +24,7 @@
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/task_scheduler.h"
 #include "base/test/test_simple_task_runner.h"
+#include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/sequence_local_storage_slot.h"
 #include "base/threading/thread.h"
@@ -1772,6 +1773,33 @@ INSTANTIATE_TEST_CASE_P(
     MessageLoopTypedTest::ParamInfoToString);
 
 #if defined(OS_WIN)
+// Verifies that the MessageLoop ignores WM_QUIT, rather than quitting.
+// Users of MessageLoop typically expect to control when their RunLoops stop
+// Run()ning explicitly, via QuitClosure() etc (see https://crbug.com/720078)
+TEST_P(MessageLoopTest, WmQuitIsIgnored) {
+  MessageLoop loop(MessageLoop::TYPE_UI);
+  RunLoop run_loop;
+  // Post a WM_QUIT message to the current thread.
+  ::PostQuitMessage(0);
+
+  // Post a task to the current thread, with a small delay to make it less
+  // likely that we process the posted task before looking for WM_* messages.
+  bool task_was_run = false;
+  loop.task_runner()->PostDelayedTask(
+      FROM_HERE,
+      BindOnce(
+          [](bool* flag, OnceClosure closure) {
+            *flag = true;
+            std::move(closure).Run();
+          },
+          &task_was_run, run_loop.QuitClosure()),
+      TestTimeouts::tiny_timeout());
+
+  // Run the loop, and ensure that the posted task is processed before we quit.
+  run_loop.Run();
+  EXPECT_TRUE(task_was_run);
+}
+
 TEST_P(MessageLoopTest, PostDelayedTask_SharedTimer_SubPump) {
   RunTest_PostDelayedTask_SharedTimer_SubPump();
 }

base/message_loop/message_pump_win.cc

@@ -351,14 +351,12 @@ bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) {
   TRACE_EVENT1("base", "MessagePumpForUI::ProcessMessageHelper",
                "message", msg.message);
   if (WM_QUIT == msg.message) {
-    // Receiving WM_QUIT is unusual and unexpected on most message loops.
+    // WM_QUIT is the standard way to exit a GetMessage() loop. Our MessageLoop
+    // has its own quit mechanism, so WM_QUIT is unexpected and should be
+    // ignored.
     UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem",
                               RECEIVED_WM_QUIT_ERROR, MESSAGE_LOOP_PROBLEM_MAX);
-    // Repost the QUIT message so that it will be retrieved by the primary
-    // GetMessage() loop.
-    state_->should_quit = true;
-    PostQuitMessage(static_cast<int>(msg.wParam));
-    return false;
+    return true;
   }
 
   // While running our main message pump, we discard kMsgHaveWork messages.