[scheduler] Change TaskQueue observer call mechanism.

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

 // Copyright 2015 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 "platform/scheduler/base/task_queue_impl.h"
 
 #include <utility>
 
 #include "base/format_macros.h"
 #include "base/memory/ptr_util.h"
@@ skipping 272 matching lines @@
   main_thread_only().delayed_incoming_queue.push(std::move(pending_task));
 
   // If |pending_task| is at the head of the queue, then make sure a wake-up
   // is requested if the queue is enabled.  Note we still want to schedule a
   // wake-up even if blocked by a fence, because we'd break throttling logic
   // otherwise.
   DelayedWakeUp new_wake_up =
       main_thread_only().delayed_incoming_queue.top().delayed_wake_up();
   if (wake_up.time == new_wake_up.time &&
       wake_up.sequence_num == new_wake_up.sequence_num) {
     ScheduleDelayedWorkInTimeDomain(now);

[alex clarke (OOO till 29th), 2017/05/09 11:51:28]

Should we notify here?

[altimin, 2017/05/09 13:15:04]

No, time domain will call ScheduleDelayedWorkInTimeDomain and it will issue the
notification.

   }
 
   TraceQueueSize();
 }
 
 void TaskQueueImpl::PushOntoDelayedIncomingQueueLocked(Task pending_task) {
   any_thread().task_queue_manager->DidQueueTask(pending_task);
 
   int thread_hop_task_sequence_number =
       any_thread().task_queue_manager->GetNextSequenceNumber();
@@ skipping 91 matching lines @@
   size_t task_count = 0;
   task_count += main_thread_only().delayed_work_queue->Size();
   task_count += main_thread_only().delayed_incoming_queue.size();
   task_count += main_thread_only().immediate_work_queue->Size();
 
   base::AutoLock lock(immediate_incoming_queue_lock_);
   task_count += immediate_incoming_queue().size();
   return task_count;
 }
 
-bool TaskQueueImpl::HasPendingImmediateWork() const {
+bool TaskQueueImpl::HasTaskToRunImmediately() const {
   // Any work queue tasks count as immediate work.
   if (!main_thread_only().delayed_work_queue->Empty() ||
       !main_thread_only().immediate_work_queue->Empty()) {
     return true;
   }
 
   // Tasks on |delayed_incoming_queue| that could run now, count as
   // immediate work.
   if (!main_thread_only().delayed_incoming_queue.empty() &&
       main_thread_only().delayed_incoming_queue.top().delayed_run_time <=
@@ skipping 398 matching lines @@
   bool is_enabled = IsQueueEnabled();
   if (was_enabled != is_enabled)
     EnableOrDisableWithSelector(is_enabled);
 }
 
 void TaskQueueImpl::EnableOrDisableWithSelector(bool enable) {
   if (!main_thread_only().task_queue_manager)
     return;
 
   if (enable) {
-    if (HasPendingImmediateWork())
-      NotifyWakeUpChangedOnMainThread(base::TimeTicks());
+    if (HasPendingImmediateWork() && main_thread_only().observer) {
+      // Delayed work notification will be issued via time domain.
+      main_thread_only().observer->OnQueueNextWakeUpChanged(this,
+                                                            base::TimeTicks());
+    }
 
     ScheduleDelayedWorkInTimeDomain(main_thread_only().time_domain->Now());
 
     // Note the selector calls TaskQueueManager::OnTaskQueueEnabled which posts
     // a DoWork if needed.
     main_thread_only().task_queue_manager->selector_.EnableQueue(this);
   } else {
     if (!main_thread_only().delayed_incoming_queue.empty())
       main_thread_only().time_domain->CancelDelayedWork(this);
     main_thread_only().task_queue_manager->selector_.DisableQueue(this);
@@ skipping 56 matching lines @@
 
 void TaskQueueImpl::ScheduleDelayedWorkInTimeDomain(base::TimeTicks now) {
   if (!IsQueueEnabled())
     return;
   if (main_thread_only().delayed_incoming_queue.empty())
     return;
 
   main_thread_only().time_domain->ScheduleDelayedWork(
       this, main_thread_only().delayed_incoming_queue.top().delayed_wake_up(),
       now);
-
-  NotifyWakeUpChangedOnMainThread(

[alex clarke (OOO till 29th), 2017/05/09 12:50:50]

For my education, was this a bug since it looks like it would have overwritten
the immediate  time from TaskQueueImpl::EnableOrDisableWithSelector?

[altimin, 2017/05/09 13:15:04]

Yes, but strictly speaking this wasn't a real bug because task queue throttler
is the only user and it is resistant to extra notifications.

-      main_thread_only().delayed_incoming_queue.top().delayed_run_time);
 }
 
-void TaskQueueImpl::NotifyWakeUpChangedOnMainThread(base::TimeTicks wake_up) {
-  if (main_thread_only().observer)
-    main_thread_only().observer->OnQueueNextWakeUpChanged(this, wake_up);
+void TaskQueueImpl::SetScheduledTimeDomainWakeUp(
+    base::Optional<base::TimeTicks> scheduled_time_domain_wake_up) {
+  main_thread_only().scheduled_time_domain_wake_up =
+      scheduled_time_domain_wake_up;
+
+  if (!scheduled_time_domain_wake_up)

[alex clarke (OOO till 29th), 2017/05/09 12:50:50]

nit: Stylistically we tend to collapse all these onto one if

[altimin, 2017/05/09 13:15:03]

I think that in this case the conditions are different and they warrant separate
ifs.

[alex clarke (OOO till 29th), 2017/05/09 19:18:42]

One of the style guide rules is to be consistent with other code. Elsewhere in
the scheduler we tend to merge these into one line.  I don't see anything
special here that warrants them being on multiple lines. I don't think it helps
readability ether.

[altimin, 2017/05/10 10:42:25]

Done.

+    return;
+  if (!main_thread_only().observer)
+    return;
+  // If queue has immediate work an appropriate notification has already
+  // been issued.
+  if (HasPendingImmediateWork())
+    return;
+
+  main_thread_only().observer->OnQueueNextWakeUpChanged(
+      this, scheduled_time_domain_wake_up.value());
+}
+
+bool TaskQueueImpl::HasPendingImmediateWork() {
+  // Any work queue tasks count as immediate work.
+  if (!main_thread_only().delayed_work_queue->Empty() ||
+      !main_thread_only().immediate_work_queue->Empty()) {
+    return true;
+  }
+
+  // Finally tasks on |immediate_incoming_queue| count as immediate work.
+  base::AutoLock lock(immediate_incoming_queue_lock_);
+  return !immediate_incoming_queue().empty();
 }
 
 }  // namespace internal
 }  // namespace scheduler
 }  // namespace blink