Prevent multiple pending UpdatePolicy tasks.

content/renderer/scheduler/renderer_scheduler_impl.cc

Index: content/renderer/scheduler/renderer_scheduler_impl.cc
diff --git a/content/renderer/scheduler/renderer_scheduler_impl.cc b/content/renderer/scheduler/renderer_scheduler_impl.cc
index 6f7821690393a3b8c61b9956f28deb523e766a6b..1d4264df8b1ba2d5a2f4a2cd66d9a420af4f6448 100644
--- a/content/renderer/scheduler/renderer_scheduler_impl.cc
+++ b/content/renderer/scheduler/renderer_scheduler_impl.cc
@@ -31,6 +31,7 @@ RendererSchedulerImpl::RendererSchedulerImpl(
           task_queue_manager_->TaskRunnerForQueue(COMPOSITOR_TASK_QUEUE)),
       loading_task_runner_(
           task_queue_manager_->TaskRunnerForQueue(LOADING_TASK_QUEUE)),
+      pending_non_delayed_update_policy_(false),
       current_policy_(Policy::NORMAL),
       last_input_type_(blink::WebInputEvent::Undefined),
       input_stream_state_(InputStreamState::INACTIVE),
@@ -38,9 +39,12 @@ RendererSchedulerImpl::RendererSchedulerImpl(
       weak_factory_(this) {
   weak_renderer_scheduler_ptr_ = weak_factory_.GetWeakPtr();
   update_policy_closure_ = base::Bind(&RendererSchedulerImpl::UpdatePolicy,
-                                      weak_renderer_scheduler_ptr_);
+                                      weak_renderer_scheduler_ptr_, true);
   end_idle_period_closure_.Reset(base::Bind(
       &RendererSchedulerImpl::EndIdlePeriod, weak_renderer_scheduler_ptr_));
+  delayed_update_policy_closure_.Reset(
+      base::Bind(&RendererSchedulerImpl::UpdatePolicy,
+                 weak_renderer_scheduler_ptr_, false));
   idle_task_runner_ = make_scoped_refptr(new SingleThreadIdleTaskRunner(
       task_queue_manager_->TaskRunnerForQueue(IDLE_TASK_QUEUE),
       control_task_after_wakeup_runner_,
@@ -186,7 +190,7 @@ void RendererSchedulerImpl::UpdateForInputEvent(
     // Update scheduler policy if we should start a new policy mode.
     input_stream_state_ = new_input_stream_state;
     policy_may_need_update_.SetWhileLocked(true);
-    PostUpdatePolicyOnControlRunner(base::TimeDelta());
+    PostUpdatePolicyOnControlRunnerLocked(base::TimeDelta());
   }
   last_input_receipt_time_on_compositor_ = Now();
   // Clear the last known input processing time so that we know an input event
@@ -264,17 +268,30 @@ RendererSchedulerImpl::Policy RendererSchedulerImpl::SchedulerPolicy() const {
 void RendererSchedulerImpl::MaybeUpdatePolicy() {
   DCHECK(main_thread_checker_.CalledOnValidThread());
   if (policy_may_need_update_.IsSet()) {
-    UpdatePolicy();
+    UpdatePolicy(false);
   }
 }
 
-void RendererSchedulerImpl::PostUpdatePolicyOnControlRunner(
+void RendererSchedulerImpl::PostUpdatePolicyOnControlRunnerLocked(
     base::TimeDelta delay) {
-  control_task_runner_->PostDelayedTask(
-      FROM_HERE, update_policy_closure_, delay);
+  incoming_signals_lock_.AssertAcquired();
+  if (delay > base::TimeDelta()) {
+    // Only allow one pending delayed UpdatePolicy task.
+    delayed_update_policy_closure_.Cancel();
+    control_task_runner_->PostDelayedTask(
+        FROM_HERE, delayed_update_policy_closure_.callback(), delay);
+  } else {
+    // Only allow one pending non-delayed UpdatePolicy task.
+    if (!pending_non_delayed_update_policy_) {
+      pending_non_delayed_update_policy_ = true;
+      control_task_runner_->PostDelayedTask(FROM_HERE, update_policy_closure_,
+                                            base::TimeDelta());
+    }
+  }
 }
 
-void RendererSchedulerImpl::UpdatePolicy() {
+void RendererSchedulerImpl::UpdatePolicy(
+    bool clear_pending_non_delayed_update_policy) {
   DCHECK(main_thread_checker_.CalledOnValidThread());
   if (!task_queue_manager_)
     return;
@@ -282,11 +299,13 @@ void RendererSchedulerImpl::UpdatePolicy() {
   base::AutoLock lock(incoming_signals_lock_);
   base::TimeTicks now;
   policy_may_need_update_.SetWhileLocked(false);
+  if (clear_pending_non_delayed_update_policy)
+    pending_non_delayed_update_policy_ = false;

[rmcilroy, 2015/03/10 16:08:39]

I don't think the pending_non_delayed_update_policy_ changes are really
required. I don't think we have a big issue with multiple non-delayed
UpdatePolicy tasks (they typically check other fields and only repost if they
really want to update the policy). Maybe we could just make
PostUpdatePolicyOnControlRunner not take a delay (only post now), and have the
UpdatePolicy function always cancel any pending delayed updatepolicy tasks and
repost a new delayed one.  I think this would end up being a fair bit simpler to
understand, WDYT?

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

Yeah, I'm all for simplicity too. I was thinking we could just keep track of
when the next policy update is due to run and act accordingly here, but I think
that's equivalent to what you are suggesting.

[alex clarke (OOO till 29th), 2015/03/10 16:21:41]

Currently they only get sent when the input_stream_state_ changes (i.e. you're
right they're not a problem at the moment). 

Certinly your suggestion would simplify things a bit, however we've had a number
of bugs when DoWork and now UpdatePolicy where posted more frequently than
anticipated. I would argue that in light of that defensive programming is a good
idea.

 
   base::TimeDelta new_policy_duration;
   Policy new_policy = ComputeNewPolicy(&new_policy_duration);
   if (new_policy_duration > base::TimeDelta())
-    PostUpdatePolicyOnControlRunner(new_policy_duration);
+    PostUpdatePolicyOnControlRunnerLocked(new_policy_duration);
 
   if (new_policy == current_policy_)
     return;