cc: Make scheduling be driven by vsync for android webview.

cc/scheduler/scheduler.cc

Index: cc/scheduler/scheduler.cc
diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc
index 3e7653f086b205ad5a3d1206447560f2c174af9b..6861024af17523d14f9776fe1cb3ba876ddc7a5c 100644
--- a/cc/scheduler/scheduler.cc
+++ b/cc/scheduler/scheduler.cc
@@ -106,8 +106,6 @@ Scheduler::Scheduler(
       base::Bind(&Scheduler::BeginRetroFrame, weak_factory_.GetWeakPtr());
   begin_impl_frame_deadline_closure_ = base::Bind(
       &Scheduler::OnBeginImplFrameDeadline, weak_factory_.GetWeakPtr());
-  poll_for_draw_triggers_closure_ = base::Bind(
-      &Scheduler::PollForAnticipatedDrawTriggers, weak_factory_.GetWeakPtr());
   advance_commit_state_closure_ = base::Bind(
       &Scheduler::PollToAdvanceCommitState, weak_factory_.GetWeakPtr());
 
@@ -219,6 +217,11 @@ void Scheduler::SetNeedsRedraw() {
   ProcessScheduledActions();
 }
 
+void Scheduler::OutputSurfaceDidRequestDraw() {
+  state_machine_.OutputSurfaceDidRequestDraw();
+  ProcessScheduledActions();
+}
+
 void Scheduler::SetNeedsAnimate() {
   state_machine_.SetNeedsAnimate();
   ProcessScheduledActions();
@@ -340,38 +343,15 @@ void Scheduler::SetupNextBeginFrameIfNeeded() {
 // We may need to poll when we can't rely on BeginFrame to advance certain
 // state or to avoid deadlock.
 void Scheduler::SetupPollingMechanisms(bool needs_begin_frame) {
-  bool needs_advance_commit_state_timer = false;
-  // Setup PollForAnticipatedDrawTriggers if we need to monitor state but
-  // aren't expecting any more BeginFrames. This should only be needed by
-  // the synchronous compositor when BeginFrameNeeded is false.
-  if (state_machine_.ShouldPollForAnticipatedDrawTriggers()) {
-    DCHECK(!state_machine_.SupportsProactiveBeginFrame());
-    DCHECK(!needs_begin_frame);
-    if (poll_for_draw_triggers_task_.IsCancelled()) {
-      poll_for_draw_triggers_task_.Reset(poll_for_draw_triggers_closure_);
-      base::TimeDelta delay = begin_impl_frame_args_.IsValid()
-                                  ? begin_impl_frame_args_.interval
-                                  : BeginFrameArgs::DefaultInterval();
-      task_runner_->PostDelayedTask(
-          FROM_HERE, poll_for_draw_triggers_task_.callback(), delay);
-    }
-  } else {
-    poll_for_draw_triggers_task_.Cancel();
-
-    // At this point we'd prefer to advance through the commit flow by
-    // drawing a frame, however it's possible that the frame rate controller
-    // will not give us a BeginFrame until the commit completes.  See
-    // crbug.com/317430 for an example of a swap ack being held on commit. Thus
-    // we set a repeating timer to poll on ProcessScheduledActions until we
-    // successfully reach BeginFrame. Synchronous compositor does not use
-    // frame rate controller or have the circular wait in the bug.
-    if (IsBeginMainFrameSentOrStarted() &&
-        !settings_.using_synchronous_renderer_compositor) {
-      needs_advance_commit_state_timer = true;
-    }
-  }
-
-  if (needs_advance_commit_state_timer) {
+  // At this point we'd prefer to advance through the commit flow by
+  // drawing a frame, however it's possible that the frame rate controller

[brianderson, 2014/12/20 00:34:13]

frame rate controller doesn't exist anymore. Since you are touching this
comment, might as well update it so it refers to the BeginFrameSource instead.

Note: I notice the advance_commit_state_task_ isn't actually needed anywhere at
ToT, because the SytheticBeginFrame source won't get blocked like the frame rate
controller used to. However, once Windows switches to the Browser as the
BeginFrame source, we could have the same deadlock issues - so we sadly can't
get rid of all polling. So close.

[sunnyps, 2014/12/20 00:39:30]

Acknowledged.

+  // will not give us a BeginFrame until the commit completes.  See
+  // crbug.com/317430 for an example of a swap ack being held on commit. Thus
+  // we set a repeating timer to poll on ProcessScheduledActions until we
+  // successfully reach BeginFrame. Synchronous compositor does not use
+  // frame rate controller or have the circular wait in the bug.
+  if (IsBeginMainFrameSentOrStarted() && 
+      !settings_.using_synchronous_renderer_compositor) {
     if (advance_commit_state_task_.IsCancelled() &&
         begin_impl_frame_args_.IsValid()) {
       // Since we'd rather get a BeginImplFrame by the normal mechanism, we
@@ -560,22 +540,10 @@ void Scheduler::BeginImplFrame(const BeginFrameArgs& args) {
 
   state_machine_.OnBeginImplFrameDeadlinePending();
 
-  if (settings_.using_synchronous_renderer_compositor) {
-    // The synchronous renderer compositor has to make its GL calls
-    // within this call.
-    // TODO(brianderson): Have the OutputSurface initiate the deadline tasks
-    // so the synchronous renderer compositor can take advantage of splitting
-    // up the BeginImplFrame and deadline as well.
-    OnBeginImplFrameDeadline();
-  } else {
-    ScheduleBeginImplFrameDeadline();
-  }
+  ProcessScheduledActions();

[brianderson, 2014/12/20 00:34:13]

Nice.

 }
 
 void Scheduler::ScheduleBeginImplFrameDeadline() {
-  // The synchronous compositor does not post a deadline task.
-  DCHECK(!settings_.using_synchronous_renderer_compositor);
-
   begin_impl_frame_deadline_task_.Cancel();
   begin_impl_frame_deadline_task_.Reset(begin_impl_frame_deadline_closure_);
 
@@ -584,7 +552,12 @@ void Scheduler::ScheduleBeginImplFrameDeadline() {
 
   base::TimeTicks deadline;
   switch (begin_impl_frame_deadline_mode_) {
-    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE:
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE_SYNCHRONOUS:
+      // The synchronous renderer compositor has to make its GL calls
+      // within this call.
+      OnBeginImplFrameDeadline();
+      return;
+    case SchedulerStateMachine::BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE_ASYNCHRONOUS:
       // We are ready to draw a new active tree immediately.
       // We don't use Now() here because it's somewhat expensive to call.
       deadline = base::TimeTicks();
@@ -602,27 +575,30 @@ void Scheduler::ScheduleBeginImplFrameDeadline() {
       break;
   }
 
-  TRACE_EVENT1(
-      "cc", "Scheduler::ScheduleBeginImplFrameDeadline", "deadline", deadline);
-
-  base::TimeDelta delta = deadline - Now();
-  if (delta <= base::TimeDelta())
-    delta = base::TimeDelta();
+  TRACE_EVENT2(
+      "cc", 
+      "Scheduler::ScheduleBeginImplFrameDeadline",
+      "mode", 
+      SchedulerStateMachine::BeginImplFrameDeadlineModeToString(
+        begin_impl_frame_deadline_mode_),
+      "deadline",
+      deadline);
+
+  base::TimeDelta delta = std::max(deadline - Now(), base::TimeDelta());
   task_runner_->PostDelayedTask(
       FROM_HERE, begin_impl_frame_deadline_task_.callback(), delta);
 }
 
-void Scheduler::RescheduleBeginImplFrameDeadlineIfNeeded() {
-  if (settings_.using_synchronous_renderer_compositor)
-    return;
-
+void Scheduler::ScheduleBeginImplFrameDeadlineIfNeeded() {
   if (state_machine_.begin_impl_frame_state() !=
       SchedulerStateMachine::BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME)
     return;
 
-  if (begin_impl_frame_deadline_mode_ !=
-      state_machine_.CurrentBeginImplFrameDeadlineMode())
-    ScheduleBeginImplFrameDeadline();
+  if (begin_impl_frame_deadline_mode_ == state_machine_.CurrentBeginImplFrameDeadlineMode() &&
+      BeginImplFrameDeadlinePending())
+    return;
+
+  ScheduleBeginImplFrameDeadline();
 }
 
 void Scheduler::OnBeginImplFrameDeadline() {
@@ -643,13 +619,6 @@ void Scheduler::OnBeginImplFrameDeadline() {
   client_->DidBeginImplFrameDeadline();
 }
 
-void Scheduler::PollForAnticipatedDrawTriggers() {
-  TRACE_EVENT0("cc", "Scheduler::PollForAnticipatedDrawTriggers");
-  poll_for_draw_triggers_task_.Cancel();
-  state_machine_.DidEnterPollForAnticipatedDrawTriggers();
-  ProcessScheduledActions();
-  state_machine_.DidLeavePollForAnticipatedDrawTriggers();
-}
 
 void Scheduler::PollToAdvanceCommitState() {
   TRACE_EVENT0("cc", "Scheduler::PollToAdvanceCommitState");
@@ -667,60 +636,66 @@ void Scheduler::ProcessScheduledActions() {
   // The top-level call will iteratively execute the next action for us anyway.
   if (inside_process_scheduled_actions_)
     return;
-
-  base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
-
-  SchedulerStateMachine::Action action;
-  do {
-    action = state_machine_.NextAction();
-    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
-                 "SchedulerStateMachine",
-                 "state",
-                 AsValue());
-    VLOG(2) << "Scheduler::ProcessScheduledActions: "
-            << SchedulerStateMachine::ActionToString(action) << " "
-            << state_machine_.GetStatesForDebugging();
-    state_machine_.UpdateState(action);
-    base::AutoReset<SchedulerStateMachine::Action>
-        mark_inside_action(&inside_action_, action);
-    switch (action) {
-      case SchedulerStateMachine::ACTION_NONE:
-        break;
-      case SchedulerStateMachine::ACTION_ANIMATE:
-        client_->ScheduledActionAnimate();
-        break;
-      case SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME:
-        client_->ScheduledActionSendBeginMainFrame();
-        break;
-      case SchedulerStateMachine::ACTION_COMMIT:
-        client_->ScheduledActionCommit();
-        break;
-      case SchedulerStateMachine::ACTION_ACTIVATE_SYNC_TREE:
-        client_->ScheduledActionActivateSyncTree();
-        break;
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE:
-        DrawAndSwapIfPossible();
-        break;
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED:
-        client_->ScheduledActionDrawAndSwapForced();
-        break;
-      case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT:
-        // No action is actually performed, but this allows the state machine to
-        // advance out of its waiting to draw state without actually drawing.
-        break;
-      case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
-        client_->ScheduledActionBeginOutputSurfaceCreation();
-        break;
-      case SchedulerStateMachine::ACTION_PREPARE_TILES:
-        client_->ScheduledActionPrepareTiles();
-        break;
-    }
-  } while (action != SchedulerStateMachine::ACTION_NONE);
+  {
+    // Separate scope for mark_inside.

[brianderson, 2014/12/20 00:34:13]

Do you only need this to allow re-entrancy when ScheduleBeginImplFrameDeadline
calls the deadline synchronously?

Would it be easier to read if this block is split out into a private method?

+    base::AutoReset<bool> mark_inside(&inside_process_scheduled_actions_, true);
+
+    SchedulerStateMachine::Action action;
+    do {
+      action = state_machine_.NextAction();
+      TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("cc.debug.scheduler"),
+                   "SchedulerStateMachine",
+                   "state",
+                   AsValue());
+      VLOG(2) << "Scheduler::ProcessScheduledActions: "
+              << SchedulerStateMachine::ActionToString(action) << " "
+              << state_machine_.GetStatesForDebugging();
+      state_machine_.UpdateState(action);
+      base::AutoReset<SchedulerStateMachine::Action>
+          mark_inside_action(&inside_action_, action);
+      switch (action) {
+        case SchedulerStateMachine::ACTION_NONE:
+          break;
+        case SchedulerStateMachine::ACTION_ANIMATE:
+          client_->ScheduledActionAnimate();
+          break;
+        case SchedulerStateMachine::ACTION_SEND_BEGIN_MAIN_FRAME:
+          client_->ScheduledActionSendBeginMainFrame();
+          break;
+        case SchedulerStateMachine::ACTION_COMMIT:
+          client_->ScheduledActionCommit();
+          break;
+        case SchedulerStateMachine::ACTION_ACTIVATE_SYNC_TREE:
+          client_->ScheduledActionActivateSyncTree();
+          break;
+        case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_IF_POSSIBLE:
+          DrawAndSwapIfPossible();
+          break;
+        case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_FORCED:
+          client_->ScheduledActionDrawAndSwapForced();
+          break;
+        case SchedulerStateMachine::ACTION_DRAW_AND_SWAP_ABORT:
+          // No action is actually performed, but this allows the state machine
+          // to advance out of it's waiting to draw state without actually 

[brianderson, 2014/12/20 00:34:13]

it's -> its

[sunnyps, 2014/12/20 00:39:30]

Acknowledged.

+          // drawing.
+          break;
+        case SchedulerStateMachine::ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
+          client_->ScheduledActionBeginOutputSurfaceCreation();
+          break;
+        case SchedulerStateMachine::ACTION_PREPARE_TILES:
+          client_->ScheduledActionPrepareTiles();
+          break;
+        case SchedulerStateMachine::ACTION_INVALIDATE_OUTPUT_SURFACE:
+          client_->ScheduledActionInvalidateOutputSurface();
+          break;
+      }
+    } while (action != SchedulerStateMachine::ACTION_NONE);
+  }
 
   SetupNextBeginFrameIfNeeded();
   client_->DidAnticipatedDrawTimeChange(AnticipatedDrawTime());
 
-  RescheduleBeginImplFrameDeadlineIfNeeded();
+  ScheduleBeginImplFrameDeadlineIfNeeded();
 }
 
 bool Scheduler::WillDrawIfNeeded() const {
@@ -762,8 +737,6 @@ void Scheduler::AsValueInto(base::debug::TracedValue* state) const {
   state->SetInteger("begin_retro_frame_args_", begin_retro_frame_args_.size());
   state->SetBoolean("begin_impl_frame_deadline_task_",
                     !begin_impl_frame_deadline_task_.IsCancelled());
-  state->SetBoolean("poll_for_draw_triggers_task_",
-                    !poll_for_draw_triggers_task_.IsCancelled());
   state->SetBoolean("advance_commit_state_task_",
                     !advance_commit_state_task_.IsCancelled());
   state->BeginDictionary("begin_impl_frame_args");