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

cc/scheduler/scheduler_state_machine.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/scheduler/scheduler_state_machine.h"
 
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
 #include "base/values.h"
 #include "ui/gfx/frame_time.h"
 
 namespace cc {
 
 SchedulerStateMachine::SchedulerStateMachine(const SchedulerSettings& settings)
     : settings_(settings),
       output_surface_state_(OUTPUT_SURFACE_LOST),
       begin_impl_frame_state_(BEGIN_IMPL_FRAME_STATE_IDLE),
       commit_state_(COMMIT_STATE_IDLE),
       forced_redraw_state_(FORCED_REDRAW_STATE_IDLE),
       commit_count_(0),
       current_frame_number_(0),
       last_frame_number_animate_performed_(-1),
       last_frame_number_swap_performed_(-1),
       last_frame_number_swap_requested_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
+      last_frame_number_invalidate_output_surface_performed_(-1),
       animate_funnel_(false),
       perform_swap_funnel_(false),
       request_swap_funnel_(false),
       send_begin_main_frame_funnel_(false),
+      invalidate_output_surface_funnel_(false),
       prepare_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       max_pending_swaps_(1),
       pending_swaps_(0),
       needs_redraw_(false),
       needs_animate_(false),
       needs_prepare_tiles_(false),
       needs_commit_(false),
-      inside_poll_for_anticipated_draw_triggers_(false),
       visible_(false),
       can_start_(false),
       can_draw_(false),
       has_pending_tree_(false),
       pending_tree_is_ready_for_activation_(false),
       active_tree_needs_first_draw_(false),
       did_create_and_initialize_first_output_surface_(false),
       impl_latency_takes_priority_(false),
       skip_next_begin_main_frame_to_reduce_latency_(false),
       skip_begin_main_frame_to_reduce_latency_(false),
@@ skipping 30 matching lines @@
       return "BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING";
     case BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME:
       return "BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME";
     case BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE:
       return "BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE";
   }
   NOTREACHED();
   return "???";
 }
 
+const char* SchedulerStateMachine::BeginImplFrameDeadlineModeToString(
+    BeginImplFrameDeadlineMode mode) {
+  switch (mode) {
+    case BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE:
+      return "BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE";
+    case BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE:
+      return "BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE";
+    case BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR:
+      return "BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR";
+    case BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE:
+      return "BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE";
+  }
+  NOTREACHED();
+  return "???";
+}
+
 const char* SchedulerStateMachine::CommitStateToString(CommitState state) {
   switch (state) {
     case COMMIT_STATE_IDLE:
       return "COMMIT_STATE_IDLE";
     case COMMIT_STATE_BEGIN_MAIN_FRAME_SENT:
       return "COMMIT_STATE_BEGIN_MAIN_FRAME_SENT";
     case COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED:
       return "COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED";
     case COMMIT_STATE_READY_TO_COMMIT:
       return "COMMIT_STATE_READY_TO_COMMIT";
@@ skipping 37 matching lines @@
     case ACTION_DRAW_AND_SWAP_IF_POSSIBLE:
       return "ACTION_DRAW_AND_SWAP_IF_POSSIBLE";
     case ACTION_DRAW_AND_SWAP_FORCED:
       return "ACTION_DRAW_AND_SWAP_FORCED";
     case ACTION_DRAW_AND_SWAP_ABORT:
       return "ACTION_DRAW_AND_SWAP_ABORT";
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
       return "ACTION_BEGIN_OUTPUT_SURFACE_CREATION";
     case ACTION_PREPARE_TILES:
       return "ACTION_PREPARE_TILES";
+    case ACTION_INVALIDATE_OUTPUT_SURFACE:
+      return "ACTION_INVALIDATE_OUTPUT_SURFACE";
   }
   NOTREACHED();
   return "???";
 }
 
 scoped_refptr<base::trace_event::ConvertableToTraceFormat>
 SchedulerStateMachine::AsValue() const {
   scoped_refptr<base::trace_event::TracedValue> state =
       new base::trace_event::TracedValue();
   AsValueInto(state.get());
@@ skipping 23 matching lines @@
   state->SetInteger("last_frame_number_swap_requested",
                     last_frame_number_swap_requested_);
   state->SetInteger("last_frame_number_begin_main_frame_sent",
                     last_frame_number_begin_main_frame_sent_);
   state->SetBoolean("funnel: animate_funnel", animate_funnel_);
   state->SetBoolean("funnel: perform_swap_funnel", perform_swap_funnel_);
   state->SetBoolean("funnel: request_swap_funnel", request_swap_funnel_);
   state->SetBoolean("funnel: send_begin_main_frame_funnel",
                     send_begin_main_frame_funnel_);
   state->SetInteger("funnel: prepare_tiles_funnel", prepare_tiles_funnel_);
+  state->SetBoolean("funnel: invalidate_output_surface_funnel",
+                    invalidate_output_surface_funnel_);
   state->SetInteger("consecutive_checkerboard_animations",
                     consecutive_checkerboard_animations_);
   state->SetInteger("max_pending_swaps_", max_pending_swaps_);
   state->SetInteger("pending_swaps_", pending_swaps_);
   state->SetBoolean("needs_redraw", needs_redraw_);
   state->SetBoolean("needs_animate_", needs_animate_);
   state->SetBoolean("needs_prepare_tiles", needs_prepare_tiles_);
   state->SetBoolean("needs_commit", needs_commit_);
   state->SetBoolean("visible", visible_);
   state->SetBoolean("can_start", can_start_);
@@ skipping 12 matching lines @@
   state->SetBoolean("skip_begin_main_frame_to_reduce_latency",
                     skip_begin_main_frame_to_reduce_latency_);
   state->SetBoolean("skip_next_begin_main_frame_to_reduce_latency",
                     skip_next_begin_main_frame_to_reduce_latency_);
   state->SetBoolean("continuous_painting", continuous_painting_);
   state->SetBoolean("children_need_begin_frames", children_need_begin_frames_);
   state->SetBoolean("defer_commits", defer_commits_);
   state->EndDictionary();
 }
 
