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

Review URL: http://codereview.chromium.org/9655006

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

3 files changed, 411 insertions, 185 deletions

diff --git a/base/timer.cc b/base/timer.cc
index 5c2aa21..e51660c 100644
--- a/base/timer.cc
+++ b/base/timer.cc
@@ -4,28 +4,181 @@
 
 #include "base/timer.h"
 
-#include "base/bind.h"
-#include "base/bind_helpers.h"
+#include "base/logging.h"
 #include "base/message_loop.h"
 
 namespace base {
 
-void BaseTimer_Helper::OrphanDelayedTask() {
-  if (delayed_task_) {
-    delayed_task_->timer_ = NULL;
-    delayed_task_ = NULL;
+// 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) {
   }
+
+  ~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 BaseTimer_Helper::InitiateDelayedTask(TimerTask* timer_task) {
-  OrphanDelayedTask();
+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();
 
-  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_);
+  // No more member accesses here: *this could be deleted at this point.
 }
 
 }  // namespace base
diff --git a/base/timer.h b/base/timer.h
index 9c9bec7..24f2bb1 100644
--- a/base/timer.h
+++ b/base/timer.h
@@ -38,6 +38,8 @@
 // calling Reset on timer_ would postpone DoStuff by another 1 second.  In
 // other words, Reset is shorthand for calling Stop and then Start again with
 // the same arguments.
+//
+// NOTE: These APIs are not thread safe. Always call from the same thread.
 
 #ifndef BASE_TIMER_H_
 #define BASE_TIMER_H_
@@ -49,174 +51,162 @@
 // should be able to tell the difference.
 
 #include "base/base_export.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback.h"
 #include "base/location.h"
