Prevent multiple pending UpdatePolicy tasks.

content/renderer/scheduler/renderer_scheduler_impl_unittest.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/renderer_scheduler_impl.h"
 
 #include "base/callback.h"
 #include "cc/output/begin_frame_args.h"
 #include "cc/test/ordered_simple_task_runner.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+
 namespace content {
 
 namespace {
 class FakeInputEvent : public blink::WebInputEvent {
  public:
   explicit FakeInputEvent(blink::WebInputEvent::Type event_type)
       : WebInputEvent(sizeof(FakeInputEvent)) {
     type = event_type;
   }
 
@@ skipping 76 matching lines @@
           idle_task_runner_->PostIdleTask(
               FROM_HERE,
               base::Bind(&AppendToVectorIdleTestTask, run_order, task));
           break;
         default:
           NOTREACHED();
       }
     }
   }
 
+  void PostUpdatePolicyOnControlRunner(base::TimeDelta delay) {
+    base::AutoLock lock(scheduler_->incoming_signals_lock_);
+    scheduler_->PostUpdatePolicyOnControlRunnerLocked(delay);
+  }
+
  protected:
   static base::TimeDelta priority_escalation_after_input_duration() {
     return base::TimeDelta::FromMilliseconds(
         RendererSchedulerImpl::kPriorityEscalationAfterInputMillis);
   }
 
   scoped_refptr<cc::TestNowSource> clock_;
   scoped_refptr<cc::OrderedSimpleTaskRunner> mock_task_runner_;
 
   scoped_ptr<RendererSchedulerImpl> scheduler_;
@@ skipping 721 matching lines @@
   RunUntilIdle();
   EXPECT_EQ(Policy::COMPOSITOR_PRIORITY, CurrentPolicy());
 
   // Process the input event with a new BeginMainFrame.
   DoMainFrame();
   clock_->AdvanceNow(2 * priority_escalation_after_input_duration());
   RunUntilIdle();
   EXPECT_EQ(Policy::NORMAL, CurrentPolicy());
 }
 
+class MockRendererSchedulerImpl : public RendererSchedulerImpl {
+ public:
+  MockRendererSchedulerImpl(
+      scoped_refptr<base::SingleThreadTaskRunner> main_task_runner)
+      : RendererSchedulerImpl(main_task_runner), update_policy_count_(0) {}
+
+  void UpdatePolicy(bool clear_pending_non_delayed_update_policy) override {
+    update_policy_count_++;
+    RendererSchedulerImpl::UpdatePolicy(
+        clear_pending_non_delayed_update_policy);
+  }
+
+  int update_policy_count_;
+};
+
+TEST_F(RendererSchedulerImplTest, OnlyOnePendingNonDelayedUpdatePolicy) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+
+  PostUpdatePolicyOnControlRunner(base::TimeDelta());

[Sami, 2015/03/10 16:19:38]

Do we know what triggered the bug in the first place? It would be good to have a
unit test that replicates the those circumstances.

[alex clarke (OOO till 29th), 2015/03/11 10:59:05]

Anything that calls MaybeUpdatePolicy can potentially cause a delayed task to
get posted if we are prioritizing input. These are the current vectors:

* UpdateForInputEvent -- but only if the input_stream_state_ changes
* IsHighPriorityWorkAnticipated
* ShouldYieldForHighPriorityWork


I've added a test which exercises these.

+  PostUpdatePolicyOnControlRunner(base::TimeDelta());
+  PostUpdatePolicyOnControlRunner(base::TimeDelta());
+  PostUpdatePolicyOnControlRunner(base::TimeDelta());
+
+  RunUntilIdle();
+
+  EXPECT_EQ(1, mock_scheduler->update_policy_count_);
+}
+
+TEST_F(RendererSchedulerImplTest, OnlyOnePendingDelayedUpdatePolicy) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+  scheduler_->SetTimeSourceForTesting(clock_);
+  mock_task_runner_->SetAutoAdvanceNowToPendingTasks(true);
+
+  PostUpdatePolicyOnControlRunner(base::TimeDelta::FromMilliseconds(1));
+  PostUpdatePolicyOnControlRunner(base::TimeDelta::FromMilliseconds(10));
+  PostUpdatePolicyOnControlRunner(base::TimeDelta::FromMilliseconds(100));
+  PostUpdatePolicyOnControlRunner(base::TimeDelta::FromMilliseconds(1000));
+
+  RunUntilIdle();
+
+  EXPECT_EQ(1, mock_scheduler->update_policy_count_);
+}
+
+TEST_F(RendererSchedulerImplTest,
+       OnePendingDelayedAndOneNonDelayedUpdatePolicy) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+  scheduler_->SetTimeSourceForTesting(clock_);
+  mock_task_runner_->SetAutoAdvanceNowToPendingTasks(true);
+
+  PostUpdatePolicyOnControlRunner(base::TimeDelta());
+  PostUpdatePolicyOnControlRunner(base::TimeDelta::FromMilliseconds(1));

[Sami, 2015/03/10 16:19:38]

Could you also try this the other way around to make sure the immediate policy
update doesn't cancel the delayed one?

[alex clarke (OOO till 29th), 2015/03/11 10:59:05]

It's now hard to test this directly.  If you're keen still I could probably add
a helper.

+
+  RunUntilIdle();
+
+  EXPECT_EQ(2, mock_scheduler->update_policy_count_);
+}
+
 }  // namespace content