-void SchedulerStateMachine::AdvanceCurrentFrameNumber() {
-  current_frame_number_++;
-
-  animate_funnel_ = false;
-  perform_swap_funnel_ = false;
-  request_swap_funnel_ = false;
-  send_begin_main_frame_funnel_ = false;
-
-  // "Drain" the PrepareTiles funnel.
-  if (prepare_tiles_funnel_ > 0)
-    prepare_tiles_funnel_--;
-
-  skip_begin_main_frame_to_reduce_latency_ =
-      skip_next_begin_main_frame_to_reduce_latency_;
-  skip_next_begin_main_frame_to_reduce_latency_ = false;
-}
-
 bool SchedulerStateMachine::PendingDrawsShouldBeAborted() const {
   // These are all the cases where we normally cannot or do not want to draw
   // but, if needs_redraw_ is true and we do not draw to make forward progress,
   // we might deadlock with the main thread.
   // This should be a superset of PendingActivationsShouldBeForced() since
   // activation of the pending tree is blocked by drawing of the active tree and
   // the main thread might be blocked on activation of the most recent commit.
   if (PendingActivationsShouldBeForced())
     return true;
 
@@ skipping 204 matching lines @@
 }
 
 bool SchedulerStateMachine::ShouldPrepareTiles() const {
   // PrepareTiles only really needs to be called immediately after commit
   // and then periodically after that. Use a funnel to make sure we average
   // one PrepareTiles per BeginImplFrame in the long run.
   if (prepare_tiles_funnel_ > 0)
     return false;
 
   // Limiting to once per-frame is not enough, since we only want to
-  // prepare tiles _after_ draws. Polling for draw triggers and
-  // begin-frame are mutually exclusive, so we limit to these two cases.
-  if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE &&
-      !inside_poll_for_anticipated_draw_triggers_)
+  // prepare tiles _after_ draws.
+  if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
     return false;
+
   return needs_prepare_tiles_;
 }
 
