Fix logic on high Windows resolution timer and have

base/time/time_win.cc

Index: base/time/time_win.cc
diff --git a/base/time/time_win.cc b/base/time/time_win.cc
index 5fa899d1f97f69be69d1df65ba854565d7240b53..ff1b119ea38a0d3a29a104c56279134f35116573 100644
--- a/base/time/time_win.cc
+++ b/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,18 @@ 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> g_high_res_lock = LAZY_INSTANCE_INITIALIZER;

[Nico, 2014/08/26 20:15:58]

Can you make this ::Leaky?

+
 }  // namespace
 
 // Time -----------------------------------------------------------------------
@@ -98,9 +107,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 +171,46 @@ 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;
+  if (enable) {

[jamesr, 2014/08/26 19:47:40]

this bit might be worth a comment explaining the state transitions. rough
attempt:

Since g_high_res_timer_count is nonzero, an earlier call to
ActivateHighResolutionTimer(true) must have called timeBeginPeriod(). If we were
in high resolution mode beforehand, it would have called
timeBeginPeriod(kMinTimerIntervalHighResMs) otherwise it called
timeBeginPeriod(kMinTimerIntervalLowResMs). We need to end that period and start
a new one with the new desired resolution. A future call to
ActivateHighResolutionTimer(false) will end this period.

+    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