cc: Add SetChildrenNeedBeginFrames() & SendBeginFramesToChildren()

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 31 matching lines @@
       can_draw_(false),
       has_pending_tree_(false),
       pending_tree_is_ready_for_activation_(false),
       active_tree_needs_first_draw_(false),
       did_commit_after_animating_(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),
       continuous_painting_(false),
-      impl_latency_takes_priority_on_battery_(false) {
+      impl_latency_takes_priority_on_battery_(false),
+      children_need_begin_frames_(false) {
 }
 
 const char* SchedulerStateMachine::OutputSurfaceStateToString(
     OutputSurfaceState state) {
   switch (state) {
     case OUTPUT_SURFACE_ACTIVE:
       return "OUTPUT_SURFACE_ACTIVE";
     case OUTPUT_SURFACE_LOST:
       return "OUTPUT_SURFACE_LOST";
     case OUTPUT_SURFACE_CREATING:
@@ skipping 171 matching lines @@
                     impl_latency_takes_priority_);
   state->SetBoolean("main_thread_is_in_high_latency_mode",
                     MainThreadIsInHighLatencyMode());
   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("impl_latency_takes_priority_on_battery",
                     impl_latency_takes_priority_on_battery_);
+  state->SetBoolean("children_need_begin_frames", children_need_begin_frames_);
   state->EndDictionary();
 }
 
 void SchedulerStateMachine::AdvanceCurrentFrameNumber() {
   current_frame_number_++;
 
   // "Drain" the ManageTiles funnel.
   if (manage_tiles_funnel_ > 0)
     manage_tiles_funnel_--;
 
@@ skipping 458 matching lines @@
   needs_manage_tiles_ = false;
 }
 
 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())
+    return false;
+

[picksi1, 2014/11/20 13:43:18]

nit: [Background: I'm a fan of keeping !'s out of if statements] It might be
more readable to remove the ! and swap the body of the if... e.g.

if (HasInitializedOutputSurface())
  return children_need_begin_frames_;
return false;

[simonhong, 2014/11/20 16:24:09]

Thanks! Looks more readable :)

+  return children_need_begin_frames_;
+}
+
 bool SchedulerStateMachine::BeginFrameNeeded() const {
   // 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.
-  if (!SupportsProactiveBeginFrame())
+  if (!SupportsProactiveBeginFrame()) {
+    // Synchronous compositor doesn't have a browser.
+    DCHECK(!children_need_begin_frames_);
     return BeginFrameNeededToAnimateOrDraw();
+  }
 
-  return BeginFrameNeededToAnimateOrDraw() || ProactiveBeginFrameWanted();
+  return (BeginFrameNeededToAnimateOrDraw() ||
+          BeginFrameNeededForChildren() ||
+          ProactiveBeginFrameWanted());

[picksi1, 2014/11/20 13:43:18]

The above code is starting to get hairy IMO!  BeginFrameNeededToAnimateOrDraw()
is [always] called in two separate places and the or'ing together of functions
in the return is starting to get a bit long winded. Is it worth refactoring this
to clean this up before a future dev follows this pattern and compounds the
problem?... The best I can do is as follows, but I don't know if this logic
follows the intention of the implementation.

if (BeginFrameNeededToAnimateOrDraw())
   return true;
if (SupportsProactiveBeginFrame())
   return BeginFrameNeededForChildren() || ProactiveBeginFrameWanted();
return false;

[simonhong, 2014/11/20 16:24:09]

Hmm, I think we can understand more easily by dividing synchronous or not.
How about below? this removes "!", too.

if (SupportsProactiveBeginFrame())
   return (BeginFrameNeededToAnimateOrDraw() ||
               BeginFrameNeededForChildren() ||
               ProactiveBeginFrameWanted());

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;
 }
 
+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 {
   // 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.
@@ skipping 334 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