+bool SchedulerStateMachine::ShouldInvalidateOutputSurface() const {
+  // Do not invalidate too many times in a frame.
+  if (invalidate_output_surface_funnel_)
+    return false;
+
+  // Only the synchronous compositor requires invalidations.
+  if (!settings_.using_synchronous_renderer_compositor)
+    return false;
+
+  // Invalidations are only performed inside a BeginFrame or inside deadline.
+  if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING &&
+      begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
+    return false;
+
+  return needs_redraw_ || needs_prepare_tiles_;

[brianderson, 2015/03/25 23:10:37]

Why needs_prepare_tiles_?

[sunnyps, 2015/03/27 00:52:48]

The previous draw might have caused tile properties to become dirty and set
needs_prepare_tiles_. It's possible we didn't prepare tiles then because of the
prepare tiles funnel. If we don't ask webview for a draw again we will never
call PrepareTiles but still keep asking for BeginFrames indefinitely.

In practice not having this causes the scheduler to never go to idle on certain
apps like Play Books.

[brianderson, 2015/03/27 21:24:24]

I think you can remove needs_prepare_tiles_ from here and still avoid the
scheduler never going idle if you drain the prepare_tiles_funnel__ in
BeginImplFrame rather than the deadline.

[sunnyps, 2015/03/27 22:38:24]

I moved the prepare_tiles_funnel_ decrement to OnBeginImplFrame but that does
not alleviate the need for checking needs_prepare_tiles here.

If needs_prepare_tiles is true we keep getting BeginFrames but unless we enter
the deadline phase we don't call PrepareTiles.

I know this causes some extra drawing to happen just for the sake of calling
PrepareTiles but it should be okay.

Aside: It would be great if we could call UDP+PrepareTiles on BeginImplFrame if
commit state is idle. That would solve this problem and probably other problems
as well.

+}
+
 SchedulerStateMachine::Action SchedulerStateMachine::NextAction() const {
   if (ShouldActivatePendingTree())
     return ACTION_ACTIVATE_SYNC_TREE;
   if (ShouldCommit())
     return ACTION_COMMIT;
   if (ShouldAnimate())
     return ACTION_ANIMATE;
   if (ShouldDraw()) {
     if (PendingDrawsShouldBeAborted())
       return ACTION_DRAW_AND_SWAP_ABORT;
     else if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
       return ACTION_DRAW_AND_SWAP_FORCED;
     else
       return ACTION_DRAW_AND_SWAP_IF_POSSIBLE;
   }
   if (ShouldPrepareTiles())
     return ACTION_PREPARE_TILES;
   if (ShouldSendBeginMainFrame())
     return ACTION_SEND_BEGIN_MAIN_FRAME;
+  if (ShouldInvalidateOutputSurface())
+    return ACTION_INVALIDATE_OUTPUT_SURFACE;
   if (ShouldBeginOutputSurfaceCreation())
     return ACTION_BEGIN_OUTPUT_SURFACE_CREATION;
   return ACTION_NONE;
 }
 
 void SchedulerStateMachine::UpdateState(Action action) {
   switch (action) {
     case ACTION_NONE:
       return;
 
     case ACTION_ACTIVATE_SYNC_TREE:
       UpdateStateOnActivation();
       return;
 
     case ACTION_ANIMATE:
-      DCHECK(!animate_funnel_);
-      last_frame_number_animate_performed_ = current_frame_number_;
-      animate_funnel_ = true;
-      needs_animate_ = false;
-      // TODO(skyostil): Instead of assuming this, require the client to tell
-      // us.
-      SetNeedsRedraw();
+      UpdateStateOnAnimate();
       return;
 
     case ACTION_SEND_BEGIN_MAIN_FRAME:
-      DCHECK(!has_pending_tree_ ||
-             settings_.main_frame_before_activation_enabled);
-      DCHECK(visible_);
-      DCHECK(!send_begin_main_frame_funnel_);
-      commit_state_ = COMMIT_STATE_BEGIN_MAIN_FRAME_SENT;
-      needs_commit_ = false;
-      send_begin_main_frame_funnel_ = true;
-      last_frame_number_begin_main_frame_sent_ =
-          current_frame_number_;
+      UpdateStateOnSendBeginMainFrame();
       return;
 
     case ACTION_COMMIT: {
       bool commit_has_no_updates = false;
       UpdateStateOnCommit(commit_has_no_updates);
       return;
     }
 
     case ACTION_DRAW_AND_SWAP_FORCED:
     case ACTION_DRAW_AND_SWAP_IF_POSSIBLE: {
       bool did_request_swap = true;
       UpdateStateOnDraw(did_request_swap);
       return;
     }
 
     case ACTION_DRAW_AND_SWAP_ABORT: {
       bool did_request_swap = false;
       UpdateStateOnDraw(did_request_swap);
       return;
     }
 
     case ACTION_BEGIN_OUTPUT_SURFACE_CREATION:
-      DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_LOST);
-      output_surface_state_ = OUTPUT_SURFACE_CREATING;
-
-      // The following DCHECKs make sure we are in the proper quiescent state.
-      // The pipeline should be flushed entirely before we start output
-      // surface creation to avoid complicated corner cases.
-      DCHECK_EQ(commit_state_, COMMIT_STATE_IDLE);
-      DCHECK(!has_pending_tree_);
-      DCHECK(!active_tree_needs_first_draw_);
+      UpdateStateOnBeginOutputSurfaceCreation();
       return;
 
     case ACTION_PREPARE_TILES:
       UpdateStateOnPrepareTiles();
       return;
+
+    case ACTION_INVALIDATE_OUTPUT_SURFACE:

[brianderson, 2015/03/25 23:10:37]

Thanks for making UpdateStateOn* method usage consistent.

+      UpdateStateOnInvalidateOutputSurface();
+      return;
   }
 }
 
