Minor cleanup to OneShotTimer and RepeatingTimer: moves more of the member variables into the Task subclass.

Also included in this change: deprecate MessageLoop::timer_manager(), and change consumers over to use OneShotTimer or RepeatingTimer.

R=beng
BUG=1346553

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

5 files changed, 100 insertions, 77 deletions

diff --git a/base/idletimer_unittest.cc b/base/idletimer_unittest.cc
index 1e2baa9..dab445e 100644
--- a/base/idletimer_unittest.cc
+++ b/base/idletimer_unittest.cc
@@ -46,7 +46,7 @@ class TestIdleTask : public IdleTimer {
 };
 
 // A task to help us quit the test.
-class TestFinishedTask : public Task {
+class TestFinishedTask {
  public:
   TestFinishedTask() {}
   void Run() {
@@ -55,7 +55,7 @@ class TestFinishedTask : public Task {
 };
 
 // A timer which resets the idle clock.
-class ResetIdleTask : public Task {
+class ResetIdleTask {
  public:
   ResetIdleTask() {}
   void Run() {
@@ -77,14 +77,15 @@ TEST_F(IdleTimerTest, NoRepeatIdle) {
 
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(false);
+
   TestFinishedTask finish_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t = loop->timer_manager()->StartTimer(1000, &finish_task, false);
+  base::OneShotTimer<TestFinishedTask> timer;
+  timer.Start(TimeDelta::FromSeconds(1), &finish_task, &TestFinishedTask::Run);
+
   test_task.Start();
-  loop->Run();
+  MessageLoop::current()->Run();
 
   EXPECT_EQ(test_task.get_idle_counter(), 1);
-  delete t;
 }
 
 TEST_F(IdleTimerTest, NoRepeatFlipIdleOnce) {
@@ -95,17 +96,22 @@ TEST_F(IdleTimerTest, NoRepeatFlipIdleOnce) {
 
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(false);
+
   TestFinishedTask finish_task;
   ResetIdleTask reset_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t1 = loop->timer_manager()->StartTimer(1000, &finish_task, false);
-  Timer* t2 = loop->timer_manager()->StartTimer(500, &reset_task, false);
+
+  base::OneShotTimer<TestFinishedTask> t1;
+  t1.Start(TimeDelta::FromMilliseconds(1000), &finish_task,
+           &TestFinishedTask::Run);
+  
+  base::OneShotTimer<ResetIdleTask> t2;
+  t2.Start(TimeDelta::FromMilliseconds(500), &reset_task,
+           &ResetIdleTask::Run);
+
   test_task.Start();
-  loop->Run();
+  MessageLoop::current()->Run();
 
   EXPECT_EQ(test_task.get_idle_counter(), 2);
-  delete t1;
-  delete t2;
 }
 
 TEST_F(IdleTimerTest, NoRepeatNotIdle) {
@@ -116,18 +122,25 @@ TEST_F(IdleTimerTest, NoRepeatNotIdle) {
 
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(false);
+
   TestFinishedTask finish_task;
   ResetIdleTask reset_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t = loop->timer_manager()->StartTimer(1000, &finish_task, false);
-  Timer* reset_timer = loop->timer_manager()->StartTimer(50, &reset_task, true);
+
+  base::OneShotTimer<TestFinishedTask> t;
+  t.Start(TimeDelta::FromMilliseconds(1000), &finish_task,
+          &TestFinishedTask::Run);
+  
+  base::RepeatingTimer<ResetIdleTask> reset_timer;
+  reset_timer.Start(TimeDelta::FromMilliseconds(50), &reset_task,
+                    &ResetIdleTask::Run);
+
   test_task.Start();
-  loop->Run();
-  loop->timer_manager()->StopTimer(reset_timer);
+
+  MessageLoop::current()->Run();
+
+  reset_timer.Stop();
 
   EXPECT_EQ(test_task.get_idle_counter(), 0);
-  delete t;
-  delete reset_timer;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -142,18 +155,21 @@ TEST_F(IdleTimerTest, Repeat) {
   // Verify that we fired 10 times.
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(true);
+
   TestFinishedTask finish_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t = loop->timer_manager()->StartTimer(1050, &finish_task, false);
+
+  base::OneShotTimer<TestFinishedTask> t;
+  t.Start(TimeDelta::FromMilliseconds(1050), &finish_task,
+          &TestFinishedTask::Run);
+
   test_task.Start();
-  loop->Run();
+  MessageLoop::current()->Run();
 
   // In a perfect world, the idle_counter should be 10.  However,
   // since timers aren't guaranteed to fire perfectly, this can
   // be less.  Just expect more than 5 and no more than 10.
   EXPECT_GT(test_task.get_idle_counter(), 5);
   EXPECT_LE(test_task.get_idle_counter(), 10);
-  delete t;
 }
 
 TEST_F(IdleTimerTest, RepeatIdleReset) {
@@ -163,21 +179,26 @@ TEST_F(IdleTimerTest, RepeatIdleReset) {
   // Verify that we fired 9 times.
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(true);
+
   ResetIdleTask reset_task;
   TestFinishedTask finish_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t1 = loop->timer_manager()->StartTimer(1000, &finish_task, false);
-  Timer* t2 = loop->timer_manager()->StartTimer(550, &reset_task, false);
+
+  base::OneShotTimer<TestFinishedTask> t1;
+  t1.Start(TimeDelta::FromMilliseconds(1000), &finish_task,
+           &TestFinishedTask::Run);
+  
+  base::OneShotTimer<ResetIdleTask> t2;
+  t2.Start(TimeDelta::FromMilliseconds(550), &reset_task,
+           &ResetIdleTask::Run);
+
   test_task.Start();
-  loop->Run();
+  MessageLoop::current()->Run();
 
   // In a perfect world, the idle_counter should be 9.  However,
   // since timers aren't guaranteed to fire perfectly, this can
   // be less.  Just expect more than 5 and no more than 9.
   EXPECT_GT(test_task.get_idle_counter(), 5);
   EXPECT_LE(test_task.get_idle_counter(), 9);
-  delete t1;
-  delete t2;
 }
 
 TEST_F(IdleTimerTest, RepeatNotIdle) {
@@ -188,17 +209,23 @@ TEST_F(IdleTimerTest, RepeatNotIdle) {
 
   mock_idle_time = GetTickCount();
   TestIdleTask test_task(true);
+
   TestFinishedTask finish_task;
   ResetIdleTask reset_task;
-  MessageLoop* loop = MessageLoop::current();
-  Timer* t1 = loop->timer_manager()->StartTimer(1000, &finish_task, false);
-  Timer* reset_timer = loop->timer_manager()->StartTimer(50, &reset_task, true);
+
+  base::OneShotTimer<TestFinishedTask> t;
+  t.Start(TimeDelta::FromMilliseconds(1000), &finish_task,
+          &TestFinishedTask::Run);
+  
+  base::RepeatingTimer<ResetIdleTask> reset_timer;
+  reset_timer.Start(TimeDelta::FromMilliseconds(50), &reset_task,
+                    &ResetIdleTask::Run);
+
   test_task.Start();
-  loop->Run();
-  loop->timer_manager()->StopTimer(reset_timer);
+  MessageLoop::current()->Run();
+
+  reset_timer.Stop();
 
   EXPECT_EQ(test_task.get_idle_counter(), 0);
-  delete t1;
-  delete reset_timer;
 }
 
diff --git a/base/message_loop.h b/base/message_loop.h
index efdb5d7..babc17a 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -199,8 +199,9 @@ class MessageLoop : public base::MessagePump::Delegate {
     return loop;
   }
 
-  // Returns the TimerManager object for the current thread.
-  base::TimerManager* timer_manager() { return &timer_manager_; }
+  // Returns the TimerManager object for the current thread.  This getter is
+  // deprecated.  Please use OneShotTimer or RepeatingTimer instead.
+  base::TimerManager* timer_manager_deprecated() { return &timer_manager_; }
 
   // Enables or disables the recursive task processing. This happens in the case
   // of recursive message loops. Some unwanted message loop may occurs when
diff --git a/base/timer.cc b/base/timer.cc
index 7de55c7..9f6e7b0 100644
--- a/base/timer.cc
+++ b/base/timer.cc
@@ -231,7 +231,8 @@ void BaseTimer_Helper::InitiateDelayedTask(TimerTask* timer_task) {
   delayed_task_ = timer_task;
   delayed_task_->timer_ = this;
   MessageLoop::current()->PostDelayedTask(
-      FROM_HERE, timer_task, static_cast<int>(delay_.InMilliseconds()));
+      FROM_HERE, timer_task,
+      static_cast<int>(timer_task->delay_.InMilliseconds()));
 }
 
 }  // namespace base
diff --git a/base/timer.h b/base/timer.h
index d78423a..e1a77e2 100644
--- a/base/timer.h
+++ b/base/timer.h
@@ -260,14 +260,16 @@ class BaseTimer_Helper {
   }
 
  protected:
-  BaseTimer_Helper(bool repeating)
-      : delayed_task_(NULL), repeating_(repeating) {
-  }
+  BaseTimer_Helper() : delayed_task_(NULL) {}
 
   // We have access to the timer_ member so we can orphan this task.
   class TimerTask : public Task {
    public:
+    TimerTask(TimeDelta delay) : delay_(delay) {
+      // timer_ is set in InitiateDelayedTask.
+    }
     BaseTimer_Helper* timer_;
+    TimeDelta delay_;
   };
 
   // Used to orphan delayed_task_ so that when it runs it does nothing.
@@ -278,8 +280,6 @@ class BaseTimer_Helper {
   void InitiateDelayedTask(TimerTask* timer_task);
 
   TimerTask* delayed_task_;
-  TimeDelta delay_;
-  bool repeating_;
 
   DISALLOW_COPY_AND_ASSIGN(BaseTimer_Helper);
 };
@@ -287,76 +287,69 @@ class BaseTimer_Helper {
 //-----------------------------------------------------------------------------
 // This class is an implementation detail of OneShotTimer and RepeatingTimer.
 // Please do not use this class directly.
-template <class Receiver>
+template <class Receiver, bool kIsRepeating>
 class BaseTimer : public BaseTimer_Helper {
  public:
   typedef void (Receiver::*ReceiverMethod)();
 
-  // The task must be set using set_task before calling Start.
-  BaseTimer(bool repeating)
-      : BaseTimer_Helper(repeating), receiver_(NULL), receiver_method_(NULL) {
-  }
-
   // Call this method to start the timer.  It is an error to call this method
   // while the timer is already running.
   void Start(TimeDelta delay, Receiver* receiver, ReceiverMethod method) {
     DCHECK(!IsRunning());
-    delay_ = delay;
-    receiver_ = receiver;
-    receiver_method_ = method;
-    InitiateDelayedTask(new TimerTask());
+    InitiateDelayedTask(new TimerTask(delay, receiver, method));
   }
 
   // Call this method to stop the timer.  It is a no-op if the timer is not
   // running.
   void Stop() {
-    receiver_ = NULL;
-    receiver_method_ = NULL;
     OrphanDelayedTask();
   }
 
   // Call this method to reset the timer delay of an already running timer.
   void Reset() {
     DCHECK(IsRunning());
-    OrphanDelayedTask();
-    InitiateDelayedTask(new TimerTask());
+    InitiateDelayedTask(static_cast<TimerTask*>(delayed_task_)->Clone());
   }
 
  private:
+  typedef BaseTimer<Receiver, kIsRepeating> SelfType;
+  
   class TimerTask : public BaseTimer_Helper::TimerTask {
    public:
+    TimerTask(TimeDelta delay, Receiver* receiver, ReceiverMethod method)
+        : BaseTimer_Helper::TimerTask(delay),
+          receiver_(receiver),
+          method_(method) {
+    }
     virtual void Run() {
       if (!timer_)  // timer_ is null if we were orphaned.
         return;
-      BaseTimer<Receiver>* self = static_cast<BaseTimer<Receiver>*>(timer_);
-      if (self->repeating_) {
+      SelfType* self = static_cast<SelfType*>(timer_);
+      if (kIsRepeating) {
         self->Reset();
       } else {
         self->delayed_task_ = NULL;
       }
-      DispatchToMethod(self->receiver_, self->receiver_method_, Tuple0());
+      DispatchToMethod(receiver_, method_, Tuple0());
+    }
+    TimerTask* Clone() const {
+      return new TimerTask(delay_, receiver_, method_);
     }
+   private:
+    Receiver* receiver_;
+    ReceiverMethod method_;
   };
-  
-  Receiver* receiver_;
-  ReceiverMethod receiver_method_;
 };
 
 //-----------------------------------------------------------------------------
 // A simple, one-shot timer.  See usage notes at the top of the file.
 template <class Receiver>
-class OneShotTimer : public BaseTimer<Receiver> {
- public:
-  OneShotTimer() : BaseTimer<Receiver>(false) {}
-};
+class OneShotTimer : public BaseTimer<Receiver, false> {};
 
 //-----------------------------------------------------------------------------
 // A simple, repeating timer.  See usage notes at the top of the file.
 template <class Receiver>
-class RepeatingTimer : public BaseTimer<Receiver> {
- public:
-  RepeatingTimer() : BaseTimer<Receiver>(true) {}
-};
+class RepeatingTimer : public BaseTimer<Receiver, true> {};
 
 }  // namespace base
 
diff --git a/base/timer_unittest.cc b/base/timer_unittest.cc
index e4b45ec..6645885 100644
--- a/base/timer_unittest.cc
+++ b/base/timer_unittest.cc
@@ -75,12 +75,13 @@ TimerTask::TimerTask(int delay, bool repeating)
       timer_(NULL) {
   Reset();  // This will just set up the variables to indicate we have a
             // running timer.
-  timer_ = message_loop()->timer_manager()->StartTimer(delay, this, repeating);
+  timer_ = message_loop()->timer_manager_deprecated()->StartTimer(
+      delay, this, repeating);
 }
 
 TimerTask::~TimerTask() {
   if (timer_) {
-    message_loop()->timer_manager()->StopTimer(timer_);
+    message_loop()->timer_manager_deprecated()->StopTimer(timer_);
     delete timer_;
   }
   if (timer_running_) {
@@ -97,7 +98,7 @@ void TimerTask::Reset() {
   }
   if (timer_) {
     start_ticks_ = TimeTicks::Now();
-    message_loop()->timer_manager()->ResetTimer(timer_);
+    message_loop()->timer_manager_deprecated()->ResetTimer(timer_);
   }
 }
 
@@ -116,7 +117,7 @@ void TimerTask::Run() {
   // If we're done running, shut down the message loop.
   if (timer_->repeating() && (iterations_ < 10))
     return;  // Iterate 10 times before terminating.
-  message_loop()->timer_manager()->StopTimer(timer_);
+  message_loop()->timer_manager_deprecated()->StopTimer(timer_);
   timer_running_ = false;
   if (--timer_count_ <= 0)
     QuitMessageLoop();
@@ -224,7 +225,7 @@ void RunTest_BrokenTimer(MessageLoop::Type message_loop_type) {
   // Simulate faulty early-firing timers. The tasks in RunTimerTest should
   // nevertheless be invoked after their specified delays, regardless of when
   // WM_TIMER fires.
-  TimerManager* manager = MessageLoop::current()->timer_manager();
+  TimerManager* manager = MessageLoop::current()->timer_manager_deprecated();
   manager->set_use_broken_delay(true);
   RunTimerTest();
   manager->set_use_broken_delay(false);