-#include "base/logging.h"
 #include "base/time.h"
 
 class MessageLoop;
 
 namespace base {
 
+class BaseTimerTaskInternal;
+
 //-----------------------------------------------------------------------------
-// This class is an implementation detail of OneShotTimer and RepeatingTimer.
-// Please do not use this class directly.
+// This class wraps MessageLoop::PostDelayedTask to manage delayed and repeating
+// tasks. It must be destructed on the same thread that starts tasks. There are
+// DCHECKs in place to verify this.
 //
-// This class exists to share code between BaseTimer<T> template instantiations.
-//
-class BASE_EXPORT BaseTimer_Helper {
+class BASE_EXPORT Timer {
  public:
-  // Stops the timer.
-  ~BaseTimer_Helper() {
-    OrphanDelayedTask();
-  }
+  // Construct a timer in repeating or one-shot mode. Start or SetTaskInfo must
+  // be called later to set task info. |retain_user_task| determines whether the
+  // user_task is retained or reset when it runs or stops.
+  Timer(bool retain_user_task, bool is_repeating);
+
+  // Construct a timer with retained task info.
+  Timer(const tracked_objects::Location& posted_from,
+        TimeDelta delay,
+        const base::Closure& user_task,
+        bool is_repeating);
+
+  virtual ~Timer();
 
   // Returns true if the timer is running (i.e., not stopped).
   bool IsRunning() const {
-    return delayed_task_ != NULL;
+    return is_running_;
   }
 
-  // Returns the current delay for this timer.  May only call this method when
-  // the timer is running!
+  // Returns the current delay for this timer.
   TimeDelta GetCurrentDelay() const {
-    DCHECK(IsRunning());
-    return delayed_task_->delay_;
+    return delay_;
   }
 
+  // Start the timer to run at the given |delay| from now. If the timer is
+  // already running, it will be replaced to call the given |user_task|.
+  void Start(const tracked_objects::Location& posted_from,
+             TimeDelta delay,
+             const base::Closure& user_task);
+
+  // Call this method to stop and cancel the timer.  It is a no-op if the timer
+  // is not running.
+  void Stop();
+
+  // Call this method to reset the timer delay. The user_task_ must be set. If
+  // the timer is not running, this will start it by posting a task.
+  void Reset();
+
+  const base::Closure& user_task() const { return user_task_; }
+
  protected:
-  BaseTimer_Helper() : delayed_task_(NULL) {}
-
-  // We have access to the timer_ member so we can orphan this task.
-  class TimerTask {
-   public:
-    TimerTask(const tracked_objects::Location& posted_from,
-              TimeDelta delay)
-        : posted_from_(posted_from),
-          timer_(NULL),
-          delay_(delay) {
-    }
-    virtual ~TimerTask() {}
-    virtual void Run() = 0;
-    tracked_objects::Location posted_from_;
-    BaseTimer_Helper* timer_;
-    TimeDelta delay_;
-  };
-
-  // Used to orphan delayed_task_ so that when it runs it does nothing.
-  void OrphanDelayedTask();
-
-  // Used to initiated a new delayed task.  This has the side-effect of
-  // orphaning delayed_task_ if it is non-null.
-  void InitiateDelayedTask(TimerTask* timer_task);
-
-  TimerTask* delayed_task_;
-
-  DISALLOW_COPY_AND_ASSIGN(BaseTimer_Helper);
+  // Used to initiate a new delayed task.  This has the side-effect of disabling
+  // scheduled_task_ if it is non-null.
+  void SetTaskInfo(const tracked_objects::Location& posted_from,
+                   TimeDelta delay,
+                   const base::Closure& user_task);
+
+ private:
+  friend class BaseTimerTaskInternal;
+
+  // Allocates a new scheduled_task_ and posts it on the current MessageLoop
+  // with the given |delay|. scheduled_task_ must be NULL. scheduled_run_time_
+  // and desired_run_time_ are reset to Now() + delay.
+  void PostNewScheduledTask(TimeDelta delay);
+
+  // Disable scheduled_task_ and abandon it so that it no longer refers back to
+  // this object.
+  void AbandonScheduledTask();
+
+  // Called by BaseTimerTaskInternal when the MessageLoop runs it.
+  void RunScheduledTask();
+
+  // Stop running task (if any) and abandon scheduled task (if any).
+  void StopAndAbandon() {
+    Stop();
+    AbandonScheduledTask();
+  }
+
+  // When non-NULL, the scheduled_task_ is waiting in the MessageLoop to call
+  // RunScheduledTask() at scheduled_run_time_.
+  BaseTimerTaskInternal* scheduled_task_;
+
+  // Location in user code.
+  tracked_objects::Location posted_from_;
+  // Delay requested by user.
+  TimeDelta delay_;
+  // user_task_ is what the user wants to be run at desired_run_time_.
+  base::Closure user_task_;
+
+  // The estimated time that the MessageLoop will run the scheduled_task_ that
+  // will call RunScheduledTask().
+  TimeTicks scheduled_run_time_;
+
+  // The desired run time of user_task_. The user may update this at any time,
+  // even if their previous request has not run yet. If desired_run_time_ is
+  // greater than scheduled_run_time_, a continuation task will be posted to
+  // wait for the remaining time. This allows us to reuse the pending task so as
+  // not to flood the MessageLoop with orphaned tasks when the user code
+  // excessively Stops and Starts the timer.
+  TimeTicks desired_run_time_;
+
+  // Thread ID of current MessageLoop for verifying single-threaded usage.
+  int thread_id_;
+
+  // Repeating timers automatically post the task again before calling the task
+  // callback.
+  const bool is_repeating_;
+
+  // If true, hold on to the user_task_ closure object for reuse.
+  const bool retain_user_task_;
+
+  // If true, user_task_ is scheduled to run sometime in the future.
+  bool is_running_;
+
+  DISALLOW_COPY_AND_ASSIGN(Timer);
 };
 
 //-----------------------------------------------------------------------------
 // This class is an implementation detail of OneShotTimer and RepeatingTimer.
 // Please do not use this class directly.
 template <class Receiver, bool kIsRepeating>
-class BaseTimer : public BaseTimer_Helper {
+class BaseTimerMethodPointer : public Timer {
  public:
   typedef void (Receiver::*ReceiverMethod)();
 
-  // Call this method to start the timer.  It is an error to call this method
-  // while the timer is already running.
+  BaseTimerMethodPointer() : Timer(kIsRepeating, kIsRepeating) {}
+
+  // Start the timer to run at the given |delay| from now. If the timer is
+  // already running, it will be replaced to call a task formed from
+  // |reviewer->*method|.
   void Start(const tracked_objects::Location& posted_from,
              TimeDelta delay,
              Receiver* receiver,
              ReceiverMethod method) {
-    DCHECK(!IsRunning());
-    InitiateDelayedTask(new TimerTask(posted_from, delay, receiver, method));
+    Timer::Start(posted_from, delay,
+                 base::Bind(method, base::Unretained(receiver)));
   }
-
-  // Call this method to stop the timer.  It is a no-op if the timer is not
-  // running.
-  void Stop() {
-    OrphanDelayedTask();
-  }
-
-  // Call this method to reset the timer delay of an already running timer.
-  void Reset() {
-    DCHECK(IsRunning());
-    InitiateDelayedTask(static_cast<TimerTask*>(delayed_task_)->Clone());
-  }
-
- private:
-  typedef BaseTimer<Receiver, kIsRepeating> SelfType;
-
-  class TimerTask : public BaseTimer_Helper::TimerTask {
-   public:
-    TimerTask(const tracked_objects::Location& posted_from,
-              TimeDelta delay,
-              Receiver* receiver,
-              ReceiverMethod method)
-        : BaseTimer_Helper::TimerTask(posted_from, delay),
-          receiver_(receiver),
-          method_(method) {
-    }
-
-    virtual ~TimerTask() {
-      // This task may be getting cleared because the MessageLoop has been
-      // destructed.  If so, don't leave the Timer with a dangling pointer
-      // to this now-defunct task.
-      ClearBaseTimer();
-    }
-
-    virtual void Run() {
-      if (!timer_)  // timer_ is null if we were orphaned.
-        return;
-      if (kIsRepeating)
-        ResetBaseTimer();
-      else
-        ClearBaseTimer();
-      (receiver_->*method_)();
-    }
-
-    TimerTask* Clone() const {
-      return new TimerTask(posted_from_, delay_, receiver_, method_);
-    }
-
-   private:
-    // Inform the Base that the timer is no longer active.
-    void ClearBaseTimer() {
-      if (timer_) {
-        SelfType* self = static_cast<SelfType*>(timer_);
-        // It is possible that the Timer has already been reset, and that this
-        // Task is old.  So, if the Timer points to a different task, assume
-        // that the Timer has already taken care of properly setting the task.
-        if (self->delayed_task_ == this)
-          self->delayed_task_ = NULL;
-        // By now the delayed_task_ in the Timer does not point to us anymore.
-        // We should reset our own timer_ because the Timer can not do this
-        // for us in its destructor.
-        timer_ = NULL;
-      }
-    }
-
-    // Inform the Base that we're resetting the timer.
-    void ResetBaseTimer() {
-      DCHECK(timer_);
-      DCHECK(kIsRepeating);
-      SelfType* self = static_cast<SelfType*>(timer_);
-      self->Reset();
-    }
-
-    Receiver* receiver_;
-    ReceiverMethod method_;
-  };
 };
 
 //-----------------------------------------------------------------------------
 // A simple, one-shot timer.  See usage notes at the top of the file.
 template <class Receiver>
-class OneShotTimer : public BaseTimer<Receiver, false> {};
+class OneShotTimer : public BaseTimerMethodPointer<Receiver, false> {};
 
 //-----------------------------------------------------------------------------
 // A simple, repeating timer.  See usage notes at the top of the file.
 template <class Receiver>
-class RepeatingTimer : public BaseTimer<Receiver, true> {};
+class RepeatingTimer : public BaseTimerMethodPointer<Receiver, true> {};
 
 //-----------------------------------------------------------------------------
 // A Delay timer is like The Button from Lost. Once started, you have to keep
@@ -230,7 +220,7 @@ class RepeatingTimer : public BaseTimer<Receiver, true> {};
 // If destroyed, the timeout is canceled and will not occur even if already
 // inflight.
 template <class Receiver>
-class DelayTimer {
+class DelayTimer : protected Timer {
  public:
   typedef void (Receiver::*ReceiverMethod)();
 
@@ -238,51 +228,11 @@ class DelayTimer {
              TimeDelta delay,
              Receiver* receiver,
              ReceiverMethod method)
-      : posted_from_(posted_from),
-        receiver_(receiver),
-        method_(method),
-        delay_(delay) {
-  }
-
-  void Reset() {
-    DelayFor(delay_);
-  }
-
- private:
-  void DelayFor(TimeDelta delay) {
-    trigger_time_ = TimeTicks::Now() + delay;
-
-    // If we already have a timer that will expire at or before the given delay,
-    // then we have nothing more to do now.
-    if (timer_.IsRunning() && timer_.GetCurrentDelay() <= delay)
-      return;
-
-    // The timer isn't running, or will expire too late, so restart it.
-    timer_.Stop();
-    timer_.Start(posted_from_, delay, this, &DelayTimer<Receiver>::Check);
-  }
-
-  void Check() {
-    if (trigger_time_.is_null())
-      return;
-
-    // If we have not waited long enough, then wait some more.
-    const TimeTicks now = TimeTicks::Now();
-    if (now < trigger_time_) {
-      DelayFor(trigger_time_ - now);
-      return;
-    }
-
-    (receiver_->*method_)();
-  }
-
-  tracked_objects::Location posted_from_;
-  Receiver *const receiver_;
-  const ReceiverMethod method_;
-  const TimeDelta delay_;
+      : Timer(posted_from, delay,
+              base::Bind(method, base::Unretained(receiver)),
+              false) {}
 
-  OneShotTimer<DelayTimer<Receiver> > timer_;
-  TimeTicks trigger_time_;
+  void Reset() { Timer::Reset(); }
 };
 
 }  // namespace base
diff --git a/base/timer_unittest.cc b/base/timer_unittest.cc
index 929bd38..71a752e 100644
--- a/base/timer_unittest.cc
+++ b/base/timer_unittest.cc
@@ -352,3 +352,126 @@ TEST(TimerTest, MessageLoopShutdown) {
 
   EXPECT_FALSE(did_run);
 }
+
+void TimerTestCallback() {
+}
+
+TEST(TimerTest, NonRepeatIsRunning) {
+  {
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+    timer.Stop();
+    EXPECT_FALSE(timer.IsRunning());
+    EXPECT_TRUE(timer.user_task().is_null());
+  }
+
+  {
+    base::Timer timer(true, false);
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+    timer.Stop();
+    EXPECT_FALSE(timer.IsRunning());
+    ASSERT_FALSE(timer.user_task().is_null());
+    timer.Reset();
+    EXPECT_TRUE(timer.IsRunning());
+  }
+}
+
+TEST(TimerTest, NonRepeatMessageLoopDeath) {
+  base::Timer timer(false, false);
+  {
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+  }
+  EXPECT_FALSE(timer.IsRunning());
+  EXPECT_TRUE(timer.user_task().is_null());
+}
+
+TEST(TimerTest, RetainRepeatIsRunning) {
+  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+  base::Timer timer(FROM_HERE, TimeDelta::FromDays(1),
+                    base::Bind(&TimerTestCallback), true);
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+  timer.Stop();
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+}
+
+TEST(TimerTest, RetainNonRepeatIsRunning) {
+  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+  base::Timer timer(FROM_HERE, TimeDelta::FromDays(1),
+                    base::Bind(&TimerTestCallback), false);
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+  timer.Stop();
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+}
+
+namespace {
+
+bool g_callback_happened1 = false;
+bool g_callback_happened2 = false;
+
+void ClearAllCallbackHappened() {
+  g_callback_happened1 = false;
+  g_callback_happened2 = false;
+}
+
+void SetCallbackHappened1() {
+  g_callback_happened1 = true;
+  MessageLoop::current()->Quit();
+}
+
+void SetCallbackHappened2() {
+  g_callback_happened2 = true;
+  MessageLoop::current()->Quit();
+}
+
+TEST(TimerTest, ContinuationStopStart) {
+  {
+    ClearAllCallbackHappened();
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(10),
+                base::Bind(&SetCallbackHappened1));
+    timer.Stop();
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(40),
+                base::Bind(&SetCallbackHappened2));
+    MessageLoop::current()->Run();
+    EXPECT_FALSE(g_callback_happened1);
+    EXPECT_TRUE(g_callback_happened2);
+  }
+}
+
+TEST(TimerTest, ContinuationReset) {
+  {
+    ClearAllCallbackHappened();
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(10),
+                base::Bind(&SetCallbackHappened1));
+    timer.Reset();
+    // Since Reset happened before task ran, the user_task must not be cleared:
+    ASSERT_FALSE(timer.user_task().is_null());
+    MessageLoop::current()->Run();
+    EXPECT_TRUE(g_callback_happened1);
+  }
+}
+
+}  // namespace