cc: Implement deadine scheduling disabled by default

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index ea1f859699ee1ad99fd2db1670b7f231186ab319..4806b3014f8f933981538bc15140e3370e911157 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -4,6 +4,7 @@
 
 #include "cc/scheduler/scheduler.h"
 
+#include <algorithm>
 #include "base/auto_reset.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
@@ -17,16 +18,14 @@ Scheduler::Scheduler(SchedulerClient* client,
       client_(client),
       weak_factory_(this),
       last_set_needs_begin_frame_(false),
-      has_pending_begin_frame_(false),
+      begin_frame_did_not_swap_count_(0),
       state_machine_(scheduler_settings),
       inside_process_scheduled_actions_(false) {
   DCHECK(client_);
-  DCHECK(!state_machine_.BeginFrameNeededToDrawByImplThread());
+  DCHECK(!state_machine_.BeginFrameNeededByImplThread());
 }
 
-Scheduler::~Scheduler() {
-  client_->SetNeedsBeginFrameOnImplThread(false);
-}
+Scheduler::~Scheduler() {}
 
 void Scheduler::SetCanStart() {
   state_machine_.SetCanStart();
@@ -48,6 +47,12 @@ void Scheduler::NotifyReadyToActivate() {
   ProcessScheduledActions();
 }
 
+void Scheduler::ActivatePendingTree() {
+  client_->ScheduledActionActivatePendingTree();
+  if (state_machine_.ShouldTriggerBeginFrameDeadlineEarly())
+    PostBeginFrameDeadline(base::TimeTicks());
+}
+
 void Scheduler::SetNeedsCommit() {
   state_machine_.SetNeedsCommit();
   ProcessScheduledActions();
@@ -78,6 +83,9 @@ void Scheduler::FinishCommit() {
   TRACE_EVENT0("cc", "Scheduler::FinishCommit");
   state_machine_.FinishCommit();
   ProcessScheduledActions();
+
+  if (state_machine_.ShouldTriggerBeginFrameDeadlineEarly())
+    PostBeginFrameDeadline(base::TimeTicks());
 }
 
 void Scheduler::BeginFrameAbortedByMainThread(bool did_handle) {
@@ -88,15 +96,19 @@ void Scheduler::BeginFrameAbortedByMainThread(bool did_handle) {
 
 void Scheduler::DidLoseOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidLoseOutputSurface");
+  last_set_needs_begin_frame_ = false;
+  begin_frame_did_not_swap_count_ = 0;
+  begin_frame_deadline_closure_.Cancel();
   state_machine_.DidLoseOutputSurface();
   ProcessScheduledActions();
 }
 
 void Scheduler::DidCreateAndInitializeOutputSurface() {
   TRACE_EVENT0("cc", "Scheduler::DidCreateAndInitializeOutputSurface");
+  DCHECK(!last_set_needs_begin_frame_);
+  DCHECK_EQ(begin_frame_did_not_swap_count_, 0);
+  DCHECK(begin_frame_deadline_closure_.IsCancelled());
   state_machine_.DidCreateAndInitializeOutputSurface();
-  has_pending_begin_frame_ = false;
-  last_set_needs_begin_frame_ = false;
   ProcessScheduledActions();
 }
 
@@ -107,12 +119,11 @@ base::TimeTicks Scheduler::AnticipatedDrawTime() {
       last_begin_frame_args_.interval <= base::TimeDelta())
     return base::TimeTicks();
 
-  // TODO(brianderson): Express this in terms of the deadline.
   base::TimeTicks now = base::TimeTicks::Now();
-  int64 intervals = 1 + ((now - last_begin_frame_args_.frame_time) /
-                         last_begin_frame_args_.interval);
-  return last_begin_frame_args_.frame_time +
-      (last_begin_frame_args_.interval * intervals);
+  base::TimeTicks timebase = std::max(last_begin_frame_args_.frame_time,
+                                      last_begin_frame_args_.deadline);
+  int64 intervals = 1 + ((now - timebase) / last_begin_frame_args_.interval);
+  return timebase + (last_begin_frame_args_.interval * intervals);
 }
 
 base::TimeTicks Scheduler::LastBeginFrameOnImplThreadTime() {
@@ -120,43 +131,36 @@ base::TimeTicks Scheduler::LastBeginFrameOnImplThreadTime() {
 }
 
 void Scheduler::SetupNextBeginFrameIfNeeded() {
-  bool needs_begin_frame_to_draw =
-      state_machine_.BeginFrameNeededToDrawByImplThread();
-  // We want to avoid proactive begin frames with the synchronous compositor
-  // because every SetNeedsBeginFrame will force a redraw.
-  bool proactive_begin_frame_wanted =
-      state_machine_.ProactiveBeginFrameWantedByImplThread() &&
-      !settings_.using_synchronous_renderer_compositor &&
-      settings_.throttle_frame_production;
-  bool needs_begin_frame = needs_begin_frame_to_draw ||
-                           proactive_begin_frame_wanted;
-  bool immediate_disables_needed =
-      settings_.using_synchronous_renderer_compositor;
-
-  // Determine if we need BeginFrame notifications.
-  // If we do, always request the BeginFrame immediately.
-  // If not, only disable on the next BeginFrame to avoid unnecessary toggles.
-  // The synchronous renderer compositor requires immediate disables though.
-  if ((needs_begin_frame ||
-       state_machine_.inside_begin_frame() ||
-       immediate_disables_needed) &&
-      (needs_begin_frame != last_set_needs_begin_frame_)) {
-    has_pending_begin_frame_ = false;
-    client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
-    last_set_needs_begin_frame_ = needs_begin_frame;
+  bool needs_begin_frame =
+      state_machine_.BeginFrameNeededByImplThread();
+
+  bool should_call_set_needs_begin_frame =
+      // Always request the BeginFrame immediately if it wasn't needed before.
+      (needs_begin_frame && !last_set_needs_begin_frame_) ||
+      // We always need to explicitly request our next BeginFrame.
+      state_machine_.begin_frame_state() ==
+            SchedulerStateMachine::BEGIN_FRAME_STATE_INSIDE_DEADLINE;
+
+  // Only disable the BeginFrame after a BeginFrame where we didn't swap,

[enne (OOO), 2013/09/12 21:56:22]

Can you phrase this in the positive to match the fact that you're setting the
variable to true below? This is really saying that you only need another begin
frame if you swapped?  (And that's because when we don't swap, that's because
we're in a readback, and so we'll move forward and draw and swap again without a
begin frame?  Or, am I mistaken about why this is needed?)

[brianderson, 2013/09/12 22:42:54]

I will change the comment.

It's trying to keep needs_begin_frame_ true if we just swapped to avoid toggling
SetNeedsBeginFrame. Every toggle requires a message to the Browser compositor on
Android (and soon for other platforms), and may cause the next BeginFrame to be
skipped if the BrowserCompositor samples the value of SetNeedsBeginFrame at an
unfortunate time.

We may not have swapped for other reasons: mostly in cases where
ProactiveBeginFrameWanted requested a BeginFrame, but new content wasn't
actually ready yet.

+  // except for the synchronous renderer compositor, which wants immediate
+  // enables/disables to avoid unnecessary draws.
+  int negative_edge_glitch_threshold =
+      settings_.using_synchronous_renderer_compositor ? 0 : 1;

[enne (OOO), 2013/09/12 21:56:22]

This seems like it should really just be if
(!settings_.using_synchronous_renderer_compositor). 
begin_frame_did_not_swap_count is always >= 0, right?

+  if (begin_frame_did_not_swap_count_ < negative_edge_glitch_threshold &&

[enne (OOO), 2013/09/12 21:56:22]

bikeshed: Could you not put 'not' in a variable name? Maybe just
swap_count_since_last_begin_frame? Or, maybe even
has_swapped_since_last_begin_frame, since you're just checking against 1 here?

[brianderson, 2013/09/12 22:42:54]

That prevents us from using a larger threshold, but I guess we can change it
later if we need to. I think I can actually put this logic into
ProactiveBeginFrameNeeded, since the state machine should have all the state it
needs already. 

+      state_machine_.begin_frame_state() ==
+          SchedulerStateMachine::BEGIN_FRAME_STATE_INSIDE_DEADLINE) {
+    needs_begin_frame = true;
   }
 
-  // Request another BeginFrame if we haven't drawn for now until we have
-  // deadlines implemented.
-  if (state_machine_.inside_begin_frame() && has_pending_begin_frame_) {
-    has_pending_begin_frame_ = false;
-    client_->SetNeedsBeginFrameOnImplThread(true);
+  if (should_call_set_needs_begin_frame) {
+    client_->SetNeedsBeginFrameOnImplThread(needs_begin_frame);
+    last_set_needs_begin_frame_ = needs_begin_frame;
   }
 
   // Setup PollForAnticipatedDrawTriggers for cases where we want a proactive
   // BeginFrame but aren't requesting one.
-  if (!needs_begin_frame &&
-      state_machine_.ProactiveBeginFrameWantedByImplThread()) {
+  if (!last_set_needs_begin_frame_ &&
+      state_machine_.ShouldPollForAnticipatedDrawTriggers()) {
     if (poll_for_draw_triggers_closure_.IsCancelled()) {
       poll_for_draw_triggers_closure_.Reset(
           base::Bind(&Scheduler::PollForAnticipatedDrawTriggers,
@@ -173,12 +177,63 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
 
 void Scheduler::BeginFrame(const BeginFrameArgs& args) {
   TRACE_EVENT0("cc", "Scheduler::BeginFrame");
-  DCHECK(!has_pending_begin_frame_);
-  has_pending_begin_frame_ = true;
+  begin_frame_did_not_swap_count_++;
   last_begin_frame_args_ = args;
-  state_machine_.DidEnterBeginFrame(args);
+  last_begin_frame_args_.deadline -= client_->DrawDurationEstimate();
+  state_machine_.OnBeginFrame(last_begin_frame_args_);
+  ProcessScheduledActions();
+  state_machine_.OnBeginFrameDeadlinePending();
+
+  if (settings_.using_synchronous_renderer_compositor) {
+    // The synchronous renderer compositor has to make its GL calls
+    // within this call to BeginFrame.
+    // TODO(brianderson): Have the OutputSurface initiate the deadline tasks
+    // so the sychronous renderer compoistor can take advantage of splitting
+    // up the BeginFrame and deadline as well.
+    OnBeginFrameDeadline();
+  } else if (!settings_.deadline_scheduling_enabled) {
+    // We emulate the old non-deadline scheduler here by posting the
+    // deadline task without any delay.
+    PostBeginFrameDeadline(base::TimeTicks());
+  } else if (state_machine_.ShouldTriggerBeginFrameDeadlineEarly()) {
+    // We are ready to draw a new active tree immediately.
+    PostBeginFrameDeadline(base::TimeTicks());
+  } else if (state_machine_.needs_redraw()) {
+    // We have an animation or fast input path on the impl thread that wants
+    // to draw, so don't wait too long for a new active tree.
+    PostBeginFrameDeadline(last_begin_frame_args_.deadline);
+  } else {
+    // The impl thread doesn't have anything it wants to draw and we are just
+    // waiting for a new active tree, so post the deadline for the next
+    // expected BeginFrame start. This allows us to draw immediately when
+    // there is a new active tree, instead of waiting for the next BeginFrame.
+    // TODO(brianderson): Handle long deadlines (that are past the next frame's
+    // frame time) properly instead of using this hack.
+    PostBeginFrameDeadline(last_begin_frame_args_.frame_time +
+                           last_begin_frame_args_.interval);
+  }
+}
+
+void Scheduler::PostBeginFrameDeadline(base::TimeTicks deadline) {
+  begin_frame_deadline_closure_.Cancel();
+  begin_frame_deadline_closure_.Reset(
+      base::Bind(&Scheduler::OnBeginFrameDeadline, weak_factory_.GetWeakPtr()));
+  client_->PostBeginFrameDeadline(begin_frame_deadline_closure_.callback(),
+                                  deadline);
+}
+
+void Scheduler::OnBeginFrameDeadline() {
+  TRACE_EVENT0("cc", "Scheduler::OnBeginFrameDeadline");
+  begin_frame_deadline_closure_.Cancel();
+  state_machine_.OnBeginFrameDeadline();
   ProcessScheduledActions();
-  state_machine_.DidLeaveBeginFrame();
+  // We only transition out of BEGIN_FRAME_STATE_INSIDE_DEADLINE when all
+  // actions that occur back-to-back in response to entering
+  // BEGIN_FRAME_STATE_INSIDE_DEADLINE have completed. This is important
+  // because sending the BeginFrame to the main thread will not occur if
+  // we transition to BEGIN_FRAME_STATE_IDLE too early.
+  state_machine_.OnBeginFrameIdle();
+  client_->DidBeginFrameDeadlineOnImplThread();
 }
 
 void Scheduler::PollForAnticipatedDrawTriggers() {
@@ -191,10 +246,12 @@ void Scheduler::DrawAndSwapIfPossible() {
   DrawSwapReadbackResult result =
       client_->ScheduledActionDrawAndSwapIfPossible();
   state_machine_.DidDrawIfPossibleCompleted(result.did_draw);
+  begin_frame_did_not_swap_count_ = 0;
 }
 
 void Scheduler::DrawAndSwapForced() {
   client_->ScheduledActionDrawAndSwapForced();
+  begin_frame_did_not_swap_count_ = 0;
 }
 
 void Scheduler::DrawAndReadback() {
@@ -232,7 +289,7 @@ void Scheduler::ProcessScheduledActions() {
         client_->ScheduledActionUpdateVisibleTiles();
         break;
       case SchedulerStateMachine::ACTION_ACTIVATE_PENDING_TREE:
-        client_->ScheduledActionActivatePendingTree();
+        ActivatePendingTree();
         break;
       case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE:
         DrawAndSwapIfPossible();