Fix a bug with throttling and timer queue suspension

components/scheduler/renderer/renderer_scheduler_impl.cc

 // Copyright 2014 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 "components/scheduler/renderer/renderer_scheduler_impl.h"
 
 #include "base/bind.h"
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
@@ skipping 181 matching lines @@
 scoped_refptr<TaskQueue> RendererSchedulerImpl::NewLoadingTaskRunner(
     const char* name) {
   helper_.CheckOnValidThread();
   scoped_refptr<TaskQueue> loading_task_queue(helper_.NewTaskQueue(
       TaskQueue::Spec(name).SetShouldMonitorQuiescence(true)));
   loading_task_runners_.insert(loading_task_queue);
   loading_task_queue->SetQueueEnabled(
       MainThreadOnly().current_policy.loading_queue_policy.is_enabled);
   loading_task_queue->SetQueuePriority(
       MainThreadOnly().current_policy.loading_queue_policy.priority);
+  if (MainThreadOnly().current_policy.loading_queue_policy.time_domain_type ==
+      TimeDomainType::THROTTLED) {
+    throttling_helper_->IncreaseThrottleRefCount(loading_task_queue.get());
+  }
   loading_task_queue->AddTaskObserver(
       &MainThreadOnly().loading_task_cost_estimator);
   return loading_task_queue;
 }
 
 scoped_refptr<TaskQueue> RendererSchedulerImpl::NewTimerTaskRunner(
     const char* name) {
   helper_.CheckOnValidThread();
   scoped_refptr<TaskQueue> timer_task_queue(
       helper_.NewTaskQueue(TaskQueue::Spec(name)
                                .SetShouldMonitorQuiescence(true)
                                .SetShouldReportWhenExecutionBlocked(true)));
   timer_task_runners_.insert(timer_task_queue);
   timer_task_queue->SetQueueEnabled(
       MainThreadOnly().current_policy.timer_queue_policy.is_enabled);
   timer_task_queue->SetQueuePriority(
       MainThreadOnly().current_policy.timer_queue_policy.priority);
+  if (MainThreadOnly().current_policy.timer_queue_policy.time_domain_type ==
+      TimeDomainType::THROTTLED) {
+    throttling_helper_->IncreaseThrottleRefCount(timer_task_queue.get());

[Sami, 2016/05/31 18:11:04]

Good catch with these two.

[alex clarke (OOO till 29th), 2016/06/02 13:51:05]

Acknowledged.

+  }
   timer_task_queue->AddTaskObserver(
       &MainThreadOnly().timer_task_cost_estimator);
   return timer_task_queue;
 }
 
 std::unique_ptr<RenderWidgetSchedulingState>
 RendererSchedulerImpl::NewRenderWidgetSchedulingState() {
   return render_widget_scheduler_signals_.NewRenderWidgetSchedulingState();
 }
 
@@ skipping 594 matching lines @@
             TimeDomainType::THROTTLED;
       }
       break;
   }
 
   MainThreadOnly().expensive_task_policy = expensive_task_policy;
 
   if (MainThreadOnly().timer_queue_suspend_count != 0 ||
       MainThreadOnly().timer_queue_suspended_when_backgrounded) {
     new_policy.timer_queue_policy.is_enabled = false;
+    new_policy.timer_queue_policy.time_domain_type = TimeDomainType::REAL;
   }
 
   // Tracing is done before the early out check, because it's quite possible we
   // will otherwise miss this information in traces.
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(
       TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"), "RendererScheduler",
       this, AsValueLocked(now));
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"), "use_case",
                  use_case);
   TRACE_COUNTER1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
@@ skipping 40 matching lines @@
                        new_policy.default_queue_policy);
 
   DCHECK(compositor_task_runner_->IsQueueEnabled());
   MainThreadOnly().current_policy = new_policy;
 }
 
 void RendererSchedulerImpl::ApplyTaskQueuePolicy(
     TaskQueue* task_queue,
     const TaskQueuePolicy& old_task_queue_policy,
     const TaskQueuePolicy& new_task_queue_policy) const {
-  // The ThrottlingHelper also calls SetQueueEnabled, so we can avoid calling
-  // this here.
-  if (new_task_queue_policy.time_domain_type != TimeDomainType::THROTTLED)
-    task_queue->SetQueueEnabled(new_task_queue_policy.is_enabled);
+  if (old_task_queue_policy.is_enabled != new_task_queue_policy.is_enabled) {
+    throttling_helper_->SetQueueEnabled(task_queue,
+                                        new_task_queue_policy.is_enabled);
+  }
+
   if (old_task_queue_policy.priority != new_task_queue_policy.priority)
     task_queue->SetQueuePriority(new_task_queue_policy.priority);
 
+  // if (new_task_queue_policy.time_domain_type != TimeDomainType::THROTTLED)

[Sami, 2016/05/31 18:11:04]

Debugging left in by mistake?

[alex clarke (OOO till 29th), 2016/06/02 13:51:05]

Done.

+  //   task_queue->SetQueueEnabled(new_task_queue_policy.is_enabled);
   if (old_task_queue_policy.time_domain_type !=
       new_task_queue_policy.time_domain_type) {
     if (new_task_queue_policy.time_domain_type == TimeDomainType::THROTTLED) {
       throttling_helper_->IncreaseThrottleRefCount(task_queue);
     } else if (old_task_queue_policy.time_domain_type ==
                TimeDomainType::THROTTLED) {
       throttling_helper_->DecreaseThrottleRefCount(task_queue);
     }
   }
 }
@@ skipping 104 matching lines @@
 }
 
 IdleTimeEstimator* RendererSchedulerImpl::GetIdleTimeEstimatorForTesting() {
   return &MainThreadOnly().idle_time_estimator;
 }
 
 void RendererSchedulerImpl::SuspendTimerQueue() {
   MainThreadOnly().timer_queue_suspend_count++;
   ForceUpdatePolicy();
 #ifndef NDEBUG
-  DCHECK(!default_timer_task_runner_->IsQueueEnabled());
-  for (const auto& runner : timer_task_runners_) {
-    DCHECK(!runner->IsQueueEnabled());
-  }
+// DCHECK(!default_timer_task_runner_->IsQueueEnabled());

[Sami, 2016/05/31 18:11:04]

Ditto.

[alex clarke (OOO till 29th), 2016/06/02 13:51:05]

Done.

+// for (const auto& runner : timer_task_runners_) {
+//    DCHECK(!runner->IsQueueEnabled());
+// }
 #endif
 }
 
 void RendererSchedulerImpl::ResumeTimerQueue() {
   MainThreadOnly().timer_queue_suspend_count--;
   DCHECK_GE(MainThreadOnly().timer_queue_suspend_count, 0);
   ForceUpdatePolicy();
 }
 
 void RendererSchedulerImpl::SetTimerQueueSuspensionWhenBackgroundedEnabled(
@@ skipping 286 matching lines @@
     BroadcastConsoleWarning(
         "Blink deferred a task in order to make scrolling smoother. "
         "Your timer and network tasks should take less than 50ms to run "
         "to avoid this. Please see "
         "https://developers.google.com/web/tools/chrome-devtools/profile/evaluate-performance/rail"
         " and https://crbug.com/574343#c40 for more information.");
   }
 }
 
 }  // namespace scheduler