Fix race in base::Timer when SetTaskRunner() is used

base/timer/timer.cc

 // Copyright (c) 2012 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 "base/timer/timer.h"
 
 #include <stddef.h>
 
 #include <utility>
 
@@ skipping 121 matching lines @@
   DCHECK(origin_sequence_checker_.CalledOnValidSequence());
 
   posted_from_ = posted_from;
   delay_ = delay;
   user_task_ = user_task;
 
   Reset();
 }
 
 void Timer::Stop() {
-  // TODO(gab): Enable this when it's no longer called racily from
-  // RunScheduledTask(): https://crbug.com/587199.
-  // DCHECK(origin_sequence_checker_.CalledOnValidSequence());
+  DCHECK(origin_sequence_checker_.CalledOnValidSequence());
 
   is_running_ = false;
   if (!retain_user_task_)
     user_task_.Reset();
   // No more member accesses here: |this| could be deleted after freeing
   // |user_task_|.
 }
 
 void Timer::Reset() {
   DCHECK(origin_sequence_checker_.CalledOnValidSequence());
@@ skipping 17 matching lines @@
     is_running_ = true;
     return;
   }
 
   // We can't reuse the |scheduled_task_|, so abandon it and post a new one.
   AbandonScheduledTask();
   PostNewScheduledTask(delay_);
 }
 
 TimeTicks Timer::Now() const {
-  // TODO(gab): Enable this when it's no longer called racily from
-  // RunScheduledTask(): https://crbug.com/587199.
-  // DCHECK(origin_sequence_checker_.CalledOnValidSequence());
+  DCHECK(origin_sequence_checker_.CalledOnValidSequence());
   return tick_clock_ ? tick_clock_->NowTicks() : TimeTicks::Now();
 }
 
 void Timer::PostNewScheduledTask(TimeDelta delay) {
-  // TODO(gab): Enable this when it's no longer called racily from
-  // RunScheduledTask(): https://crbug.com/587199.
-  // DCHECK(origin_sequence_checker_.CalledOnValidSequence());
+  DCHECK(origin_sequence_checker_.CalledOnValidSequence());
   DCHECK(!scheduled_task_);
   is_running_ = true;
   scheduled_task_ = new BaseTimerTaskInternal(this);
   if (delay > TimeDelta::FromMicroseconds(0)) {
-    // TODO(gab): Posting BaseTimerTaskInternal::Run to another sequence makes
-    // this code racy. https://crbug.com/587199
-    GetTaskRunner()->PostDelayedTask(
+    SequencedTaskRunnerHandle::Get()->PostDelayedTask(
         posted_from_,
         base::BindOnce(&BaseTimerTaskInternal::Run,
                        base::Owned(scheduled_task_)),
         delay);
     scheduled_run_time_ = desired_run_time_ = Now() + delay;
   } else {
-    GetTaskRunner()->PostTask(posted_from_,
-                              base::BindOnce(&BaseTimerTaskInternal::Run,
-                                             base::Owned(scheduled_task_)));
+    SequencedTaskRunnerHandle::Get()->PostTask(
+        posted_from_, base::BindOnce(&BaseTimerTaskInternal::Run,
+                                     base::Owned(scheduled_task_)));
     scheduled_run_time_ = desired_run_time_ = TimeTicks();
   }
 }
 
-scoped_refptr<SequencedTaskRunner> Timer::GetTaskRunner() {
-  return task_runner_.get() ? task_runner_ : SequencedTaskRunnerHandle::Get();
-}
-
 void Timer::AbandonScheduledTask() {
-  // TODO(gab): Enable this when it's no longer called racily from
-  // RunScheduledTask() -> Stop(): https://crbug.com/587199.
-  // DCHECK(origin_sequence_checker_.CalledOnValidSequence());
+  DCHECK(origin_sequence_checker_.CalledOnValidSequence());
   if (scheduled_task_) {
     scheduled_task_->Abandon();
     scheduled_task_ = nullptr;
   }
 }
 
 void Timer::RunScheduledTask() {
-  // TODO(gab): Enable this when it's no longer called racily:
-  // https://crbug.com/587199.
-  // DCHECK(origin_sequence_checker_.CalledOnValidSequence());
+  DCHECK(origin_sequence_checker_.CalledOnValidSequence());
 
   // 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_) {
     // Now() can be expensive, so only call it if we know the user has changed
     // the |desired_run_time_|.
     TimeTicks now = Now();
     // Task runner 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();
+  if (task_runner_ && !task_runner_->RunsTasksOnCurrentThread())
+    task_runner_->PostTask(posted_from_, std::move(task));
+  else
+    task.Run();
 
   // No more member accesses here: |this| could be deleted at this point.
 }
 
 }  // namespace base