[MessageLoop] Disable hi-res timers when not sleeping

Time::ActivateHighResolutionTimer(bool activating) is a per-thread vote
for a system-wide side-effect. For a given thread, hi-res timers are
only useful when going to sleep (if it has pending hi-res tasks).

Deactivating a thread's vote while it's active will prevent other
threads on the system which do not have hi-res requirements from
being forced to use hi-res timers in that period.

Bug: 854237
Change-Id: I1393e184cac6c9321d13b92b6077a38c62b1f590
Reviewed-on: https://chromium-review.googlesource.com/1107110Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574259}

base/message_loop/message_loop.cc

@@ -192,7 +192,8 @@ MessageLoop::~MessageLoop() {
 #if defined(OS_WIN)
   if (in_high_res_mode_)
     Time::ActivateHighResolutionTimer(false);
-#endif
+#endif  // defined (OS_WIN)
+
   // Clean up any unprocessed tasks, but take care: deleting a task could
   // result in the addition of more tasks (e.g., via DeleteSoon).  We set a
   // limit on the number of times we will allow a deleted task to generate more
@@ -490,6 +491,18 @@ bool MessageLoop::DoWork() {
   if (!task_execution_allowed_)
     return false;
 
+#if defined(OS_WIN)
+  // Raising timer frequency has a system-wide effect. Ensure this thread's vote
+  // to raise the frequency is only active while it is sleeping (with pending
+  // hi-res tasks) to avoid unnecessarily keeping the entire system in a high
+  // frequency mode while this thread isn't sleeping (even if it has pending
+  // hi-res delayed tasks). Ref. https://crbug.com/854237#c3
+  if (in_high_res_mode_) {
+    in_high_res_mode_ = false;
+    Time::ActivateHighResolutionTimer(false);
+  }
+#endif  // defined (OS_WIN)
+
   // Execute oldest task.
   while (incoming_task_queue_->triage_tasks().HasTasks()) {
     if (!scheduled_wakeup_.next_run_time.is_null()) {
@@ -660,15 +673,16 @@ bool MessageLoop::DoIdleWork() {
     // for some tasks.
     need_high_res_timers =
         incoming_task_queue_->HasPendingHighResolutionTasks();
-#endif
+#endif  // defined (OS_WIN)
   }
 
 #if defined(OS_WIN)
-  if (in_high_res_mode_ != need_high_res_timers) {
-    in_high_res_mode_ = need_high_res_timers;
-    Time::ActivateHighResolutionTimer(in_high_res_mode_);
+  if (need_high_res_timers) {
+    DCHECK(!in_high_res_mode_);
+    in_high_res_mode_ = true;
+    Time::ActivateHighResolutionTimer(true);
   }
-#endif
+#endif  // defined (OS_WIN)
 
   // When we return we will do a kernel wait for more tasks.
   return false;

base/message_loop/message_loop.h

@@ -271,8 +271,8 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate,
   const Type type_;
 
 #if defined(OS_WIN)
-  // Tracks if we have requested high resolution timers. Its only use is to
-  // turn off the high resolution timer upon loop destruction.
+  // Tracks if this MessageLoop has requested system-wide high resolution timers
+  // for its sleep period. Used to downvote that request when the sleep is over.
   bool in_high_res_mode_ = false;
 #endif
 

base/message_loop/message_loop_unittest.cc

@@ -21,8 +21,10 @@
 #include "base/posix/eintr_wrapper.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/atomic_flag.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/task_scheduler/task_scheduler.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/gtest_util.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/test/test_timeouts.h"
@@ -1873,40 +1875,71 @@ TEST_P(MessageLoopTest, WaitForIO) {
 }
 
 TEST_P(MessageLoopTest, HighResolutionTimer) {
+  Thread verification_thread("verification thread");
+  verification_thread.StartAndWaitForTesting();
+
   MessageLoop message_loop;
   Time::EnableHighResolutionTimer(true);
 
-  constexpr TimeDelta kFastTimer = TimeDelta::FromMilliseconds(5);
+  constexpr TimeDelta kFastTimer = TimeDelta::FromMilliseconds(31);
   constexpr TimeDelta kSlowTimer = TimeDelta::FromMilliseconds(100);
 
+  // Since MessageLoop disables its vote to activate high-resolution timers the
+  // instant it's active, the high-resolution timer's activation can only be
+  // tested async. Try to make this as reliable as possible by verifying in the
+  // middle of the MessageLoop's sleep period (giving plenty of time before for
+  // it to establish the high-res timer and plenty of time after for it not to
+  // wakeup while verifying).
+  constexpr TimeDelta kVerificationDelay = kFastTimer / 2;
+
   {
     // Post a fast task to enable the high resolution timers.
     RunLoop run_loop;
     message_loop.task_runner()->PostDelayedTask(
-        FROM_HERE,
-        BindOnce(
-            [](RunLoop* run_loop) {
-              EXPECT_TRUE(Time::IsHighResolutionTimerInUse());
-              run_loop->QuitWhenIdle();
-            },
-            &run_loop),
+        FROM_HERE, BindLambdaForTesting([&]() {
+          // The loop deactivates high resolution timers the instant it's
+          // active.
+          EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
+          run_loop.Quit();
+        }),
         kFastTimer);
+
+    // Ensures the verification runs (that thread could theoretically never be
+    // scheduled -- skipping the test).
+    AtomicFlag verification_ran;
+    verification_thread.task_runner()->PostDelayedTask(
+        FROM_HERE, BindLambdaForTesting([&]() {
+          EXPECT_TRUE(Time::IsHighResolutionTimerInUse());
+          verification_ran.Set();
+        }),
+        kVerificationDelay);
+
     run_loop.Run();
+    ASSERT_TRUE(verification_ran.IsSet());
   }
   EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
   {
     // Check that a slow task does not trigger the high resolution logic.
     RunLoop run_loop;
     message_loop.task_runner()->PostDelayedTask(
-        FROM_HERE,
-        BindOnce(
-            [](RunLoop* run_loop) {
-              EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
-              run_loop->QuitWhenIdle();
-            },
-            &run_loop),
+        FROM_HERE, BindLambdaForTesting([&]() {
+          // The loop deactivates high resolution timers the instant it's
+          // active.
+          EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
+          run_loop.Quit();
+        }),
         kSlowTimer);
+
+    AtomicFlag verification_ran;
+    verification_thread.task_runner()->PostDelayedTask(
+        FROM_HERE, BindLambdaForTesting([&]() {
+          EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
+          verification_ran.Set();
+        }),
+        kVerificationDelay);
+
     run_loop.Run();
+    ASSERT_TRUE(verification_ran.IsSet());
   }
   Time::EnableHighResolutionTimer(false);
   Time::ResetHighResolutionTimerUsage();