+void SchedulerStateMachine::UpdateStateOnAnimate() {
+  DCHECK(!animate_funnel_);
+  last_frame_number_animate_performed_ = current_frame_number_;
+  animate_funnel_ = true;
+  needs_animate_ = false;
+  // TODO(skyostil): Instead of assuming this, require the client to tell
+  // us.

[brianderson, 2015/03/25 23:10:37]

Comment can be one line now.

[sunnyps, 2015/03/27 00:52:48]

Done.

+  SetNeedsRedraw();
+}
+
+void SchedulerStateMachine::UpdateStateOnSendBeginMainFrame() {
+  DCHECK(!has_pending_tree_ || settings_.main_frame_before_activation_enabled);
+  DCHECK(visible_);
+  DCHECK(!send_begin_main_frame_funnel_);
+  commit_state_ = COMMIT_STATE_BEGIN_MAIN_FRAME_SENT;
+  needs_commit_ = false;
+  send_begin_main_frame_funnel_ = true;
+  last_frame_number_begin_main_frame_sent_ = current_frame_number_;
+}
+
 void SchedulerStateMachine::UpdateStateOnCommit(bool commit_has_no_updates) {
   commit_count_++;
 
   if (!commit_has_no_updates)
     animate_funnel_ = false;
 
   if (commit_has_no_updates || settings_.main_frame_before_activation_enabled) {
     commit_state_ = COMMIT_STATE_IDLE;
   } else if (settings_.impl_side_painting) {
     commit_state_ = COMMIT_STATE_WAITING_FOR_ACTIVATION;
@@ skipping 75 matching lines @@
     DCHECK(!request_swap_funnel_);
     request_swap_funnel_ = true;
     last_frame_number_swap_requested_ = current_frame_number_;
   }
 }
 
 void SchedulerStateMachine::UpdateStateOnPrepareTiles() {
   needs_prepare_tiles_ = false;
 }
 
