Reland old fix that was reverted incorrectly.

Fix regression where high resolution timers could be activated even under
battery power. Add unit test to protect chromium from developers like me
in the future.

The fix is a one-liner in hi_res_timer_manager_win.cc. The rest of the code
change is the mechanics to enable the unit test.

BUG=59528
TEST=HiResTimerManagerTest.ToggleOnOff
Review URL: http://codereview.chromium.org/6904117

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85413 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 105 insertions, 12 deletions

diff --git a/base/message_loop.cc b/base/message_loop.cc
index a3520a6..059cdc25 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -15,6 +15,7 @@
 #include "base/scoped_ptr.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
 #include "base/threading/thread_local.h"
+#include "base/time.h"
 #include "base/tracked_objects.h"
 
 #if defined(OS_MACOSX)
@@ -226,6 +227,16 @@ MessageLoop::~MessageLoop() {
 
   // OK, now make it so that no one can find us.
   lazy_tls_ptr.Pointer()->Set(NULL);
+
+#if defined(OS_WIN)
+  // If we left the high-resolution timer activated, deactivate it now.
+  // Doing this is not-critical, it is mainly to make sure we track
+  // the high resolution timer activations properly in our unit tests.
+  if (!high_resolution_timer_expiration_.is_null()) {
+    base::Time::ActivateHighResolutionTimer(false);
+    high_resolution_timer_expiration_ = base::TimeTicks();
+  }
+#endif
 }
 
 // static
@@ -576,9 +587,10 @@ TimeTicks MessageLoop::CalculateDelayedRuntime(int64 delay_ms) {
       bool needs_high_res_timers =
           delay_ms < (2 * base::Time::kMinLowResolutionThresholdMs);
       if (needs_high_res_timers) {
-        base::Time::ActivateHighResolutionTimer(true);
-        high_resolution_timer_expiration_ = TimeTicks::Now() +
-            TimeDelta::FromMilliseconds(kHighResolutionTimerModeLeaseTimeMs);
+        if (base::Time::ActivateHighResolutionTimer(true)) {
+          high_resolution_timer_expiration_ = TimeTicks::Now() +
+              TimeDelta::FromMilliseconds(kHighResolutionTimerModeLeaseTimeMs);
+        }
       }
     }
 #endif
diff --git a/base/time.h b/base/time.h
index 58b0eb2..1b05e24 100644
--- a/base/time.h
+++ b/base/time.h
@@ -25,6 +25,7 @@
 
 #include <time.h>
 
+#include "base/atomicops.h"
 #include "base/base_api.h"
 #include "base/basictypes.h"
 
