Revert "[base] Migrate the Win MessagePump to DoSomeWork"

This reverts commit 8c5fb8cb.

Reason for revert: DCHECK(!installed_native_timer_); in dev builds
when moving and resizing windows. Seems interactive_ui_tests might
not cover those cases..?

Original change's description:
> [base] Migrate the Win MessagePump to DoSomeWork
>
> This CL migrates both MessagePumpForUI and ForIO as they use the same
> base class and it's easier to do both at once.
>
> Tweaks to MessagePumpBase and/or both pumps:
>  * Use DoSomeWork() instead of DoWork()/DoDelatedWork()
>  * Replace |delayed_work_time_| by a timeout value computed on the stack
>    from the result of DoSomeWork(). Conversely making
>    ScheduleDelayedWork() a no-op as desired.
>  * Refactor GetCurrentDelay() into GetSleepTimeoutMs() and use modern
>    safe_conversions.h to perform the math.
>
> MessagePumpForUI specific tweaks:
>  * Fixed crbug.com/929263: ::SetTimer() used 0 as an nIDEvent but
>    ::KillTimer() used |this| so they weren't even interacting...
>  * Fixed bug: even with the previous bug fixed. HandleTimerMessage()
>    could be invoked while in a non-native DoRunLoop() (e.g. since
>    KillTimer doesn't remove WM_TIMER messages already enqueued) and as
>    such result in redundant sources of scheduling since it would then
>    continue to RescheduleTimer() and confusing state for DoRunLoop()).
>  * Ignore WM_TIMER messages which weren't scheduled by our ::SetTimer().
>  * Simplified usage of native ::SetTimer(). We now *only* rely on it
>    when in a native nested loop and kill it immediately after to avoid
>    double-scheduling.
>
> Note: MessageLoopTest.PostDelayedTask_SharedTimer_SubPump tests the
> necessity for a native timer when entering a native nested loop and
> confirms that the new behavior is WAI.
>
> Also adding regression tests for a tricky use case (and finding that
> it's already broken on ToT..!). Filed crbug.com/930940 to follow-up.
>
> Bug: 885371, 929263, 930940
> Change-Id: I7f2d18a1a5a494c6a4f31fcc0fad9b4de25d70a7
> Reviewed-on: https://chromium-review.googlesource.com/c/1455266
> Reviewed-by: Alex Clarke <alexclarke@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Auto-Submit: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#631231}

TBR=gab@chromium.org,fdoray@chromium.org,alexclarke@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 885371, 929263, 930940
Change-Id: Ic179d0493d01d4f44330927d9ccdb0f1454d2e78
Reviewed-on: https://chromium-review.googlesource.com/c/1471254Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631885}

base/message_loop/message_loop_unittest.cc

@@ -45,7 +45,6 @@
 #include "base/process/memory.h"
 #include "base/strings/string16.h"
 #include "base/win/current_module.h"
-#include "base/win/message_window.h"
 #include "base/win/scoped_handle.h"
 #endif
 
@@ -1648,90 +1647,6 @@ TEST_F(MessageLoopTest, PostDelayedTask_SharedTimer_SubPump) {
   EXPECT_TRUE(run_time.is_null());
 }
 
-namespace {
-
-// When this fires (per the associated WM_TIMER firing), it posts an
-// application task to quit the native loop.
-bool QuitOnSystemTimer(UINT message,
-                       WPARAM wparam,
-                       LPARAM lparam,
-                       LRESULT* result) {
-  if (message == static_cast<UINT>(WM_TIMER)) {
-    ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
-                                            BindOnce(&::PostQuitMessage, 0));
-  }
-  return true;
-}
-
-// When this fires (per the associated WM_TIMER firing), it posts a delayed
-// application task to quit the native loop.
-bool DelayedQuitOnSystemTimer(UINT message,
-                              WPARAM wparam,
-                              LPARAM lparam,
-                              LRESULT* result) {
-  if (message == static_cast<UINT>(WM_TIMER)) {
-    ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-        FROM_HERE, BindOnce(&::PostQuitMessage, 0),
-        TimeDelta::FromMilliseconds(10));
-  }
-  return true;
-}
-
-}  // namespace
-
-// This is a regression test for
-// https://crrev.com/c/1455266/9/base/message_loop/message_pump_win.cc#125
-// See below for the delayed task version.
-// TODO(alexclarke): This test never ends up calling
-// MessagePumpForUI::ScheduleWork().
-TEST_F(MessageLoopTest, DISABLED_PostImmediateTaskFromSystemPump) {
-  MessageLoop message_loop(MessageLoop::TYPE_UI);
-
-  RunLoop run_loop;
-
-  // A native message window to generate a system message which invokes
-  // QuitOnSystemTimer() when the native timer fires.
-  win::MessageWindow local_message_window;
-  local_message_window.Create(BindRepeating(&QuitOnSystemTimer));
-  ASSERT_TRUE(::SetTimer(local_message_window.hwnd(), 0, 20, nullptr));
-
-  // The first task will enter a native message loop. This test then verifies
-  // that the pump is able to run an immediate application task after the native
-  // pump went idle.
-  message_loop.task_runner()->PostTask(
-      FROM_HERE, BindOnce(&SubPumpFunc, run_loop.QuitClosure()));
-
-  // Test success is determined by not hanging in this Run() call.
-  run_loop.Run();
-}
-
-// This is a regression test for
-// https://crrev.com/c/1455266/9/base/message_loop/message_pump_win.cc#125 This
-// is the delayed task equivalent of the above PostImmediateTaskFromSystemPump
-// test.
-// TODO(alexclarke): This test never ends up calling
-// MessagePumpForUI::ScheduleDelayedWork().
-TEST_F(MessageLoopTest, DISABLED_PostDelayedTaskFromSystemPump) {
-  MessageLoop message_loop(MessageLoop::TYPE_UI);
-
-  RunLoop run_loop;
-
-  // A native message window to generate a system message which invokes
-  // DelayedQuitOnSystemTimer() when the native timer fires.
-  win::MessageWindow local_message_window;
-  local_message_window.Create(BindRepeating(&DelayedQuitOnSystemTimer));
-  ASSERT_TRUE(::SetTimer(local_message_window.hwnd(), 0, 20, nullptr));
-
-  // The first task will enter a native message loop. This test then verifies
-  // that the pump is able to run a delayed application task after the native
-  // pump went idle.
-  message_loop.task_runner()->PostTask(
-      FROM_HERE, BindOnce(&SubPumpFunc, run_loop.QuitClosure()));
-
-  // Test success is determined by not hanging in this Run() call.
-  run_loop.Run();
-}
-
 TEST_F(MessageLoopTest, WmQuitIsVisibleToSubPump) {
   MessageLoop message_loop(MessageLoop::TYPE_UI);
 

base/message_loop/message_pump.h

@@ -167,13 +167,10 @@ class BASE_EXPORT MessagePump {
   virtual void ScheduleWork() = 0;
 
   // Schedule a DoDelayedWork callback to happen at the specified time,
-  // cancelling any pending DoDelayedWork callback. This method may only be used
-  // on the thread that called Run.
-  //
-  // This is mostly a no-op in the DoSomeWork() world but must still be invoked
-  // when the new |delayed_work_time| is sooner than the last one returned from
-  // DoSomeWork(). TODO(gab): Clarify this API once all pumps have been
-  // migrated.
+  // cancelling any pending DoDelayedWork callback.  This method may only be
+  // used on the thread that called Run.
+  // TODO(gab): This method is obsolete in the DoSomeWork() variant, remove it
+  // once the migration is complete.
   virtual void ScheduleDelayedWork(const TimeTicks& delayed_work_time) = 0;
 
   // Sets the timer slack to the specified value.

base/message_loop/message_pump_unittest.cc

@@ -30,28 +30,20 @@ bool PumpTypeUsesDoSomeWork(MessageLoopBase::Type type) {
 #else
       return true;
 #endif
-
     case MessageLoopBase::Type::TYPE_UI:
 #if defined(OS_IOS)
       // iOS uses a MessagePumpDefault for UI in unit tests, ref.
       // test_support_ios.mm::CreateMessagePumpForUIForTests().
       return true;
-#elif defined(OS_WIN)
-      return true;
 #else
       // TODO(gab): Complete migration of all UI pumps to DoSomeWork() as part
       // of crbug.com/885371.
       return false;
 #endif
-
     case MessageLoopBase::Type::TYPE_IO:
-#if defined(OS_WIN)
-      return true;
-#else
       // TODO(gab): Complete migration of all IO pumps to DoSomeWork() as part
       // of crbug.com/885371.
       return false;
-#endif
 
     case MessageLoopBase::Type::TYPE_CUSTOM:
 #if defined(OS_ANDROID)

base/message_loop/message_pump_win.h

@@ -14,7 +14,6 @@
 #include "base/base_export.h"
 #include "base/message_loop/message_pump.h"
 #include "base/observer_list.h"
-#include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "base/win/message_window.h"
 #include "base/win/scoped_handle.h"
@@ -45,6 +44,10 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
   };
 
   virtual void DoRunLoop() = 0;
+  int GetCurrentDelay() const;
+
+  // The time at which delayed work should run.
+  TimeTicks delayed_work_time_;
 
   // True iff:
   //   * MessagePumpForUI: there's a kMsgDoWork message pending in the Windows
@@ -71,8 +74,6 @@ class BASE_EXPORT MessagePumpWin : public MessagePump {
 
   // State for the current invocation of Run.
   RunState* state_ = nullptr;
-
-  THREAD_CHECKER(bound_thread_);
 };
 
 //-----------------------------------------------------------------------------
@@ -150,11 +151,10 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
   bool MessageCallback(
       UINT message, WPARAM wparam, LPARAM lparam, LRESULT* result);
   void DoRunLoop() override;
-  void WaitForWork(Delegate::NextWorkInfo next_work_info);
+  void WaitForWork();
   void HandleWorkMessage();
   void HandleTimerMessage();
-  void ScheduleNativeTimer(Delegate::NextWorkInfo next_work_info);
-  void KillNativeTimer();
+  void RescheduleTimer();
   bool ProcessNextWindowsMessage();
   bool ProcessMessageHelper(const MSG& msg);
   bool ProcessPumpReplacementMessage();
@@ -165,14 +165,6 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
   // TODO(thestig): Remove when the Cloud Print Service goes away.
   bool enable_wm_quit_ = false;
 
-  // True if inside a nested native loop and a native timer was installed.
-  bool installed_native_timer_ = false;
-
-  // This will become true when a native loop takes our kMsgHaveWork out of the
-  // system queue. It will be reset to false whenever DoRunLoop regains control.
-  // Used to decide whether ScheduleDelayedWork() should start a native timer.
-  bool in_native_loop_ = false;
-
   ObserverList<Observer>::Unchecked observers_;
 };
 
@@ -274,7 +266,7 @@ class BASE_EXPORT MessagePumpForIO : public MessagePumpWin {
   };
 
   void DoRunLoop() override;
-  void WaitForWork(Delegate::NextWorkInfo next_work_info);
+  void WaitForWork();
   bool MatchCompletedIOItem(IOHandler* filter, IOItem* item);
   bool GetIOItem(DWORD timeout, IOItem* item);
   bool ProcessInternalIOItem(const IOItem& item);