+void SchedulerStateMachine::UpdateStateOnBeginOutputSurfaceCreation() {
+  DCHECK_EQ(output_surface_state_, OUTPUT_SURFACE_LOST);
+  output_surface_state_ = OUTPUT_SURFACE_CREATING;
+
+  // The following DCHECKs make sure we are in the proper quiescent state.
+  // The pipeline should be flushed entirely before we start output
+  // surface creation to avoid complicated corner cases.
+  DCHECK_EQ(commit_state_, COMMIT_STATE_IDLE);
+  DCHECK(!has_pending_tree_);
+  DCHECK(!active_tree_needs_first_draw_);
+}
+
+void SchedulerStateMachine::UpdateStateOnInvalidateOutputSurface() {
+  DCHECK(!invalidate_output_surface_funnel_);
+  invalidate_output_surface_funnel_ = true;
+  last_frame_number_invalidate_output_surface_performed_ =
+      current_frame_number_;
+  // There is no guarantee that a draw will arrive from the synchronous
+  // compositor. We clear this flag here so that the pipeline isn't blocked.
+  active_tree_needs_first_draw_ = false;

[brianderson, 2015/03/25 23:10:37]

I'm worried that this will make us skip frames or draw duplicate frames. For
example, if we activated since the last draw, clear
active_tree_needs_first_draw_ here and then activate again before the next draw.

How often might we not get a draw from WebView after an Invalidate? If WebView
knows it's in this state, can it call something like DrawAbort() instead? That
way we wouldn't have to unconditionally remove the back pressure.

[sunnyps, 2015/03/27 00:52:48]

Webview can't even tell if it's in this state. Since webview allows apps to call
Java code from JS - it's possible to stop drawing at any point. Not setting this
flag to false can cause commit to be blocked indefinitely. An alternative to
this would be to not block commit due to this flag.

[brianderson, 2015/03/27 21:24:24]

Is the problem that we are blocking the main thread after sending a
BeginMainFrame? Or is it that we never send a BeginMainFrame in the first place?

In the first case, I think that could only happen because of the
SetNextCommitWaitsForActivation logic. In which case, maybe we should remove it.

In the second case, I think that is what we want. If the app isn't drawing, we
shouldn't start new commits.

[sunnyps, 2015/03/27 22:38:24]

The problem is that the main thread is blocked indefinitely while it's waiting
for a commit to happen.
The first case also happens if commit is blocked indefinitely.

[sunnyps, 2015/03/30 21:49:18]

I removed this piece of code and found that I could not reproduce the bug that
it fixed any more. So I'm removing it - we can add it later if we see the same
symptoms again.

+}
+
 void SchedulerStateMachine::SetSkipNextBeginMainFrameToReduceLatency() {
   TRACE_EVENT_INSTANT0("cc",
                        "Scheduler: SkipNextBeginMainFrameToReduceLatency",
                        TRACE_EVENT_SCOPE_THREAD);
   skip_next_begin_main_frame_to_reduce_latency_ = true;
 }
 
 bool SchedulerStateMachine::BeginFrameNeededForChildren() const {
   if (HasInitializedOutputSurface())

[brianderson, 2015/03/25 23:10:37]

The other BeginFrameNeeded* methods don't check HasInitializedOutputSurface
since BeginFrameNeeded does that already. SHould we remove it from here too? 

[sunnyps, 2015/03/27 00:52:48]

Done.

     return children_need_begin_frames_;
 
   return false;
 }
 
 bool SchedulerStateMachine::BeginFrameNeeded() const {
   // We can't handle BeginFrames when output surface isn't initialized.
   // TODO(brianderson): Support output surface creation inside a BeginFrame.
   if (!HasInitializedOutputSurface())
     return false;
-
-  if (SupportsProactiveBeginFrame()) {
-    return (BeginFrameNeededToAnimateOrDraw() ||
-            BeginFrameNeededForChildren() ||
-            ProactiveBeginFrameWanted());
-  }
-
-  // Proactive BeginFrames are bad for the synchronous compositor because we
-  // have to draw when we get the BeginFrame and could end up drawing many
-  // duplicate frames if our new frame isn't ready in time.
-  // To poll for state with the synchronous compositor without having to draw,
-  // we rely on ShouldPollForAnticipatedDrawTriggers instead.
-  // Synchronous compositor doesn't have a browser.
-  DCHECK(!children_need_begin_frames_);
-  return BeginFrameNeededToAnimateOrDraw();
-}
-
-bool SchedulerStateMachine::ShouldPollForAnticipatedDrawTriggers() const {
-  // ShouldPollForAnticipatedDrawTriggers is what we use in place of
-  // ProactiveBeginFrameWanted when we are using the synchronous
-  // compositor.
-  if (!SupportsProactiveBeginFrame()) {
-    return !BeginFrameNeededToAnimateOrDraw() && ProactiveBeginFrameWanted();
-  }
-
-  // Non synchronous compositors should rely on
-  // ProactiveBeginFrameWanted to poll for state instead.
-  return false;
-}
-
-// Note: If SupportsProactiveBeginFrame is false, the scheduler should poll
-// for changes in it's draw state so it can request a BeginFrame when it's
-// actually ready.
-bool SchedulerStateMachine::SupportsProactiveBeginFrame() const {
-  // It is undesirable to proactively request BeginFrames if we are
-  // using a synchronous compositor because we *must* draw for every
-  // BeginFrame, which could cause duplicate draws.
-  return !settings_.using_synchronous_renderer_compositor;
+  return (BeginFrameNeededToAnimateOrDraw() || BeginFrameNeededForChildren() ||

[brianderson, 2015/03/25 23:10:37]

So much easier to understand!

+          ProactiveBeginFrameWanted());
 }
 
 void SchedulerStateMachine::SetChildrenNeedBeginFrames(
     bool children_need_begin_frames) {
   children_need_begin_frames_ = children_need_begin_frames;
 }
 
 void SchedulerStateMachine::SetDeferCommits(bool defer_commits) {
   defer_commits_ = defer_commits;
 }
