[content]: Cancel in-flight EndIdlePeriod tasks in Blink Scheduler.

The scheduler will post a delayed EndIdlePeriod task to end the idle period
at the expected time of the next frame draw. If the next WillBeginFrame
is called before this task has been run, then it is possible for the task
to end the next frames idle period. Prevent this by cancelling the in-flight
tasks when EndIdlePeriod is called.

BUG=447478

Review URL: https://codereview.chromium.org/847903004

Cr-Commit-Position: refs/heads/master@{#313038}

content/content_renderer.gypi

@@ -355,6 +355,8 @@
       'renderer/sad_plugin.h',
       'renderer/savable_resources.cc',
       'renderer/savable_resources.h',
+      'renderer/scheduler/cancelable_closure_holder.cc',
+      'renderer/scheduler/cancelable_closure_holder.h',
       'renderer/scheduler/null_renderer_scheduler.cc',
       'renderer/scheduler/null_renderer_scheduler.h',
       'renderer/scheduler/renderer_scheduler.cc',

content/renderer/scheduler/cancelable_closure_holder.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 "content/renderer/scheduler/cancelable_closure_holder.h"
+
+namespace content {
+
+CancelableClosureHolder::CancelableClosureHolder() {
+}
+
+CancelableClosureHolder::~CancelableClosureHolder() {
+}
+
+void CancelableClosureHolder::Reset(const base::Closure& callback) {
+  callback_ = callback;
+  cancelable_callback_.Reset(callback_);
+}
+
+void CancelableClosureHolder::Cancel() {
+  DCHECK(!callback_.is_null());
+  cancelable_callback_.Reset(callback_);
+}
+
+const base::Closure& CancelableClosureHolder::callback() const {
+  DCHECK(!callback_.is_null());
+  return cancelable_callback_.callback();
+}
+
+}  // namespace content

content/renderer/scheduler/cancelable_closure_holder.h

+// 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.
+
+#ifndef CONTENT_RENDERER_SCHEDULER_CANCELABLE_CLOSURE_HOLDER_H_
+#define CONTENT_RENDERER_SCHEDULER_CANCELABLE_CLOSURE_HOLDER_H_
+
+#include "base/cancelable_callback.h"
+
+namespace content {
+
+// A CancelableClosureHolder is a CancelableCallback which resets its wrapped
+// callback with a cached closure whenever it is canceled.
+class CancelableClosureHolder {
+ public:
+  CancelableClosureHolder();
+  ~CancelableClosureHolder();
+
+  // Resets the closure to be wrapped by the cancelable callback.  Cancels any
+  // outstanding callbacks.
+  void Reset(const base::Closure& callback);
+
+  // Cancels any outstanding closures returned by callback().
+  void Cancel();
+
+  // Returns a callback that will be disabled by calling Cancel(). Callback
+  // must have been set using Reset() before calling this function.
+  const base::Closure& callback() const;
+
+ private:
+  base::Closure callback_;
+  base::CancelableClosure cancelable_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(CancelableClosureHolder);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_SCHEDULER_CANCELABLE_CLOSURE_HOLDER_H_

content/renderer/scheduler/renderer_scheduler_impl.cc

@@ -35,8 +35,8 @@ RendererSchedulerImpl::RendererSchedulerImpl(
   weak_renderer_scheduler_ptr_ = weak_factory_.GetWeakPtr();
   update_policy_closure_ = base::Bind(&RendererSchedulerImpl::UpdatePolicy,
                                       weak_renderer_scheduler_ptr_);
-  end_idle_period_closure_ = base::Bind(&RendererSchedulerImpl::EndIdlePeriod,
-                                        weak_renderer_scheduler_ptr_);
+  end_idle_period_closure_.Reset(base::Bind(
+      &RendererSchedulerImpl::EndIdlePeriod, weak_renderer_scheduler_ptr_));
   idle_task_runner_ = make_scoped_refptr(new SingleThreadIdleTaskRunner(
       task_queue_manager_->TaskRunnerForQueue(IDLE_TASK_QUEUE),
       base::Bind(&RendererSchedulerImpl::CurrentIdleTaskDeadlineCallback,
@@ -111,7 +111,8 @@ void RendererSchedulerImpl::DidCommitFrameToCompositor() {
   base::TimeTicks now(Now());
   if (now < estimated_next_frame_begin_) {
     StartIdlePeriod();
-    control_task_runner_->PostDelayedTask(FROM_HERE, end_idle_period_closure_,
+    control_task_runner_->PostDelayedTask(FROM_HERE,
+                                          end_idle_period_closure_.callback(),
                                           estimated_next_frame_begin_ - now);
   }
 }
@@ -178,8 +179,8 @@ void RendererSchedulerImpl::MaybeUpdatePolicy() {
 
 void RendererSchedulerImpl::PostUpdatePolicyOnControlRunner(
     base::TimeDelta delay) {
-  control_task_runner_->PostDelayedTask(FROM_HERE, update_policy_closure_,
-                                        delay);
+  control_task_runner_->PostDelayedTask(
+      FROM_HERE, update_policy_closure_, delay);
 }
 
 void RendererSchedulerImpl::UpdatePolicy() {
@@ -250,6 +251,7 @@ void RendererSchedulerImpl::EndIdlePeriod() {
   TRACE_EVENT_ASYNC_END0("renderer.scheduler",
                          "RendererSchedulerIdlePeriod", this);
   main_thread_checker_.CalledOnValidThread();
+  end_idle_period_closure_.Cancel();
   renderer_task_queue_selector_->DisableQueue(IDLE_TASK_QUEUE);
 }
 

content/renderer/scheduler/renderer_scheduler_impl.h

@@ -8,6 +8,7 @@
 #include "base/atomicops.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread_checker.h"
+#include "content/renderer/scheduler/cancelable_closure_holder.h"
 #include "content/renderer/scheduler/renderer_scheduler.h"
 #include "content/renderer/scheduler/single_thread_idle_task_runner.h"
 #include "content/renderer/scheduler/task_queue_manager.h"
@@ -125,7 +126,7 @@ class CONTENT_EXPORT RendererSchedulerImpl : public RendererScheduler {
   scoped_refptr<SingleThreadIdleTaskRunner> idle_task_runner_;
 
   base::Closure update_policy_closure_;
-  base::Closure end_idle_period_closure_;
+  CancelableClosureHolder end_idle_period_closure_;
 
   // Don't access current_policy_ directly, instead use SchedulerPolicy().
   Policy current_policy_;

content/renderer/scheduler/renderer_scheduler_impl_unittest.cc

@@ -256,6 +256,43 @@ TEST_F(RendererSchedulerImplTest, TestIdleTaskExceedsDeadline) {
   EXPECT_EQ(2, run_count);
 }
 
+TEST_F(RendererSchedulerImplTest, TestDelayedEndIdlePeriodCanceled) {
+  bool task_run = false;
+
+  base::TimeTicks deadline_in_task;
+  idle_task_runner_->PostIdleTask(
+      FROM_HERE, base::Bind(&IdleTestTask, &task_run, &deadline_in_task));
+
+  // Trigger the beginning of an idle period for 1000ms.
+  scheduler_->WillBeginFrame(cc::BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, clock_->Now(), base::TimeTicks(),
+      base::TimeDelta::FromMilliseconds(1000), cc::BeginFrameArgs::NORMAL));
+  scheduler_->DidCommitFrameToCompositor();
+
+  // End the idle period early (after 500ms), and send a WillBeginFrame which
+  // specifies that the next idle period should end 1000ms from now.
+  clock_->AdvanceNow(base::TimeDelta::FromMilliseconds(500));
+  scheduler_->WillBeginFrame(cc::BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, clock_->Now(), base::TimeTicks(),
+      base::TimeDelta::FromMilliseconds(1000), cc::BeginFrameArgs::NORMAL));
+
+  RunUntilIdle();
+  EXPECT_FALSE(task_run);  // Not currently in an idle period.
+
+  // Trigger the start of the idle period before the task to end the previous
+  // idle period has been triggered.
+  clock_->AdvanceNow(base::TimeDelta::FromMilliseconds(400));
+  scheduler_->DidCommitFrameToCompositor();
+
+  // Post a task which simulates running until after the previous end idle
+  // period delayed task was scheduled for
+  scheduler_->DefaultTaskRunner()->PostTask(FROM_HERE, base::Bind(NullTask));
+  clock_->AdvanceNow(base::TimeDelta::FromMilliseconds(300));
+
+  RunUntilIdle();
+  EXPECT_TRUE(task_run);  // We should still be in the new idle period.
+}
+
 TEST_F(RendererSchedulerImplTest, TestDefaultPolicy) {
   std::vector<std::string> order;