cc: Unify use of DidSwapBuffers() and did_request_swap

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/debug/trace_event.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/stringprintf.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),
       readback_state_(READBACK_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_request_sent_(-1),
       last_frame_number_begin_main_frame_sent_(-1),
+      last_frame_number_draw_request_sent_(-1),
       last_frame_number_update_visible_tiles_was_called_(-1),
       manage_tiles_funnel_(0),
       consecutive_checkerboard_animations_(0),
       max_pending_swaps_(1),
       pending_swaps_(0),
       needs_redraw_(false),
       needs_animate_(false),
       needs_manage_tiles_(false),
       swap_used_incomplete_tile_(false),
       needs_commit_(false),
@@ skipping 179 matching lines @@
   state->Set("major_timestamps_in_ms", timestamps_state.release());
 
   scoped_ptr<base::DictionaryValue> minor_state(new base::DictionaryValue);
   minor_state->SetInteger("commit_count", commit_count_);
   minor_state->SetInteger("current_frame_number", current_frame_number_);
 
   minor_state->SetInteger("last_frame_number_animate_performed",
                           last_frame_number_animate_performed_);
   minor_state->SetInteger("last_frame_number_swap_performed",
                           last_frame_number_swap_performed_);
+  minor_state->SetInteger("last_frame_number_swap_request_sent",
+                          last_frame_number_swap_request_sent_);
   minor_state->SetInteger(
       "last_frame_number_begin_main_frame_sent",
       last_frame_number_begin_main_frame_sent_);
   minor_state->SetInteger(
+      "last_frame_number_draw_request_sent",
+      last_frame_number_draw_request_sent_);
+  minor_state->SetInteger(
       "last_frame_number_update_visible_tiles_was_called",
       last_frame_number_update_visible_tiles_was_called_);
 
   minor_state->SetInteger("manage_tiles_funnel", manage_tiles_funnel_);
   minor_state->SetInteger("consecutive_checkerboard_animations",
                           consecutive_checkerboard_animations_);
   minor_state->SetInteger("max_pending_swaps_", max_pending_swaps_);
   minor_state->SetInteger("pending_swaps_", pending_swaps_);
   minor_state->SetBoolean("needs_redraw", needs_redraw_);
   minor_state->SetBoolean("needs_animate_", needs_animate_);
@@ skipping 44 matching lines @@
 
 bool SchedulerStateMachine::HasUpdatedVisibleTilesThisFrame() const {
   return current_frame_number_ ==
          last_frame_number_update_visible_tiles_was_called_;
 }
 
 bool SchedulerStateMachine::HasSwappedThisFrame() const {
   return current_frame_number_ == last_frame_number_swap_performed_;
 }
 
+bool SchedulerStateMachine::HasRequestedSwapThisFrame() const {
+  return current_frame_number_ == last_frame_number_swap_request_sent_;
+}
+
+bool SchedulerStateMachine::HasRequestedDrawThisFrame() const {
+  return current_frame_number_ == last_frame_number_draw_request_sent_;
+}
+
 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 58 matching lines @@
     return true;
 
   // If we need to abort draws, we should do so ASAP since the draw could
   // be blocking other important actions (like output surface initialization),
   // from occuring. If we are waiting for the first draw, then perfom the
   // aborted draw to keep things moving. If we are not waiting for the first
   // draw however, we don't want to abort for no reason.
   if (PendingDrawsShouldBeAborted())
     return active_tree_needs_first_draw_;
 
-  // After this line, we only want to swap once per frame.
-  if (HasSwappedThisFrame())
+  // After this line, we only want to send swap request once per frame.

[Sami, 2014/05/08 14:18:11]

nit: "send a swap request"

[simonhong, 2014/05/08 14:50:14]

Done.

+  if (HasRequestedSwapThisFrame())
     return false;
 
   // Do not queue too many swaps.
   if (pending_swaps_ >= max_pending_swaps_)
     return false;
 
   // Except for the cases above, do not draw outside of the BeginImplFrame
   // deadline.
   if (begin_impl_frame_state_ != BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE)
     return false;
@@ skipping 399 matching lines @@
     forced_redraw_state_ = FORCED_REDRAW_STATE_IDLE;
   }
 
   if (!has_pending_tree_ &&
       commit_state_ == COMMIT_STATE_WAITING_FOR_FIRST_DRAW) {
     commit_state_ = COMMIT_STATE_IDLE;
   }
 
   needs_redraw_ = false;
   active_tree_needs_first_draw_ = false;