@@ skipping 43 matching lines @@
 
   // If the last commit was aborted because of early out (no updates), we should
   // still want a begin frame in case there is a commit coming again.
   if (last_commit_had_no_updates_)
     return true;
 
   return false;
 }
 
 void SchedulerStateMachine::OnBeginImplFrame() {
-  AdvanceCurrentFrameNumber();
-  DCHECK_EQ(begin_impl_frame_state_, BEGIN_IMPL_FRAME_STATE_IDLE)
-      << AsValue()->ToString();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING;
+  current_frame_number_++;
+
   last_commit_had_no_updates_ = false;
+
+  // Clear funnels for any actions we perform during the frame.
+  animate_funnel_ = false;
+  perform_swap_funnel_ = false;
+  request_swap_funnel_ = false;
+  send_begin_main_frame_funnel_ = false;
+  invalidate_output_surface_funnel_ = false;
+
+  skip_begin_main_frame_to_reduce_latency_ =
+      skip_next_begin_main_frame_to_reduce_latency_;
+  skip_next_begin_main_frame_to_reduce_latency_ = false;
 }
 
 void SchedulerStateMachine::OnBeginImplFrameDeadlinePending() {
-  DCHECK_EQ(begin_impl_frame_state_,
-            BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING)
-      << AsValue()->ToString();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME;
 }
 
 void SchedulerStateMachine::OnBeginImplFrameDeadline() {
-  DCHECK_EQ(begin_impl_frame_state_, BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME)
-      << AsValue()->ToString();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE;
+
+  // Clear funnels for any actions we perform during the deadline.

[brianderson, 2015/03/25 23:10:37]

Why are these cleared in both the deadline and BeginImplFrame?

[sunnyps, 2015/03/27 00:52:48]

For invalidate_output_surface_funnel_ the reason is that we can invalidate in
both the BeginImplFrame phase and the Draw phase.

[brianderson, 2015/03/27 21:24:24]

Can add a comment about that and also explain in what cases it makes sense to
invalidate twice in one frame?

Without knowing details, I feel like resetting the funnel only in the deadline
would be sufficient. If we invalidate just after draw1, we should be able to
expect a draw2 even if we don't invalidate in the BeginImplFrame2.  If we don't
invalidate just after draw1, the funnel will be empty, allowing BeginImplFrame2
to invalidate if needed.

Is there a hole in my understanding of WebView?

[sunnyps, 2015/03/27 22:38:24]

I changed this so that it's invalidated only once, I think you're looking at old
patch set.

+  perform_swap_funnel_ = false;
+  request_swap_funnel_ = false;
+  invalidate_output_surface_funnel_ = false;
+
+  // "Drain" the PrepareTiles funnel.

[brianderson, 2015/03/25 23:10:37]

Why is this moved to the deadline rather than the BeginImplFrame where it was
before?

[sunnyps, 2015/03/27 00:52:48]

Because prepare tiles (when initiated by the scheduler) is performed only in the
deadline. We should revisit this if we make changes to when PrepareTiles gets
called.

[brianderson, 2015/03/27 21:24:24]

Was the intent not to have a functional change by moving it here?

I think this will sometimes call prepare tiles inside of a draw just after
having prepared tiles for a commit, which is something we want to avoid.

[sunnyps, 2015/03/27 22:38:24]

Good catch. Done.

+  if (prepare_tiles_funnel_ > 0)
+    prepare_tiles_funnel_--;
 }
 
 void SchedulerStateMachine::OnBeginImplFrameIdle() {
-  DCHECK_EQ(begin_impl_frame_state_, BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
-      << AsValue()->ToString();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_IDLE;
 }
 
 SchedulerStateMachine::BeginImplFrameDeadlineMode
 SchedulerStateMachine::CurrentBeginImplFrameDeadlineMode() const {
-  if (ShouldTriggerBeginImplFrameDeadlineImmediately()) {
+  if (settings_.using_synchronous_renderer_compositor) {
+    // No deadline for synchronous compositor.
+    return BEGIN_IMPL_FRAME_DEADLINE_MODE_NONE;
+  } else if (ShouldTriggerBeginImplFrameDeadlineImmediately()) {
     return BEGIN_IMPL_FRAME_DEADLINE_MODE_IMMEDIATE;
   } else if (needs_redraw_ && pending_swaps_ < max_pending_swaps_) {
     // 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.
     // If we are swap throttled we should wait until we are unblocked.
     return BEGIN_IMPL_FRAME_DEADLINE_MODE_REGULAR;
   } else {
     // The impl thread doesn't have anything it wants to draw and we are just
     // waiting for a new active tree or we are swap throttled. In short we are
     // blocked.
     return BEGIN_IMPL_FRAME_DEADLINE_MODE_LATE;
   }
 }
 
 bool SchedulerStateMachine::ShouldTriggerBeginImplFrameDeadlineImmediately()
     const {
   // TODO(brianderson): This should take into account multiple commit sources.
-
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME)
     return false;
 
   // If we've lost the output surface, end the current BeginImplFrame ASAP
   // so we can start creating the next output surface.
   if (output_surface_state_ == OUTPUT_SURFACE_LOST)
     return true;
 
   // SwapAck throttle the deadline since we wont draw and swap anyway.
   if (pending_swaps_ >= max_pending_swaps_)
@@ skipping 54 matching lines @@
     // which case the main thread is in a high latency mode.
     return (active_tree_needs_first_draw_ || perform_swap_funnel_) &&
            !send_begin_main_frame_funnel_;
   }
 
   // If the active tree needs its first draw in any other state, we know the
   // main thread is in a high latency mode.
   return active_tree_needs_first_draw_;
 }
 
-void SchedulerStateMachine::DidEnterPollForAnticipatedDrawTriggers() {
-  AdvanceCurrentFrameNumber();
-  inside_poll_for_anticipated_draw_triggers_ = true;
-}
-
-void SchedulerStateMachine::DidLeavePollForAnticipatedDrawTriggers() {
-  inside_poll_for_anticipated_draw_triggers_ = false;
-}
-
 void SchedulerStateMachine::SetVisible(bool visible) { visible_ = visible; }
 
 void SchedulerStateMachine::SetCanDraw(bool can_draw) { can_draw_ = can_draw; }
 
 void SchedulerStateMachine::SetNeedsRedraw() { needs_redraw_ = true; }
 
 void SchedulerStateMachine::SetNeedsAnimate() {
   needs_animate_ = true;
 }
 
@@ skipping 160 matching lines @@
       static_cast<int>(begin_impl_frame_state_),
       static_cast<int>(commit_state_),
       has_pending_tree_ ? 'T' : 'F',
       pending_tree_is_ready_for_activation_ ? 'T' : 'F',
       active_tree_needs_first_draw_ ? 'T' : 'F',
       max_pending_swaps_,
       pending_swaps_);
 }
 
 }  // namespace cc