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..d4656f5237186494fb7520f3dfcaf57516ad9a76 100644
--- a/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
+++ b/content/renderer/scheduler/renderer_scheduler_impl_unittest.cc
@@ -10,6 +10,8 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using testing::_;
+
 namespace content {
 
 namespace {
@@ -106,6 +108,11 @@ class RendererSchedulerImplTest : public testing::Test {
     }
   }
 
+  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(
@@ -847,4 +854,67 @@ 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 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