+  last_frame_number_draw_request_sent_ = current_frame_number_;
 
   if (did_request_swap)
-    last_frame_number_swap_performed_ = current_frame_number_;
+    last_frame_number_swap_request_sent_ = current_frame_number_;
 }
 
 void SchedulerStateMachine::UpdateStateOnManageTiles() {
   needs_manage_tiles_ = false;
 }
 
 void SchedulerStateMachine::SetSkipNextBeginMainFrameToReduceLatency() {
   skip_next_begin_main_frame_to_reduce_latency_ = true;
 }
 
@@ skipping 87 matching lines @@
   // If the pending tree activates quickly, we'll want a BeginImplFrame soon
   // to draw the new active tree.
   if (has_pending_tree_)
     return true;
 
   // Changing priorities may allow us to activate (given the new priorities),
   // which may result in a new frame.
   if (needs_manage_tiles_)
     return true;
 
-  // If we just swapped, it's likely that we are going to produce another
-  // frame soon. This helps avoid negative glitches in our
+  // If we just sent draw request, it's likely that we are going to produce

[Sami, 2014/05/08 14:18:11]

nit: "sent a draw request" or "requested a draw"

[simonhong, 2014/05/08 14:50:14]

Done.

+  // another frame soon. This helps avoid negative glitches in our
   // SetNeedsBeginFrame requests, which may propagate to the BeginImplFrame
   // provider and get sampled at an inopportune time, delaying the next
   // BeginImplFrame.
-  if (last_frame_number_swap_performed_ == current_frame_number_)
+  if (HasRequestedDrawThisFrame())
     return true;
 
   return false;
 }
 
 void SchedulerStateMachine::OnBeginImplFrame(const BeginFrameArgs& args) {
   AdvanceCurrentFrameNumber();
   begin_impl_frame_args_ = args;
   DCHECK_EQ(begin_impl_frame_state_, BEGIN_IMPL_FRAME_STATE_IDLE) << *AsValue();
   begin_impl_frame_state_ = BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING;
@@ skipping 60 matching lines @@
 }
 
 bool SchedulerStateMachine::MainThreadIsInHighLatencyMode() const {
   // If a commit is pending before the previous commit has been drawn, we
   // are definitely in a high latency mode.
   if (CommitPending() && (active_tree_needs_first_draw_ || has_pending_tree_))
     return true;
 
   // If we just sent a BeginMainFrame and haven't hit the deadline yet, the main
   // thread is in a low latency mode.
-  if (last_frame_number_begin_main_frame_sent_ == current_frame_number_ &&
+  if (HasSentBeginMainFrameThisFrame() &&
       (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_BEGIN_FRAME_STARTING ||
        begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_BEGIN_FRAME))
     return false;
 
   // If there's a commit in progress it must either be from the previous frame
   // or it started after the impl thread's deadline. In either case the main
   // thread is in high latency mode.
-  if (commit_state_ == COMMIT_STATE_BEGIN_MAIN_FRAME_SENT ||
-      commit_state_ == COMMIT_STATE_BEGIN_MAIN_FRAME_STARTED ||
-      commit_state_ == COMMIT_STATE_READY_TO_COMMIT)
+  if (CommitPending())
     return true;
 
   // Similarly, if there's a pending tree the main thread is in high latency
   // mode, because either
   //   it's from the previous frame
   // or
   //   we're currently drawing the active tree and the pending tree will thus
   //   only be drawn in the next frame.
   if (has_pending_tree_)
     return true;
 
   if (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_INSIDE_DEADLINE) {
     // Even if there's a new active tree to draw at the deadline or we've just
-    // drawn it, it may have been triggered by a previous BeginImplFrame, in
+    // swapped it, it may have been triggered by a previous BeginImplFrame, in
     // which case the main thread is in a high latency mode.
-    return (active_tree_needs_first_draw_ ||
-            last_frame_number_swap_performed_ == current_frame_number_) &&
-           last_frame_number_begin_main_frame_sent_ != current_frame_number_;
+    return (active_tree_needs_first_draw_ || HasSwappedThisFrame()) &&

[Sami, 2014/05/08 14:18:11]

Should this be HasSwappedThisFrame() or HasSentSwapRequestThisFrame()? To keep
existing behavior I think we should call the latter, but I'm not sure if that's
right.

[simonhong, 2014/05/08 14:50:14]

As brian said, we should check whether swap is performed(not just requested) or
not.
With this checking condition, if swap is early out, we can transition from high
latency mode
to low latency mode without skipping BeginMainFrame because this swap is handled
immediately.
If we check whether swap is requested or not in here, we don't know this commit
is swapped or not.
So, we should skip BeginMainFrame even if swap is early out.

+           !HasSentBeginMainFrameThisFrame();
   }
 
   // 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;
@@ skipping 21 matching lines @@
   }
 }
 
 void SchedulerStateMachine::SetMaxSwapsPending(int max) {
   max_pending_swaps_ = max;
 }
 
 void SchedulerStateMachine::DidSwapBuffers() {
   pending_swaps_++;
   DCHECK_LE(pending_swaps_, max_pending_swaps_);
+
+  last_frame_number_swap_performed_ = current_frame_number_;
 }
 
 void SchedulerStateMachine::SetSwapUsedIncompleteTile(
     bool used_incomplete_tile) {
   swap_used_incomplete_tile_ = used_incomplete_tile;
 }
 
 void SchedulerStateMachine::DidSwapBuffersComplete() {
   DCHECK_GT(pending_swaps_, 0);
   pending_swaps_--;
 }
 
 void SchedulerStateMachine::SetSmoothnessTakesPriority(
     bool smoothness_takes_priority) {
   smoothness_takes_priority_ = smoothness_takes_priority;
 }
 
