Fix logic on high Windows resolution timer and have

two possible period values for timeBeginPeriod and timeEndPeriod.

Currently while on battery we disable calls to timeBeginPeriod
which make the windows timers have 15ms resolution.

This change makes it so when EnableHighResolutionTimer(true) which
is on AC power the timer is 1ms and EnableHighResolutionTimer(false)
is 4ms.

This should provide significant power savings while meeting some
timer resolution requirements needed by the GPU compositor.

But also this CL fixes the following:

EnableHighResolutionTimer() and ActivateHighResolutionTimer() are
pretty broken. This CL fixes most issues:

1- The existing logic fails to account that EnableHighResolutionTimer
can be called while the browser is running

2- All related functions need to be thread safe.

3- ActivateHighResolutionTimer was buggy.

BUG=153139

Review URL: https://codereview.chromium.org/489793003

Cr-Commit-Position: refs/heads/master@{#292094}

base/time/time.h

@@ -340,13 +340,7 @@ class BASE_EXPORT Time {
   // treat it as static across all windows versions.
   static const int kMinLowResolutionThresholdMs = 16;
 
-  // Enable or disable Windows high resolution timer. If the high resolution
-  // timer is not enabled, calls to ActivateHighResolutionTimer will fail.
-  // When disabling the high resolution timer, this function will not cause
-  // the high resolution timer to be deactivated, but will prevent future
-  // activations.
-  // Must be called from the main thread.
-  // For more details see comments in time_win.cc.
+  // Enable or disable Windows high resolution timer.
   static void EnableHighResolutionTimer(bool enable);
 
   // Activates or deactivates the high resolution timer based on the |activate|
@@ -493,16 +487,6 @@ class BASE_EXPORT Time {
   // platform-dependent epoch.
   static const int64 kTimeTToMicrosecondsOffset;
 
-#if defined(OS_WIN)
-  // Indicates whether fast timers are usable right now.  For instance,
-  // when using battery power, we might elect to prevent high speed timers
-  // which would draw more power.
-  static bool high_resolution_timer_enabled_;
-  // Count of activations on the high resolution timer.  Only use in tests
-  // which are single threaded.
-  static int high_resolution_timer_activated_;
-#endif
-
   // Time in microseconds in UTC.
   int64 us_;
 };

base/time/time_win.cc

@@ -29,10 +29,7 @@
 //
 // To work around all this, we're going to generally use timeGetTime().  We
 // will only increase the system-wide timer if we're not running on battery
-// power.  Using timeBeginPeriod(1) is a requirement in order to make our
-// message loop waits have the same resolution that our time measurements
-// do.  Otherwise, WaitForSingleObject(..., 1) will no less than 15ms when
-// there is nothing else to waken the Wait.
+// power.
 
 #include "base/time/time.h"
 
@@ -87,6 +84,19 @@ void InitializeClock() {
   initial_time = CurrentWallclockMicroseconds();
 }
 
+// The two values that ActivateHighResolutionTimer uses to set the systemwide
+// timer interrupt frequency on Windows. It controls how precise timers are
+// but also has a big impact on battery life.
+const int kMinTimerIntervalHighResMs = 1;
+const int kMinTimerIntervalLowResMs = 4;
+// Track if kMinTimerIntervalHighResMs or kMinTimerIntervalLowResMs is active.
+bool g_high_res_timer_enabled = false;
+// How many times the high resolution timer has been called.
+int g_high_res_timer_count = 0;
+// The lock to control access to the above two variables.
+base::LazyInstance<base::Lock>::Leaky g_high_res_lock =
+    LAZY_INSTANCE_INITIALIZER;
+
 }  // namespace
 
 // Time -----------------------------------------------------------------------
@@ -98,9 +108,6 @@ void InitializeClock() {
 // static
 const int64 Time::kTimeTToMicrosecondsOffset = GG_INT64_C(11644473600000000);
 
-bool Time::high_resolution_timer_enabled_ = false;
-int Time::high_resolution_timer_activated_ = 0;
-
 // static
 Time Time::Now() {
   if (initial_time == 0)
@@ -165,44 +172,51 @@ FILETIME Time::ToFileTime() const {
 
 // static
 void Time::EnableHighResolutionTimer(bool enable) {
-  // Test for single-threaded access.
-  static PlatformThreadId my_thread = PlatformThread::CurrentId();
-  DCHECK(PlatformThread::CurrentId() == my_thread);
-
-  if (high_resolution_timer_enabled_ == enable)
+  base::AutoLock lock(g_high_res_lock.Get());
+  if (g_high_res_timer_enabled == enable)
     return;
-
-  high_resolution_timer_enabled_ = enable;
+  g_high_res_timer_enabled = enable;
+  if (!g_high_res_timer_count)
+    return;
+  // Since g_high_res_timer_count != 0, an ActivateHighResolutionTimer(true)
+  // was called which called timeBeginPeriod with g_high_res_timer_enabled
+  // with a value which is the opposite of |enable|. With that information we
+  // call timeEndPeriod with the same value used in timeBeginPeriod and
+  // therefore undo the period effect.
+  if (enable) {
+    timeEndPeriod(kMinTimerIntervalLowResMs);
+    timeBeginPeriod(kMinTimerIntervalHighResMs);
+  } else {
+    timeEndPeriod(kMinTimerIntervalHighResMs);
+    timeBeginPeriod(kMinTimerIntervalLowResMs);
+  }
 }
 
 // static
 bool Time::ActivateHighResolutionTimer(bool activating) {
-  if (!high_resolution_timer_enabled_ && activating)
-    return false;
+  // We only do work on the transition from zero to one or one to zero so we
+  // can easily undo the effect (if necessary) when EnableHighResolutionTimer is
+  // called.
+  base::AutoLock lock(g_high_res_lock.Get());
+  UINT period = g_high_res_timer_enabled ? kMinTimerIntervalHighResMs
+                                         : kMinTimerIntervalLowResMs;
+  int high_res_count =
+      activating ? ++g_high_res_timer_count : --g_high_res_timer_count;
 
-  // Using anything other than 1ms makes timers granular
-  // to that interval.
-  const int kMinTimerIntervalMs = 1;
-  MMRESULT result;
   if (activating) {
-    result = timeBeginPeriod(kMinTimerIntervalMs);
-    high_resolution_timer_activated_++;
+    if (high_res_count == 1)
+      timeBeginPeriod(period);
   } else {
-    result = timeEndPeriod(kMinTimerIntervalMs);
-    high_resolution_timer_activated_--;
+    if (high_res_count == 0)
+      timeEndPeriod(period);
   }
-  return result == TIMERR_NOERROR;
+  return (period == kMinTimerIntervalHighResMs);
 }
 
 // static
 bool Time::IsHighResolutionTimerInUse() {
-  // Note:  we should track the high_resolution_timer_activated_ value
-  // under a lock if we want it to be accurate in a system with multiple
-  // message loops.  We don't do that - because we don't want to take the
-  // expense of a lock for this.  We *only* track this value so that unit
-  // tests can see if the high resolution timer is on or off.
-  return high_resolution_timer_enabled_ &&
-      high_resolution_timer_activated_ > 0;
+  base::AutoLock lock(g_high_res_lock.Get());
+  return g_high_res_timer_enabled && g_high_res_timer_count > 0;
 }
 
 // static

base/timer/hi_res_timer_manager_unittest.cc

@@ -14,25 +14,22 @@
 namespace base {
 
 #if defined(OS_WIN)
-// http://crbug.com/114048
-TEST(HiResTimerManagerTest, DISABLED_ToggleOnOff) {
-  base::MessageLoop loop;
+TEST(HiResTimerManagerTest, ToggleOnOff) {
+  // The power monitor creates Window to receive power notifications from
+  // Windows, which makes this test flaky if you run while the machine
+  // goes in or out of AC power.
+  base::MessageLoop loop(base::MessageLoop::TYPE_UI);
   scoped_ptr<base::PowerMonitorSource> power_monitor_source(
       new base::PowerMonitorDeviceSource());
   scoped_ptr<base::PowerMonitor> power_monitor(
       new base::PowerMonitor(power_monitor_source.Pass()));
-  HighResolutionTimerManager manager;
 
-  // At this point, we don't know if the high resolution timers are on or off,
-  // it depends on what system the tests are running on (for example, if this
-  // test is running on a laptop/battery, then the PowerMonitor would have
-  // already set the PowerState to battery power; but if we're running on a
-  // desktop, then the PowerState will be non-battery power).  Simulate a power
-  // level change to get to a deterministic state.
-  manager.OnPowerStateChange(/* on_battery */ false);
+  HighResolutionTimerManager manager;
+  // Simulate a on-AC power event to get to a known initial state.
+  manager.OnPowerStateChange(false);
 
   // Loop a few times to test power toggling.
-  for (int loop = 2; loop >= 0; --loop) {
+  for (int times = 0; times != 3; ++times) {
     // The manager has the high resolution clock enabled now.
     EXPECT_TRUE(manager.hi_res_clock_available());
     // But the Time class has it off, because it hasn't been activated.
@@ -43,12 +40,12 @@ TEST(HiResTimerManagerTest, DISABLED_ToggleOnOff) {
     EXPECT_TRUE(base::Time::IsHighResolutionTimerInUse());
 
     // Simulate a on-battery power event.
-    manager.OnPowerStateChange(/* on_battery */ true);
+    manager.OnPowerStateChange(true);
     EXPECT_FALSE(manager.hi_res_clock_available());
     EXPECT_FALSE(base::Time::IsHighResolutionTimerInUse());
 
-    // Simulate a off-battery power event.
-    manager.OnPowerStateChange(/* on_battery */ false);
+    // Back to on-AC power.
+    manager.OnPowerStateChange(false);
     EXPECT_TRUE(manager.hi_res_clock_available());
     EXPECT_TRUE(base::Time::IsHighResolutionTimerInUse());