cc: Split out BeginFrame needed verse proactive for commits.

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 400 matching lines @@
     return false;
 
   // We can not perform commits if we are not visible.
   if (!visible_)
     return false;
 
   return true;
 }
 
 bool SchedulerStateMachine::ShouldSendBeginMainFrame() const {
+  // If begin frames are not needed, we shouldn't be sending an BeginMainFrame.
+  if (!BeginFrameNeeded())
+    return false;
+
   if (!CouldSendBeginMainFrame())
     return false;
 
   // Only send BeginMainFrame when there isn't another commit pending already.
   if (commit_state_ != COMMIT_STATE_IDLE)
     return false;
 
   // Don't send BeginMainFrame early if we are prioritizing the active tree
   // because of impl_latency_takes_priority_.
   if (impl_latency_takes_priority_ &&
       (has_pending_tree_ || active_tree_needs_first_draw_)) {
     return false;
   }
 
   // We should not send BeginMainFrame while we are in
   // BEGIN_IMPL_FRAME_STATE_IDLE since we might have new
   // user input arriving soon.
   // TODO(brianderson): Allow sending BeginMainFrame while idle when the main
   // thread isn't consuming user input.
-  if (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_IDLE &&
-      BeginFrameNeeded())

[brianderson, 2014/12/19 23:34:06]

The check for BeginFrameNeeded is so we can commit activate and be ready to draw
before we request a BeginFrame from WebView.

I think Sunny's patch is a pre-req to this patch, and is likely why you were
seeing WebView test flake:
https://codereview.chromium.org/817603002

+  if (begin_impl_frame_state_ == BEGIN_IMPL_FRAME_STATE_IDLE)
     return false;
 
   // We need a new commit for the forced redraw. This honors the
   // single commit per interval because the result will be swapped to screen.
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_COMMIT)
     return true;
 
   // After this point, we only start a commit once per frame.
   if (HasSentBeginMainFrameThisFrame())
     return false;
@@ skipping 238 matching lines @@
 
 bool SchedulerStateMachine::BeginFrameNeededForChildren() const {
   if (HasInitializedOutputSurface())
     return children_need_begin_frames_;
 
   return false;
 }
 
 bool SchedulerStateMachine::BeginFrameNeeded() const {
   if (SupportsProactiveBeginFrame()) {
-    return (BeginFrameNeededToAnimateOrDraw() ||
-            BeginFrameNeededForChildren() ||
-            ProactiveBeginFrameWanted());
+    return (BeginFrameNeededToAnimateOrDrawOrCommit() ||
+            BeginFrameNeededForChildren() || ProactiveBeginFrameWanted());

[sunnyps, 2015/03/05 00:19:47]

OR needs_commit_ here.

   }
 
   // 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();
+  return BeginFrameNeededToAnimateOrDrawOrCommit();
 }
 
 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();
+    return !BeginFrameNeededToAnimateOrDrawOrCommit() &&
+           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;
 }
 
 void SchedulerStateMachine::SetChildrenNeedBeginFrames(
     bool children_need_begin_frames) {
   DCHECK(settings_.forward_begin_frames_to_children);
   children_need_begin_frames_ = children_need_begin_frames;
 }
 
 // These are the cases where we definitely (or almost definitely) have a
 // new frame to animate and/or draw and can draw.
-bool SchedulerStateMachine::BeginFrameNeededToAnimateOrDraw() const {
+bool SchedulerStateMachine::BeginFrameNeededToAnimateOrDrawOrCommit() const {
   // The output surface is the provider of BeginImplFrames, so we are not going
   // to get them even if we ask for them.
   if (!HasInitializedOutputSurface())
     return false;
 
   // The forced draw respects our normal draw scheduling, so we need to
   // request a BeginImplFrame for it.
   if (forced_redraw_state_ == FORCED_REDRAW_STATE_WAITING_FOR_DRAW)
     return true;
 
-  return needs_animate_ || needs_redraw_;
+  return needs_animate_ || needs_redraw_ || needs_commit_;

[sunnyps, 2015/03/05 00:19:47]

Remove needs_commit_ from here.

 }
 
 // These are cases where we are very likely to draw soon, but might not
 // actually have a new frame to draw when we receive the next BeginImplFrame.
 // Proactively requesting the BeginImplFrame helps hide the round trip latency
 // of the SetNeedsBeginFrame request that has to go to the Browser.
 bool SchedulerStateMachine::ProactiveBeginFrameWanted() const {
   // The output surface is the provider of BeginImplFrames,
   // so we are not going to get them even if we ask for them.
   if (!HasInitializedOutputSurface())
@@ skipping 335 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