@@ -281,10 +282,16 @@ class BASE_API Time {
   // Activates or deactivates the high resolution timer based on the |activate|
   // flag.  If the HighResolutionTimer is not Enabled (see
   // EnableHighResolutionTimer), this function will return false.  Otherwise
-  // returns true.
+  // returns true.  Each successful activate call must be paired with a
+  // subsequent deactivate call.
   // All callers to activate the high resolution timer must eventually call
   // this function to deactivate the high resolution timer.
   static bool ActivateHighResolutionTimer(bool activate);
+
+  // Returns true if the high resolution timer is both enabled and activated.
+  // This is provided for testing only, and is not tracked in a thread-safe
+  // way.
+  static bool IsHighResolutionTimerInUse();
 #endif
 
   // Converts an exploded structure representing either the local time or UTC
@@ -403,6 +410,9 @@ class BASE_API Time {
   // 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.
diff --git a/base/time_win.cc b/base/time_win.cc
index 7124a74..d439a1e 100644
--- a/base/time_win.cc
+++ b/base/time_win.cc
@@ -99,6 +99,7 @@ void InitializeClock() {
 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() {
@@ -162,22 +163,36 @@ void Time::EnableHighResolutionTimer(bool enable) {
 }
 
 // static
-bool Time::ActivateHighResolutionTimer(bool activate) {
-  if (!high_resolution_timer_enabled_)
+bool Time::ActivateHighResolutionTimer(bool activating) {
+  if (!high_resolution_timer_enabled_ && activating)
     return false;
 
   // Using anything other than 1ms makes timers granular
   // to that interval.
   const int kMinTimerIntervalMs = 1;
   MMRESULT result;
-  if (activate)
+  if (activating) {
     result = timeBeginPeriod(kMinTimerIntervalMs);
-  else
+    high_resolution_timer_activated_++;
+  } else {
     result = timeEndPeriod(kMinTimerIntervalMs);
+    high_resolution_timer_activated_--;
+  }
   return result == TIMERR_NOERROR;
 }
 
 // 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;
+}
+
+// static
 Time Time::FromExploded(bool is_local, const Exploded& exploded) {
   // Create the system struct representing our exploded time. It will either be
   // in local time or UTC.
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 097e53d..44ad45c 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1911,6 +1911,7 @@
         '../content/common/font_descriptor_mac_unittest.mm',
         '../content/common/gpu/gpu_feature_flags_unittest.cc',
         '../content/common/gpu/gpu_info_unittest.cc',
+        '../content/common/hi_res_timer_manager_unittest.cc',
         '../content/common/notification_service_unittest.cc',
         '../content/common/process_watcher_unittest.cc',
         '../content/common/property_bag_unittest.cc',
diff --git a/content/common/hi_res_timer_manager.h b/content/common/hi_res_timer_manager.h
index 0fa64f9..af522cd 100644
--- a/content/common/hi_res_timer_manager.h
+++ b/content/common/hi_res_timer_manager.h
@@ -18,11 +18,14 @@ class HighResolutionTimerManager : public ui::SystemMonitor::PowerObserver {
   // ui::SystemMonitor::PowerObserver:
   virtual void OnPowerStateChange(bool on_battery_power);
 
+  // Returns true if the hi resolution clock could be used right now.
+  bool hi_res_clock_available() const { return hi_res_clock_available_; }
+
  private:
   // Enable or disable the faster multimedia timer.
   void UseHiResClock(bool use);
 
-  bool hi_res_clock_used_;
+  bool hi_res_clock_available_;
 
   DISALLOW_COPY_AND_ASSIGN(HighResolutionTimerManager);
 };
diff --git a/content/common/hi_res_timer_manager_posix.cc b/content/common/hi_res_timer_manager_posix.cc
index 63e7300..430902a 100644
--- a/content/common/hi_res_timer_manager_posix.cc
+++ b/content/common/hi_res_timer_manager_posix.cc
@@ -7,7 +7,7 @@
 // On POSIX we don't need to do anything special with the system timer.
 
 HighResolutionTimerManager::HighResolutionTimerManager()
-    : hi_res_clock_used_(false) {
+    : hi_res_clock_available_(false) {
 }
 
 HighResolutionTimerManager::~HighResolutionTimerManager() {
diff --git a/content/common/hi_res_timer_manager_unittest.cc b/content/common/hi_res_timer_manager_unittest.cc
new file mode 100644
index 0000000..c1d3ed0
--- /dev/null
+++ b/content/common/hi_res_timer_manager_unittest.cc
@@ -0,0 +1,51 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include "base/memory/scoped_ptr.h"
+#include "base/time.h"
+#include "content/common/hi_res_timer_manager.h"
+#include "ui/base/system_monitor/system_monitor.h"
+
+#if defined(OS_WIN)
+TEST(HiResTimerManagerTest, ToggleOnOff) {
+  MessageLoop loop;
+  scoped_ptr<ui::SystemMonitor> system_monitor(new ui::SystemMonitor());
+  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 SystemMonitor 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);
+
+  // Loop a few times to test power toggling.
+  for (int loop = 2; loop >= 0; --loop) {
+    // 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.
+    EXPECT_FALSE(base::Time::IsHighResolutionTimerInUse());
+
+    // Activate the high resolution timer.
+    base::Time::ActivateHighResolutionTimer(true);
+    EXPECT_TRUE(base::Time::IsHighResolutionTimerInUse());
+
+    // Simulate a on-battery power event.
+    manager.OnPowerStateChange(/* on_battery */ true);
+    EXPECT_FALSE(manager.hi_res_clock_available());
+    EXPECT_FALSE(base::Time::IsHighResolutionTimerInUse());
+
+    // Simulate a off-battery power event.
+    manager.OnPowerStateChange(/* on_battery */ false);
+    EXPECT_TRUE(manager.hi_res_clock_available());
+    EXPECT_TRUE(base::Time::IsHighResolutionTimerInUse());
+
+    // De-activate the high resolution timer.
+    base::Time::ActivateHighResolutionTimer(false);
+  }
+}
+#endif  // defined(OS_WIN)
diff --git a/content/common/hi_res_timer_manager_win.cc b/content/common/hi_res_timer_manager_win.cc
index 9c86fa0..d41c4bd 100644
--- a/content/common/hi_res_timer_manager_win.cc
+++ b/content/common/hi_res_timer_manager_win.cc
@@ -7,7 +7,7 @@
 #include "base/time.h"
 
 HighResolutionTimerManager::HighResolutionTimerManager()
-    : hi_res_clock_used_(false) {
+    : hi_res_clock_available_(false) {
   ui::SystemMonitor* system_monitor = ui::SystemMonitor::Get();
   system_monitor->AddObserver(this);
   UseHiResClock(!system_monitor->BatteryPower());
@@ -23,7 +23,8 @@ void HighResolutionTimerManager::OnPowerStateChange(bool on_battery_power) {
 }
 
 void HighResolutionTimerManager::UseHiResClock(bool use) {
-  if (use == hi_res_clock_used_)
+  if (use == hi_res_clock_available_)
     return;
+  hi_res_clock_available_ = use;
   base::Time::EnableHighResolutionTimer(use);
 }