Revert 128993 - Refactor BaseTimer to avoid spamming the MessageLoop with orphaned tasks.

This change maintains the same API promises*, but instead of orphaning tasks when they are stopped, the BaseTimer_Helper class holds on to the task until either (1) it expires or (2) the user requests a delay that would arrive earlier than the pending task. If the user requests a longer delay than the pending task, a followup task will be posted when the pending task fires to span the remaining time.

* The one change of usage is related to threading. The threading requirements are now more strict. It is not allowed to destruct a timer on a different thread than the one used to post tasks. A thread ID DCHECK is now in place that will help catch misuse. Some existing instances are changed as part of this CL.

A side effect of this change is that the BaseTimer and DelayTimer are simplified to use features of BaseTimer_Helper (which is now called Timer).

As suggested in timer.h, I ran the disabled TimerTest tests from linux, and they pass consistently. I also added some new tests to verify correct run states.

BUG=117451,103667,119714,119750

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128412

Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=128506

Review URL: https://chromiumcodereview.appspot.com/9655006

TBR=jbates@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9791009

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

1 files changed, 14 insertions, 167 deletions

diff --git a/base/timer.cc b/base/timer.cc
index e51660c..5c2aa21 100644
--- a/base/timer.cc
+++ b/base/timer.cc
@@ -4,181 +4,28 @@
 
 #include "base/timer.h"
 
-#include "base/logging.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/message_loop.h"
 
 namespace base {
 
-// BaseTimerTaskInternal is a simple delegate for scheduling a callback to
-// Timer in the MessageLoop. It also handles the following edge
-// cases:
-// - deleted by MessageLoop.
-// - abandoned (orphaned) by Timer.
-class BaseTimerTaskInternal {
- public:
-  BaseTimerTaskInternal(Timer* timer)
-      : timer_(timer) {
+void BaseTimer_Helper::OrphanDelayedTask() {
+  if (delayed_task_) {
+    delayed_task_->timer_ = NULL;
+    delayed_task_ = NULL;
   }
-
-  ~BaseTimerTaskInternal() {
-    // This task may be getting cleared because the MessageLoop has been
-    // destructed.  If so, don't leave Timer with a dangling pointer
-    // to this.
-    if (timer_)
-      timer_->StopAndAbandon();
-  }
-
-  void Run() {
-    // timer_ is NULL if we were abandoned.
-    if (!timer_)
-      return;
-
-    // *this will be deleted by the MessageLoop, so Timer needs to
-    // forget us:
-    timer_->scheduled_task_ = NULL;
-
-    // Although Timer should not call back into *this, let's clear
-    // the timer_ member first to be pedantic.
-    Timer* timer = timer_;
-    timer_ = NULL;
-    timer->RunScheduledTask();
-  }
-
-  // The task remains in the MessageLoop queue, but nothing will happen when it
-  // runs.
-  void Abandon() {
-    timer_ = NULL;
-  }
-
- private:
-  Timer* timer_;
-};
-
-Timer::Timer(bool retain_user_task, bool is_repeating)
-    : scheduled_task_(NULL),
-      thread_id_(0),
-      is_repeating_(is_repeating),
-      retain_user_task_(retain_user_task),
-      is_running_(false) {
-}
-
-Timer::Timer(const tracked_objects::Location& posted_from,
-             TimeDelta delay,
-             const base::Closure& user_task,
-             bool is_repeating)
-    : scheduled_task_(NULL),
-      posted_from_(posted_from),
-      delay_(delay),
-      user_task_(user_task),
-      thread_id_(0),
-      is_repeating_(is_repeating),
-      retain_user_task_(true),
-      is_running_(false) {
-}
-
-Timer::~Timer() {
-  StopAndAbandon();
-}
-
-void Timer::Start(const tracked_objects::Location& posted_from,
-                  TimeDelta delay,
-                  const base::Closure& user_task) {
-  SetTaskInfo(posted_from, delay, user_task);
-  Reset();
-}
-
-void Timer::Stop() {
-  is_running_ = false;
-  if (!retain_user_task_)
-    user_task_.Reset();
-}
-
-void Timer::Reset() {
-  DCHECK(!user_task_.is_null());
-
-  // If there's no pending task, start one up and return.
-  if (!scheduled_task_) {
-    PostNewScheduledTask(delay_);
-    return;
-  }
-
-  // Set the new desired_run_time_.
-  desired_run_time_ = TimeTicks::Now() + delay_;
-
-  // We can use the existing scheduled task if it arrives before the new
-  // desired_run_time_.
-  if (desired_run_time_ > scheduled_run_time_) {
-    is_running_ = true;
-    return;
-  }
-
-  // We can't reuse the scheduled_task_, so abandon it and post a new one.
-  AbandonScheduledTask();
-  PostNewScheduledTask(delay_);
 }
 
-void Timer::SetTaskInfo(const tracked_objects::Location& posted_from,
-                        TimeDelta delay,
-                        const base::Closure& user_task) {
-  posted_from_ = posted_from;
-  delay_ = delay;
-  user_task_ = user_task;
-}
-
-void Timer::PostNewScheduledTask(TimeDelta delay) {
-  DCHECK(scheduled_task_ == NULL);
-  is_running_ = true;
-  scheduled_task_ = new BaseTimerTaskInternal(this);
-  MessageLoop::current()->PostDelayedTask(posted_from_,
-      base::Bind(&BaseTimerTaskInternal::Run, base::Owned(scheduled_task_)),
-      delay);
-  scheduled_run_time_ = desired_run_time_ = TimeTicks::Now() + delay;
-  // Remember the thread ID that posts the first task -- this will be verified
-  // later when the task is abandoned to detect misuse from multiple threads.
-  if (!thread_id_)
-    thread_id_ = static_cast<int>(PlatformThread::CurrentId());
-}
-
-void Timer::AbandonScheduledTask() {
-  DCHECK(thread_id_ == 0 ||
-         thread_id_ == static_cast<int>(PlatformThread::CurrentId()));
-  if (scheduled_task_) {
-    scheduled_task_->Abandon();
-    scheduled_task_ = NULL;
-  }
-}
-
-void Timer::RunScheduledTask() {
-  // Task may have been disabled.
-  if (!is_running_)
-    return;
-
-  // First check if we need to delay the task because of a new target time.
-  if (desired_run_time_ > scheduled_run_time_) {
-    // TimeTicks::Now() can be expensive, so only call it if we know the user
-    // has changed the desired_run_time_.
-    TimeTicks now = TimeTicks::Now();
-    // MessageLoop may have called us late anyway, so only post a continuation
-    // task if the desired_run_time_ is in the future.
-    if (desired_run_time_ > now) {
-      // Post a new task to span the remaining time.
-      PostNewScheduledTask(desired_run_time_ - now);
-      return;
-    }
-  }
-
-  // Make a local copy of the task to run. The Stop method will reset the
-  // user_task_ member if retain_user_task_ is false.
-  base::Closure task = user_task_;
-
-  if (is_repeating_)
-    PostNewScheduledTask(delay_);
-  else
-    Stop();
-
-  task.Run();
+void BaseTimer_Helper::InitiateDelayedTask(TimerTask* timer_task) {
+  OrphanDelayedTask();
 
-  // No more member accesses here: *this could be deleted at this point.
+  delayed_task_ = timer_task;
+  delayed_task_->timer_ = this;
+  MessageLoop::current()->PostDelayedTask(
+      timer_task->posted_from_,
+      base::Bind(&TimerTask::Run, base::Owned(timer_task)),
+      timer_task->delay_);
 }
 
 }  // namespace base