-void SchedulerStateMachine::DidDrawIfPossibleCompleted(
-    DrawSwapReadbackResult::DrawResult result) {
+void SchedulerStateMachine::DidDrawIfPossibleCompleted(DrawResult result) {
   switch (result) {
-    case DrawSwapReadbackResult::INVALID_RESULT:
-      NOTREACHED() << "Uninitialized DrawSwapReadbackResult.";
+    case INVALID_RESULT:
+      NOTREACHED() << "Uninitialized DrawResult.";
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_CANT_DRAW:
-    case DrawSwapReadbackResult::DRAW_ABORTED_CANT_READBACK:
-    case DrawSwapReadbackResult::DRAW_ABORTED_CONTEXT_LOST:
+    case DRAW_ABORTED_CANT_DRAW:
+    case DRAW_ABORTED_CANT_READBACK:
+    case DRAW_ABORTED_CONTEXT_LOST:
       NOTREACHED() << "Invalid return value from DrawAndSwapIfPossible:"
                    << result;
       break;
-    case DrawSwapReadbackResult::DRAW_SUCCESS:
+    case DRAW_SUCCESS:
       consecutive_checkerboard_animations_ = 0;
       forced_redraw_state_ = FORCED_REDRAW_STATE_IDLE;
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_CHECKERBOARD_ANIMATIONS:
+    case DRAW_ABORTED_CHECKERBOARD_ANIMATIONS:
       needs_redraw_ = true;
 
       // If we're already in the middle of a redraw, we don't need to
       // restart it.
       if (forced_redraw_state_ != FORCED_REDRAW_STATE_IDLE)
         return;
 
       needs_commit_ = true;
       consecutive_checkerboard_animations_++;
       if (settings_.timeout_and_draw_when_animation_checkerboards &&
           consecutive_checkerboard_animations_ >=
               settings_.maximum_number_of_failed_draws_before_draw_is_forced_) {
         consecutive_checkerboard_animations_ = 0;
         // We need to force a draw, but it doesn't make sense to do this until
         // we've committed and have new textures.
         forced_redraw_state_ = FORCED_REDRAW_STATE_WAITING_FOR_COMMIT;
       }
       break;
-    case DrawSwapReadbackResult::DRAW_ABORTED_MISSING_HIGH_RES_CONTENT:
+    case DRAW_ABORTED_MISSING_HIGH_RES_CONTENT:
       // It's not clear whether this missing content is because of missing
       // pictures (which requires a commit) or because of memory pressure
       // removing textures (which might not).  To be safe, request a commit
       // anyway.
       needs_commit_ = true;
       break;
   }
 }
 
 void SchedulerStateMachine::SetNeedsCommit() { needs_commit_ = true; }
@@ skipping 99 matching lines @@
     case OUTPUT_SURFACE_ACTIVE:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_COMMIT:
     case OUTPUT_SURFACE_WAITING_FOR_FIRST_ACTIVATION:
       return true;
   }
   NOTREACHED();
   return false;
 }
 
 }  // namespace cc