Revert of High resolution timer fix for Windows (https://codereview.chromium.org/395913006/)

Reason for revert:
This patch seems to make following browser_tests flakey

PPAPINaClNewlibTest.Graphics2D_FlushOffscreenUpdate
NetInternalsTest.netInternalsHSTSViewAddOverwrite
NetInternalsTest.netInternalsHSTSViewAddDelete
NetInternalsTest.netInternalsHSTSViewAddTwice

http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%282%29/builds/34734
http://build.chromium.org/p/chromium.win/builders/XP%20Tests%20%281%29/builds/32086
http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%281%29/builds/47545


Original issue's description:
> This is jamesr@ code I am landing.
> 
> On Windows the message pump code tried to manage the systemwide timer resolution to fire delayed tasks with better than 15ms resolution but it was buggy:
> 
> 1- A short task that was not followed by any other task will leave the systemwide timer pegged to 1ms
> 
> 2- After we decided to crank up the timer we would 'lease' the timer for 1 second, for no good reason.
> 
> Both issues are detrimental to battery power.
> 
> The source of both problems is that we tried to decide with incomplete information. This patch solves that by having 1 bit for each pending task that requires a high resolution timer and a sum of the number of tasks that require high res timers.
> 
> BUG=153139
> TEST=included here, also see the bug for manual testing.
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284625

TBR=jamesr@chromium.org,darin@chromium.org,cpu@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=153139

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

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

base/message_loop/incoming_task_queue.cc

@@ -8,14 +8,12 @@
 #include "base/location.h"
 #include "base/message_loop/message_loop.h"
 #include "base/synchronization/waitable_event.h"
-#include "base/time/time.h"
 
 namespace base {
 namespace internal {
 
 IncomingTaskQueue::IncomingTaskQueue(MessageLoop* message_loop)
-    : high_res_task_count_(0),
-      message_loop_(message_loop),
+    : message_loop_(message_loop),
       next_sequence_num_(0) {
 }
 
@@ -27,23 +25,15 @@ bool IncomingTaskQueue::AddToIncomingQueue(
   AutoLock locked(incoming_queue_lock_);
   PendingTask pending_task(
       from_here, task, CalculateDelayedRuntime(delay), nestable);
-#if defined(OS_WIN)
-  // We consider the task needs a high resolution timer if the delay is
-  // more than 0 and less than 32ms. This caps the relative error to
-  // less than 50% : a 33ms wait can wake at 48ms since the default
-  // resolution on Windows is between 10 and 15ms.
-  if (delay > TimeDelta() &&
-      delay.InMilliseconds() < (2 * Time::kMinLowResolutionThresholdMs)) {
-    ++high_res_task_count_;
-    pending_task.is_high_res = true;
-  }
-#endif
   return PostPendingTask(&pending_task);
 }
 
-bool IncomingTaskQueue::HasHighResolutionTasks() {
-  AutoLock lock(incoming_queue_lock_);
-  return high_res_task_count_ > 0;
+bool IncomingTaskQueue::IsHighResolutionTimerEnabledForTesting() {
+#if defined(OS_WIN)
+  return !high_resolution_timer_expiration_.is_null();
+#else
+  return true;
+#endif
 }
 
 bool IncomingTaskQueue::IsIdleForTesting() {
@@ -51,22 +41,29 @@ bool IncomingTaskQueue::IsIdleForTesting() {
   return incoming_queue_.empty();
 }
 
-int IncomingTaskQueue::ReloadWorkQueue(TaskQueue* work_queue) {
+void IncomingTaskQueue::ReloadWorkQueue(TaskQueue* work_queue) {
   // Make sure no tasks are lost.
   DCHECK(work_queue->empty());
 
   // Acquire all we can from the inter-thread queue with one lock acquisition.
   AutoLock lock(incoming_queue_lock_);
   if (!incoming_queue_.empty())
-    incoming_queue_.Swap(work_queue);
+    incoming_queue_.Swap(work_queue);  // Constant time
 
-  // Reset the count of high resolution tasks since our queue is now empty.
-  int high_res_tasks = high_res_task_count_;
-  high_res_task_count_ = 0;
-  return high_res_tasks;
+  DCHECK(incoming_queue_.empty());
 }
 
 void IncomingTaskQueue::WillDestroyCurrentMessageLoop() {
+#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()) {
+    Time::ActivateHighResolutionTimer(false);
+    high_resolution_timer_expiration_ = TimeTicks();
+  }
+#endif
+
   AutoLock lock(incoming_queue_lock_);
   message_loop_ = NULL;
 }
@@ -78,10 +75,40 @@ IncomingTaskQueue::~IncomingTaskQueue() {
 
 TimeTicks IncomingTaskQueue::CalculateDelayedRuntime(TimeDelta delay) {
   TimeTicks delayed_run_time;
-  if (delay > TimeDelta())
+  if (delay > TimeDelta()) {
     delayed_run_time = TimeTicks::Now() + delay;
-  else
+
+#if defined(OS_WIN)
+    if (high_resolution_timer_expiration_.is_null()) {
+      // Windows timers are granular to 15.6ms.  If we only set high-res
+      // timers for those under 15.6ms, then a 18ms timer ticks at ~32ms,
+      // which as a percentage is pretty inaccurate.  So enable high
+      // res timers for any timer which is within 2x of the granularity.
+      // This is a tradeoff between accuracy and power management.
+      bool needs_high_res_timers = delay.InMilliseconds() <
+          (2 * Time::kMinLowResolutionThresholdMs);
+      if (needs_high_res_timers) {
+        if (Time::ActivateHighResolutionTimer(true)) {
+          high_resolution_timer_expiration_ = TimeTicks::Now() +
+              TimeDelta::FromMilliseconds(
+                  MessageLoop::kHighResolutionTimerModeLeaseTimeMs);
+        }
+      }
+    }
+#endif
+  } else {
     DCHECK_EQ(delay.InMilliseconds(), 0) << "delay should not be negative";
+  }
+
+#if defined(OS_WIN)
+  if (!high_resolution_timer_expiration_.is_null()) {
+    if (TimeTicks::Now() > high_resolution_timer_expiration_) {
+      Time::ActivateHighResolutionTimer(false);
+      high_resolution_timer_expiration_ = TimeTicks();
+    }
+  }
+#endif
+
   return delayed_run_time;
 }
 

base/message_loop/incoming_task_queue.h

@@ -38,17 +38,16 @@ class BASE_EXPORT IncomingTaskQueue
                           TimeDelta delay,
                           bool nestable);
 
-  // Returns true if the queue contains tasks that require higher than default
-  // timer resolution. Currently only needed for Windows.
-  bool HasHighResolutionTasks();
+  // Returns true if the message loop has high resolution timers enabled.
+  // Provided for testing.
+  bool IsHighResolutionTimerEnabledForTesting();
 
   // Returns true if the message loop is "idle". Provided for testing.
   bool IsIdleForTesting();
 
   // Loads tasks from the |incoming_queue_| into |*work_queue|. Must be called
-  // from the thread that is running the loop. Returns the number of tasks that
-  // require high resolution timers.
-  int ReloadWorkQueue(TaskQueue* work_queue);
+  // from the thread that is running the loop.
+  void ReloadWorkQueue(TaskQueue* work_queue);
 
   // Disconnects |this| from the parent message loop.
   void WillDestroyCurrentMessageLoop();
@@ -66,11 +65,12 @@ class BASE_EXPORT IncomingTaskQueue
   // does not retain |pending_task->task| beyond this function call.
   bool PostPendingTask(PendingTask* pending_task);
 
-  // Number of tasks that require high resolution timing. This value is kept
-  // so that ReloadWorkQueue() completes in constant time.
-  int high_res_task_count_;
+#if defined(OS_WIN)
+  TimeTicks high_resolution_timer_expiration_;
+#endif
 
-  // The lock that protects access to the members of this class.
+  // The lock that protects access to |incoming_queue_|, |message_loop_| and
+  // |next_sequence_num_|.
   base::Lock incoming_queue_lock_;
 
   // An incoming queue of tasks that are acquired under a mutex for processing

base/message_loop/message_loop.cc

@@ -127,8 +127,6 @@ MessageLoop::DestructionObserver::~DestructionObserver() {
 
 MessageLoop::MessageLoop(Type type)
     : type_(type),
-      pending_high_res_tasks_(0),
-      in_high_res_mode_(false),
       nestable_tasks_allowed_(true),
 #if defined(OS_WIN)
       os_modal_loop_(false),
@@ -143,8 +141,6 @@ MessageLoop::MessageLoop(Type type)
 MessageLoop::MessageLoop(scoped_ptr<MessagePump> pump)
     : pump_(pump.Pass()),
       type_(TYPE_CUSTOM),
-      pending_high_res_tasks_(0),
-      in_high_res_mode_(false),
       nestable_tasks_allowed_(true),
 #if defined(OS_WIN)
       os_modal_loop_(false),
@@ -157,11 +153,8 @@ MessageLoop::MessageLoop(scoped_ptr<MessagePump> pump)
 
 MessageLoop::~MessageLoop() {
   DCHECK_EQ(this, current());
+
   DCHECK(!run_loop_);
-#if defined(OS_WIN)
-  if (in_high_res_mode_)
-    Time::ActivateHighResolutionTimer(false);
-#endif
 
   // 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
@@ -376,8 +369,8 @@ bool MessageLoop::is_running() const {
   return run_loop_ != NULL;
 }
 
-bool MessageLoop::HasHighResolutionTasks() {
-  return incoming_task_queue_->HasHighResolutionTasks();
+bool MessageLoop::IsHighResolutionTimerEnabledForTesting() {
+  return incoming_task_queue_->IsHighResolutionTimerEnabledForTesting();
 }
 
 bool MessageLoop::IsIdleForTesting() {
@@ -445,11 +438,6 @@ void MessageLoop::RunTask(const PendingTask& pending_task) {
       "src_file", pending_task.posted_from.file_name(),
       "src_func", pending_task.posted_from.function_name());
 
-  if (pending_task.is_high_res) {
-    pending_high_res_tasks_--;
-    CHECK(pending_high_res_tasks_ >= 0);
-  }
-
   DCHECK(nestable_tasks_allowed_);
   // Execute the task and assume the worst: It is probably not reentrant.
   nestable_tasks_allowed_ = false;
@@ -535,10 +523,8 @@ void MessageLoop::ReloadWorkQueue() {
   // |*work_queue| by waiting until the last minute (|*work_queue| is empty) to
   // load. That reduces the number of locks-per-task significantly when our
   // queues get large.
-  if (work_queue_.empty()) {
-    pending_high_res_tasks_ +=
-        incoming_task_queue_->ReloadWorkQueue(&work_queue_);
-  }
+  if (work_queue_.empty())
+    incoming_task_queue_->ReloadWorkQueue(&work_queue_);
 }
 
 void MessageLoop::ScheduleWork(bool was_empty) {
@@ -643,14 +629,6 @@ bool MessageLoop::DoIdleWork() {
   if (run_loop_->quit_when_idle_received_)
     pump_->Quit();
 
-  // We will now do a kernel wait for more tasks.
-#if defined(OS_WIN)
-  // The Windows scheduler has by default ~15ms resolution. If we have high
-  // resolution tasks pending we need to temporarity increase the systemwide
-  // timer resolution to 1ms, and if we don't we need to go back to 15ms.
-  in_high_res_mode_ = pending_high_res_tasks_ > 0;
-  Time::ActivateHighResolutionTimer(in_high_res_mode_);
-#endif
   return false;
 }
 

base/message_loop/message_loop.h

@@ -371,6 +371,10 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
   void AddTaskObserver(TaskObserver* task_observer);
   void RemoveTaskObserver(TaskObserver* task_observer);
 
+  // When we go into high resolution timer mode, we will stay in hi-res mode
+  // for at least 1s.
+  static const int kHighResolutionTimerModeLeaseTimeMs = 1000;
+
 #if defined(OS_WIN)
   void set_os_modal_loop(bool os_modal_loop) {
     os_modal_loop_ = os_modal_loop;
@@ -386,7 +390,7 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
 
   // Returns true if the message loop has high resolution timers enabled.
   // Provided for testing.
-  bool HasHighResolutionTasks();
+  bool IsHighResolutionTimerEnabledForTesting();
 
   // Returns true if the message loop is "idle". Provided for testing.
   bool IsIdleForTesting();
@@ -456,14 +460,6 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
   // this queue is only accessed (push/pop) by our current thread.
   TaskQueue work_queue_;
 
-  // How many high resolution tasks are in the pending task queue. This value
-  // increases by N every time we call ReloadWorkQueue() and decreases by 1
-  // every time we call RunTask() if the task needs a high resolution timer.
-  int pending_high_res_tasks_;
-  // Tracks if we have requested high resolution timers. Its only use is to
-  // turn off the high resolution timer upon loop destruction.
-  bool in_high_res_mode_;
-
   // Contains delayed tasks, sorted by their 'delayed_run_time' property.
   DelayedTaskQueue delayed_work_queue_;
 

base/message_loop/message_loop_unittest.cc

@@ -730,20 +730,30 @@ TEST(MessageLoopTest, HighResolutionTimer) {
   const TimeDelta kFastTimer = TimeDelta::FromMilliseconds(5);
   const TimeDelta kSlowTimer = TimeDelta::FromMilliseconds(100);
 
-  EXPECT_FALSE(loop.HasHighResolutionTasks());
+  EXPECT_FALSE(loop.IsHighResolutionTimerEnabledForTesting());
+
   // Post a fast task to enable the high resolution timers.
   loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
                        kFastTimer);
-  EXPECT_TRUE(loop.HasHighResolutionTasks());
   loop.Run();
-  EXPECT_FALSE(loop.HasHighResolutionTasks());
-  EXPECT_FALSE(Time::IsHighResolutionTimerInUse());
-  // Check that a slow task does not trigger the high resolution logic.
+  EXPECT_TRUE(loop.IsHighResolutionTimerEnabledForTesting());
+
+  // Post a slow task and verify high resolution timers
+  // are still enabled.
+  loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
+                       kSlowTimer);
+  loop.Run();
+  EXPECT_TRUE(loop.IsHighResolutionTimerEnabledForTesting());
+
+  // Wait for a while so that high-resolution mode elapses.
+  PlatformThread::Sleep(TimeDelta::FromMilliseconds(
+      MessageLoop::kHighResolutionTimerModeLeaseTimeMs));
+
+  // Post a slow task to disable the high resolution timers.
   loop.PostDelayedTask(FROM_HERE, Bind(&PostNTasksThenQuit, 1),
                        kSlowTimer);
-  EXPECT_FALSE(loop.HasHighResolutionTasks());
   loop.Run();
-  EXPECT_FALSE(loop.HasHighResolutionTasks());
+  EXPECT_FALSE(loop.IsHighResolutionTimerEnabledForTesting());
 }
 
 #endif  // defined(OS_WIN)

base/message_loop/message_pump.h

@@ -39,8 +39,7 @@ class BASE_EXPORT MessagePump : public NonThreadSafe {
     virtual bool DoDelayedWork(TimeTicks* next_delayed_work_time) = 0;
 
     // Called from within Run just before the message pump goes to sleep.
-    // Returns true to indicate that idle work was done. Returning false means
-    // the pump will now wait.
+    // Returns true to indicate that idle work was done.
     virtual bool DoIdleWork() = 0;
   };
 

base/pending_task.cc

@@ -8,14 +8,19 @@
 
 namespace base {
 
+#if _MSC_VER >= 1700
+// This a temporary fix for compiling on VS2012. http://crbug.com/154744
+PendingTask::PendingTask() : sequence_num(-1), nestable(false) {
+}
+#endif
+
 PendingTask::PendingTask(const tracked_objects::Location& posted_from,
                          const base::Closure& task)
     : base::TrackingInfo(posted_from, TimeTicks()),
       task(task),
       posted_from(posted_from),
       sequence_num(0),
-      nestable(true),
-      is_high_res(false) {
+      nestable(true) {
 }
 
 PendingTask::PendingTask(const tracked_objects::Location& posted_from,
@@ -26,8 +31,7 @@ PendingTask::PendingTask(const tracked_objects::Location& posted_from,
       task(task),
       posted_from(posted_from),
       sequence_num(0),
-      nestable(nestable),
-      is_high_res(false) {
+      nestable(nestable) {
 }
 
 PendingTask::~PendingTask() {

base/pending_task.h

@@ -18,6 +18,9 @@ namespace base {
 // Contains data about a pending task. Stored in TaskQueue and DelayedTaskQueue
 // for use by classes that queue and execute tasks.
 struct BASE_EXPORT PendingTask : public TrackingInfo {
+#if _MSC_VER >= 1700
+  PendingTask();
+#endif
   PendingTask(const tracked_objects::Location& posted_from,
               const Closure& task);
   PendingTask(const tracked_objects::Location& posted_from,
@@ -40,9 +43,6 @@ struct BASE_EXPORT PendingTask : public TrackingInfo {
 
   // OK to dispatch from a nested loop.
   bool nestable;
-
-  // Needs high resolution timers.
-  bool is_high_res;
 };
 
 // Wrapper around std::queue specialized for PendingTask which adds a Swap