Prevent multiple pending UpdatePolicy tasks.

content/renderer/scheduler/renderer_scheduler_impl_unittest.cc

Index: content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
diff --git a/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc b/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
index 4a97a0454749f11708d1fdb98d7f81e2202a5354..8c8e02211c450f6cd6c8f0480461bf4b3484b4c4 100644
--- a/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
+++ b/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
@@ -56,7 +56,14 @@ class RendererSchedulerImplTest : public testing::Test {
   }
   ~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(
@@ -69,6 +76,11 @@ class RendererSchedulerImplTest : public testing::Test {
 
   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:
@@ -847,4 +859,99 @@ TEST_F(RendererSchedulerImplTest, InputArrivesAfterBeginFrame) {
   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