Separate the create and start phases in DelayedTaskManager.

base/task_scheduler/delayed_task_manager.cc

Index: base/task_scheduler/delayed_task_manager.cc
diff --git a/base/task_scheduler/delayed_task_manager.cc b/base/task_scheduler/delayed_task_manager.cc
index 0bf7565a33957ae622b3f29b518df6d5e0be20b8..b0ef0812696158307eff8a02825b7d90865f8ad4 100644
--- a/base/task_scheduler/delayed_task_manager.cc
+++ b/base/task_scheduler/delayed_task_manager.cc
@@ -4,7 +4,7 @@
 
 #include "base/task_scheduler/delayed_task_manager.h"
 
-#include <utility>
+#include <algorithm>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ -14,30 +14,77 @@
 namespace base {
 namespace internal {
 
-DelayedTaskManager::DelayedTaskManager(
-    scoped_refptr<TaskRunner> service_thread_task_runner)
-    : service_thread_task_runner_(std::move(service_thread_task_runner)) {
-  DCHECK(service_thread_task_runner_);
+DelayedTaskManager::DelayedTaskManager(std::unique_ptr<TickClock> tick_clock)
+    : tick_clock_(std::move(tick_clock)) {
+  DCHECK(tick_clock_);
 }
 
 DelayedTaskManager::~DelayedTaskManager() = default;
 
+void DelayedTaskManager::Start(
+    scoped_refptr<TaskRunner> service_thread_task_runner) {
+  decltype(tasks_added_before_start_) tasks_added_before_start;
+
+  {
+    AutoSchedulerLock auto_lock(lock_);
+    DCHECK(service_thread_task_runner);

[robliao, 2017/04/19 17:54:07]

Nit: Move this DCHECK to the top of the function now that it's in its own scope.

[fdoray, 2017/04/20 12:08:52]

Done.

+    DCHECK(!service_thread_task_runner_);
+    DCHECK(!started_.IsSet());
+    service_thread_task_runner_ = std::move(service_thread_task_runner);
+    tasks_added_before_start = std::move(tasks_added_before_start_);
+    // |service_thread_task_runner_| must not change after |started_| is set.

[robliao, 2017/04/19 17:54:07]

Add a note explaining the why (accesses to service_thread_task_runner_ are
unsynchronized after this or refer to the header.)

[fdoray, 2017/04/20 12:08:52]

Done.

+    started_.Set();
+  }
+
+  const TimeTicks now = tick_clock_->NowTicks();
+  for (auto& task_and_callback : tasks_added_before_start) {
+    const TimeDelta delay =
+        std::max(TimeDelta(), task_and_callback.first->delayed_run_time - now);
+    AddDelayedTaskNow(std::move(task_and_callback.first), delay,
+                      std::move(task_and_callback.second));
+  }
+}
+
 void DelayedTaskManager::AddDelayedTask(
     std::unique_ptr<Task> task,
-    const PostTaskNowCallback& post_task_now_callback) {
+    PostTaskNowCallback post_task_now_callback) {
   DCHECK(task);
 
+  const TimeDelta delay = task->delay;
+  DCHECK(!delay.is_zero());
+
   // Use CHECK instead of DCHECK to crash earlier. See http://crbug.com/711167
   // for details.
   CHECK(task->task);
 
-  const TimeDelta delay = task->delay;
-  DCHECK(!delay.is_zero());
+  if (started_.IsSet()) {
+    // Fast path.

[robliao, 2017/04/19 17:54:07]

Remove this comment and the one below. It's not clear if noting a fast or slow
path adds value here.

Instead, state that if started_.IsSet(), then the delayed task manager is in a
stable state allowing a direct call to AddDelayedTaskNow.

Otherwise, we have to synchronize and recheck.

[fdoray, 2017/04/20 12:08:52]

Done.

+    AddDelayedTaskNow(std::move(task), delay,
+                      std::move(post_task_now_callback));
+  } else {
+    // Slow path.
+    AutoSchedulerLock auto_lock(lock_);
+    if (started_.IsSet()) {
+      AddDelayedTaskNow(std::move(task), delay,
+                        std::move(post_task_now_callback));
+    } else {
+      tasks_added_before_start_.push_back(
+          {std::move(task), std::move(post_task_now_callback)});
+    }
+  }
+}
 
+void DelayedTaskManager::AddDelayedTaskNow(
+    std::unique_ptr<Task> task,
+    TimeDelta delay,
+    PostTaskNowCallback post_task_now_callback) {
+  DCHECK(task);
+  DCHECK(started_.IsSet());
   // TODO(fdoray): Use |task->delayed_run_time| on the service thread
   // MessageLoop rather than recomputing it from |delay|.
   service_thread_task_runner_->PostDelayedTask(
-      FROM_HERE, BindOnce(post_task_now_callback, Passed(std::move(task))),
+      FROM_HERE,
+      BindOnce(std::move(post_task_now_callback), Passed(std::move(task))),
       delay);
 }