cc: Trigger deadline immediately when commit is aborted.

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/debug/trace_event_argument.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
@@ skipping 835 matching lines @@
   // 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_)
-    return false;
+  if (needs_redraw_) {
+    // SwapAck throttle the deadline since we wont draw and swap anyway.
+    if (pending_swaps_ >= max_pending_swaps_)

[brianderson, 2015/01/09 23:54:22]

Putting this inside the if(needs_redraw) I think will make us not wait until the
swap ack to send the BeginMainFrame when needs_commit_ is true and we are
otherwise idle. This will delay the BeginMainFrame until the next BeginFrame,
which will hurt throughput.

Can you put this above the if(needs_redraw) and add a test for the above case?

[sunnyps, 2015/01/13 19:36:01]

Done. The test I had to change in scheduler_state_machine_unittests kinda
already tests this so I reverted my change there.

+      return false;
 
-  if (active_tree_needs_first_draw_)
-    return true;
+    if (active_tree_needs_first_draw_)
+      return true;
 
-  if (!needs_redraw_)
-    return false;
+    // Prioritize impl-thread draws in impl_latency_takes_priority_ mode.
+    if (impl_latency_takes_priority_)
+      return true;
+
+    // If we are on battery power and want to prioritize impl latency because
+    // we don't trust deadline tasks to execute at the right time.
+    if (impl_latency_takes_priority_on_battery_)
+      return true;
+  }
 
   // This is used to prioritize impl-thread draws when the main thread isn't
   // producing anything, e.g., after an aborted commit. We also check that we
   // don't have a pending tree -- otherwise we should give it a chance to
   // activate.
   // TODO(skyostil): Revisit this when we have more accurate deadline estimates.
-  if (commit_state_ == COMMIT_STATE_IDLE && !has_pending_tree_)
-    return true;
-
-  // Prioritize impl-thread draws in impl_latency_takes_priority_ mode.
-  if (impl_latency_takes_priority_)
-    return true;
-
-  // If we are on battery power and want to prioritize impl latency because
-  // we don't trust deadline tasks to execute at the right time.
-  if (impl_latency_takes_priority_on_battery_)
-    return true;
-
-  return false;
+  return (commit_state_ == COMMIT_STATE_IDLE && !has_pending_tree_);
 }
 
 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.
@@ skipping 209 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