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"
@@ skipping 38 matching lines @@
         mock_task_runner_(new cc::OrderedSimpleTaskRunner(clock_, false)),
         scheduler_(new RendererSchedulerImpl(mock_task_runner_)),
         default_task_runner_(scheduler_->DefaultTaskRunner()),
         compositor_task_runner_(scheduler_->CompositorTaskRunner()),
         loading_task_runner_(scheduler_->LoadingTaskRunner()),
         idle_task_runner_(scheduler_->IdleTaskRunner()) {
     scheduler_->SetTimeSourceForTesting(clock_);
   }
   ~RendererSchedulerImplTest() override {}
 
-  void RunUntilIdle() { mock_task_runner_->RunUntilIdle(); }
+  void TearDown() override {
+    // Check that all tests stop posting tasks.
+    while (RunUntilIdle()) {
+      clock_->AdvanceNow(base::TimeDelta::FromMilliseconds(100));

[Sami, 2015/03/11 16:58:25]

Thanks for adding this. By the way, would SetAutoAdvanceNowToPendingTasks
accomplish the same thing?

[alex clarke (OOO till 29th), 2015/03/12 13:41:32]

Seems so.

+    }
+  }
+
+  bool RunUntilIdle() { return mock_task_runner_->RunUntilIdle(); }
 
   void DoMainFrame() {
     scheduler_->WillBeginFrame(cc::BeginFrameArgs::Create(
         BEGINFRAME_FROM_HERE, clock_->Now(), base::TimeTicks(),
         base::TimeDelta::FromMilliseconds(1000), cc::BeginFrameArgs::NORMAL));
     scheduler_->DidCommitFrameToCompositor();
   }
 
   void EnableIdleTasks() { DoMainFrame(); }
 
   Policy CurrentPolicy() { return scheduler_->current_policy_; }
 
+  void ScheduleUrgentPolicyUpdate() {
+    base::AutoLock lock(scheduler_->incoming_signals_lock_);
+    scheduler_->ScheduleUrgentPolicyUpdate(FROM_HERE);
+  }
+
   // Helper for posting several tasks of specific types. |task_descriptor| is a
   // string with space delimited task identifiers. The first letter of each
   // task identifier specifies the task type:
   // - 'D': Default task
   // - 'C': Compositor task
   // - 'L': Loading task
   // - 'I': Idle task
   void PostTestTasks(std::vector<std::string>* run_order,
                      const std::string& task_descriptor) {
     std::istringstream stream(task_descriptor);
@@ skipping 758 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 UpdatePolicyLocked() override {

[Sami, 2015/03/11 16:58:25]

The way we have to reach into the internals to test this things makes me think
we should just pull the policy update mechanism out of the scheduler proper. I
don't have a better suggestion of how to test this without doing that. I guess a
little less invasive option would be to give the scheduler an closure/interface
which it would call whenever the policy updates. WDYT?

[alex clarke (OOO till 29th), 2015/03/12 13:41:32]

I think we should pick one style and stick to it.  But perhaps we should do that
after?

+    update_policy_count_++;
+    RendererSchedulerImpl::UpdatePolicyLocked();
+  }
+
+  int update_policy_count_;
+};
+
+TEST_F(RendererSchedulerImplTest, OnlyOnePendingNonDelayedUpdatePolicy) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+
+  ScheduleUrgentPolicyUpdate();
+  ScheduleUrgentPolicyUpdate();
+  ScheduleUrgentPolicyUpdate();
+  ScheduleUrgentPolicyUpdate();
+
+  RunUntilIdle();
+
+  EXPECT_EQ(1, mock_scheduler->update_policy_count_);
+}
+
+TEST_F(RendererSchedulerImplTest, UpdatePolicyCountTriggeredByOneInputEvent) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+  scheduler_->SetTimeSourceForTesting(clock_);
+  mock_task_runner_->SetAutoAdvanceNowToPendingTasks(true);
+
+  scheduler_->DidReceiveInputEventOnCompositorThread(
+      FakeInputEvent(blink::WebInputEvent::TouchStart));
+
+  RunUntilIdle();
+
+  EXPECT_EQ(2, mock_scheduler->update_policy_count_);

[Sami, 2015/03/11 16:58:25]

Could you add a comment saying which two updates these are?

[alex clarke (OOO till 29th), 2015/03/12 13:41:32]

Done.

+}
+
+TEST_F(RendererSchedulerImplTest, UpdatePolicyCountTriggeredByTwoInputEvents) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+  scheduler_->SetTimeSourceForTesting(clock_);
+  mock_task_runner_->SetAutoAdvanceNowToPendingTasks(true);
+
+  scheduler_->DidReceiveInputEventOnCompositorThread(
+      FakeInputEvent(blink::WebInputEvent::TouchStart));
+  scheduler_->DidReceiveInputEventOnCompositorThread(
+      FakeInputEvent(blink::WebInputEvent::TouchMove));
+
+  RunUntilIdle();
+
+  EXPECT_EQ(2, mock_scheduler->update_policy_count_);
+}
+
+TEST_F(RendererSchedulerImplTest, EnsureUpdatePolicyNotTriggeredTooOften) {
+  MockRendererSchedulerImpl* mock_scheduler =
+      new MockRendererSchedulerImpl(mock_task_runner_);
+  scheduler_.reset(mock_scheduler);
+  scheduler_->SetTimeSourceForTesting(clock_);
+  mock_task_runner_->SetAutoAdvanceNowToPendingTasks(true);
+
+  scheduler_->DidReceiveInputEventOnCompositorThread(
+      FakeInputEvent(blink::WebInputEvent::TouchStart));
+  scheduler_->DidReceiveInputEventOnCompositorThread(
+      FakeInputEvent(blink::WebInputEvent::TouchMove));
+
+  // We expect the first call to IsHighPriorityWorkAnticipated to be called
+  // after recieving an input event (but before the UpdateTask was processed) to
+  // call UpdatePolicy.
+  EXPECT_EQ(0, mock_scheduler->update_policy_count_);
+  scheduler_->IsHighPriorityWorkAnticipated();
+  EXPECT_EQ(1, mock_scheduler->update_policy_count_);
+  // Subsequent calls should not call UpdateTask.
+  scheduler_->IsHighPriorityWorkAnticipated();
+  scheduler_->IsHighPriorityWorkAnticipated();
+  scheduler_->IsHighPriorityWorkAnticipated();
+  scheduler_->ShouldYieldForHighPriorityWork();
+  scheduler_->ShouldYieldForHighPriorityWork();
+  scheduler_->ShouldYieldForHighPriorityWork();
+  scheduler_->ShouldYieldForHighPriorityWork();
+
+  EXPECT_EQ(1, mock_scheduler->update_policy_count_);
+
+  // There should be only two UpdatePolicy tasks posted.
+  RunUntilIdle();
+  EXPECT_EQ(3, mock_scheduler->update_policy_count_);
+}
+
 }  // namespace content