Refactor TimeDomain::MaybeAdvanceTime in preparation for DoWork deduping

third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc

Index: third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
diff --git a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
index cbfc90cdd527d88838cf21921758e75ccd0ec4bb..906e937675c2403704dc1f9687999781babb54ea 100644
--- a/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
+++ b/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
@@ -147,15 +147,17 @@ void TaskQueueManager::UnregisterTaskQueue(
   selector_.RemoveQueue(task_queue.get());
 }
 
-void TaskQueueManager::UpdateWorkQueues(LazyNow lazy_now) {
+void TaskQueueManager::UpdateWorkQueues(LazyNow* lazy_now) {
   TRACE_EVENT0(disabled_by_default_tracing_category_,
                "TaskQueueManager::UpdateWorkQueues");
 
   for (TimeDomain* time_domain : time_domains_) {
-    LazyNow lazy_now_in_domain = time_domain == real_time_domain_.get()
-                                     ? lazy_now
-                                     : time_domain->CreateLazyNow();
-    time_domain->UpdateWorkQueues(lazy_now_in_domain);
+    if (time_domain == real_time_domain_.get()) {
+      time_domain->UpdateWorkQueues(lazy_now);
+    } else {
+      LazyNow time_domain_lazy_now = time_domain->CreateLazyNow();
+      time_domain->UpdateWorkQueues(&time_domain_lazy_now);
+    }
   }
 }
 
@@ -232,7 +234,7 @@ void TaskQueueManager::DoWork(base::TimeTicks run_time, bool from_main_thread) {
     queues_to_delete_.clear();
 
   LazyNow lazy_now(real_time_domain()->CreateLazyNow());

[Sami, 2017/01/18 18:45:57]

General comment re: |lazy_now|, not sure if it concerns this patch: it's pretty
hard to see what the period of validity of each instance is and to accidentally
hang on to one for too long. I was thinking we might want some debug-only thing
that invalidates all outstanding LazyNows in key places (e.g., when a task is
run) and blows a DCHECK if the LazyNow is held on to across that boundary.

Alternatively we could just simplify things and create one LazyNow per batch
iteration.

-  UpdateWorkQueues(lazy_now);
+  UpdateWorkQueues(&lazy_now);
 
   for (int i = 0; i < work_batch_size_; i++) {
     internal::WorkQueue* work_queue;
@@ -251,7 +253,7 @@ void TaskQueueManager::DoWork(base::TimeTicks run_time, bool from_main_thread) {
 
     work_queue = nullptr;  // The queue may have been unregistered.
 
-    UpdateWorkQueues(lazy_now);
+    UpdateWorkQueues(&lazy_now);
 
     // Only run a single task per batch in nested run loops so that we can
     // properly exit the nested loop when someone calls RunLoop::Quit().
@@ -264,16 +266,38 @@ void TaskQueueManager::DoWork(base::TimeTicks run_time, bool from_main_thread) {
   // TODO(alexclarke): Consider refactoring the above loop to terminate only
   // when there's no more work left to be done, rather than posting a
   // continuation task.
-  if (!selector_.EnabledWorkQueuesEmpty() || TryAdvanceTimeDomains())
+  base::Optional<base::TimeDelta> next_delay =
+      ComputeDelayTillNextTask(&lazy_now);
+
+  if (!next_delay)
+    return;
+
+  base::TimeDelta delay = next_delay.value();
+  if (delay.is_zero()) {
     MaybeScheduleImmediateWork(FROM_HERE);
+  } else {
+    MaybeScheduleDelayedWork(FROM_HERE, lazy_now.Now(), delay);
+  }
 }
 
-bool TaskQueueManager::TryAdvanceTimeDomains() {
-  bool can_advance = false;
+base::Optional<base::TimeDelta> TaskQueueManager::ComputeDelayTillNextTask(
+    LazyNow* lazy_now) {
+  // If the selector has non-empty queues we trivially know there is immediate
+  // work to be done.
+  if (!selector_.EnabledWorkQueuesEmpty())
+    return base::TimeDelta();
+
+  // Otherwise we need to find the shortest delay, if any.
+  base::Optional<base::TimeDelta> next_continuation;
   for (TimeDomain* time_domain : time_domains_) {
-    can_advance |= time_domain->MaybeAdvanceTime();
+    base::Optional<base::TimeDelta> continuation =
+        time_domain->DelayTillNextTask(lazy_now);
+    if (!continuation)
+      continue;
+    if (!next_continuation || next_continuation.value() > continuation.value())
+      next_continuation = continuation;
   }
-  return can_advance;
+  return next_continuation;
 }
 
 bool TaskQueueManager::SelectWorkQueueToService(
@@ -299,8 +323,10 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
       work_queue->TakeTaskFromWorkQueue();
 
   // It's possible the task was canceled, if so bail out.
-  if (pending_task.task.IsCancelled())
+  if (pending_task.task.IsCancelled()) {
+    *lazy_now = real_time_domain()->CreateLazyNow();

[Sami, 2017/01/18 18:45:57]

We were assuming that operations not involving running tasks were infinitely
fast, so |lazy_now| was still valid. Do you think we should change that
assumption?

[alex clarke (OOO till 29th), 2017/01/19 08:56:10]

Yes because task selection takes time.  Constructing a LazyNow is very fast, we
could move this out of the if if you prefer.

[Sami, 2017/01/19 12:01:00]

Yeah, actually I think we could just create one at the beginning of the loop
where we're calling this and not rely on ProcessTaskFromWorkQueue keeping it up
to date. That way it's really clear when we sample the time.

[Sami, 2017/01/19 12:10:15]

Offline: turns out we can't because of time observers. Decided to split this
into two different lazy nows: lazy_now_before_task and lazy_now_after_task.

[alex clarke (OOO till 29th), 2017/01/19 14:25:02]

Done.

     return ProcessTaskResult::EXECUTED;
+  }
 
   internal::TaskQueueImpl* queue = work_queue->task_queue();
   if (queue->GetQuiescenceMonitored())
@@ -316,6 +342,7 @@ TaskQueueManager::ProcessTaskResult TaskQueueManager::ProcessTaskFromWorkQueue(
     delegate_->PostNonNestableTask(
         pending_task.posted_from,
         UnsafeConvertOnceClosureToRepeating(std::move(pending_task.task)));
+    *lazy_now = real_time_domain()->CreateLazyNow();
     return ProcessTaskResult::DEFERRED;
   }