cc: Implement deadine scheduling disabled by default

cc/trees/thread_proxy.cc

Index: cc/trees/thread_proxy.cc
diff --git a/cc/trees/thread_proxy.cc b/cc/trees/thread_proxy.cc
index 7f590f06be330d2f445f013eafa1aac4364045b6..7180de464ddeb8a94e2cfa60ec9e928949893336 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -80,6 +80,7 @@ ThreadProxy::ThreadProxy(
       inside_commit_(false),
       weak_factory_on_impl_thread_(this),
       weak_factory_(this),
+      frame_did_draw_(false),
       begin_frame_sent_to_main_thread_completion_event_on_impl_thread_(NULL),
       readback_request_on_impl_thread_(NULL),
       commit_completion_event_on_impl_thread_(NULL),
@@ -397,14 +398,35 @@ void ThreadProxy::SetNeedsBeginFrameOnImplThread(bool enable) {
   TRACE_EVENT1("cc", "ThreadProxy::SetNeedsBeginFrameOnImplThread",
                "enable", enable);
   layer_tree_host_impl_->SetNeedsBeginFrame(enable);
+  UpdateBackgroundAnimateTicking();
 }
 
 void ThreadProxy::BeginFrameOnImplThread(const BeginFrameArgs& args) {
   DCHECK(IsImplThread());
   TRACE_EVENT0("cc", "ThreadProxy::BeginFrameOnImplThread");
+
+  base::TimeTicks monotonic_time =
+      layer_tree_host_impl_->CurrentFrameTimeTicks();
+  base::Time wall_clock_time = layer_tree_host_impl_->CurrentFrameTime();
+  if (layer_tree_host_impl_->active_tree()->root_layer())
+    layer_tree_host_impl_->Animate(monotonic_time, wall_clock_time);
+
+  // Reinitialize for the current frame.
+  frame_did_draw_ = false;
+
   scheduler_on_impl_thread_->BeginFrame(args);
 }
 
+void ThreadProxy::DidBeginFrameDeadlineOnImplThread() {
+  // Do not start animations if we skip drawing the frame to avoid
+  // checkerboarding.
+  if (layer_tree_host_impl_->active_tree()->root_layer()) {
+    layer_tree_host_impl_->UpdateAnimationState(
+        frame_did_draw_ || !layer_tree_host_impl_->CanDraw());

[enne (OOO), 2013/09/13 22:28:51]

Maybe I just missed this in previous patches, but what is moving around all the
Animate/UpdateAnimationState/ResetCurrentFrameTimeForNextFrame solving here?

[brianderson, 2013/09/13 22:50:33]

Since we have a BeginFrame+deadline interval (in which we can commit, activate,
draw, and swap within a single vsync) I wanted to update the animation time at
the start of the BeginFrame for a few reasons:

1) So that activation of a new commit depends on the animation time of the draw.
Otherwise, I was worried we could activate prematurely and needlessly
checkerboard when we trigger the deadline early.
2) This also allows the RAF frame time to match the draw time when we are in the
low-latency mode.
3) As a followup patch, I want to tie BeginFrameArgs::frame_time in
ThreadProxy::BeginFrameOnImplThread to the animation time, rather than relying
on the LTHI::CurrentFrameTimeTicks and LTHI::CurrrentFrameTime to sample
base::TimeTicks::Now() at a somewhat jittery phase.

[enne (OOO), 2013/09/13 23:21:54]

With this patch, the animation time is the previous draw time, since you're
animating in BeginFrameOnImplThread and setting the time in
DidBeginFrameDeadlineOnImplThread, which happens afterwards.

It seems like ResetCurrentFrameTime should happen in BeginFrameOnImplThread,
which will then allow you to set the time to be in the future, rather than in
the past?

UpdateAnimationState also seems out of place, because you animate in
BeginFrameOnImplThread and only later in DidBeginFrameDeadline do you activate
animations from the previous draw.  If I'm reading that right, you'll have two
draws with the start frame of animation?  Can this just stay in
DrawSwapReadbackInternal, or at least move to DidBeginFrameOnImplThread prior to
animate (which is sort of where it was previously via
DidAnticipatedDrawTimeChange)?

Maybe ajuma or vollick can sanity check my animation thoughts here.

[brianderson, 2013/09/14 00:09:59]

I think the naming of the timing functions is a little confusing (especially
given it's previous usage) but, with this patch, the frame time should get
sampled in BeginFrameOnImplThread. So we will not use a frame time in the past
and we will not have two draws with the start frame of animation.

Reasoning:
LTHI::ResetCurrentFrameTimeForNextFrame() sets the frame time to zero internally
in LTHI, so that the next time LTHI::CurrentFrameTimeTicks() is called it will
sample a new frame time.  The next time LTHI::CurrentFrameTimeTicks() is called
will be inside of ThreadProxy::BeginFrameOnImplThread().

I also think putting UpdateAnimationState inside of
DidBeginFrameDeadlineOnImpleThread makes sense because it ensures we have
balanced calls to Animate and UpdateAnimationState as not every BeginFrame will
result in a draw.

I would also like feedback from @ajuma and @vollick though. The balanced calls
give me a warm fuzzy feeling, but maybe they don't need to be balanced?

[ajuma, 2013/09/14 00:19:40]

They should stay balanced. In the old code, we were relying on background
ticking in situations where we were visible but CanDraw was false. Since we're
now foreground ticking in those situations, we need to make sure we
UpdateAnimationState too (so putting the call in
DidBeginFrameDeadlineOnImplThread seems reasonable to me).

[ajuma, 2013/09/14 00:19:40]

The call to Animate moves animations into a Starting state (where they get
ticked at their starting frame). When UpdateAnimationState promotes animations
from started to running, it uses the time of the last tick as the animation's
start time.. So the next time that those animations get ticked, they'll be past
their start frame.

+  }
+  layer_tree_host_impl_->ResetCurrentFrameTimeForNextFrame();
+}
+
 void ThreadProxy::OnCanDrawStateChanged(bool can_draw) {
   DCHECK(IsImplThread());
   TRACE_EVENT1(
@@ -993,6 +1015,10 @@ void ThreadProxy::ScheduledActionCommit() {
   begin_frame_to_commit_duration_history_.InsertSample(
       commit_complete_time_ - begin_frame_sent_to_main_thread_time_);
 
+  // The commit may have added animations, requiring us to start

[enne (OOO), 2013/09/13 22:28:51]

Can you mention that this is only true if impl-side painting is not turned on?

[brianderson, 2013/09/13 22:50:33]

Now that you've pointed that out, is this call even needed? cc_unittests pass
without it.

I forget why this was added. @ajuma, do you remember?

We only background tick if we are not visible. If we are not visible, commits
will be aborted by the main thread and we shouldn't expect any new animations
that would cause us to start background ticking.

Only a readback commit would go through while not visible, and that shouldn't
add animations.

[ajuma, 2013/09/14 00:19:40]

This was added because a test was failing in a situation where CanDraw was false
but we were still getting commits; since we background tick only if both CanDraw
is false and there are animations, a commit that added an animation without
causing CanDraw to be true would require us to start background ticking. In the
current state of this CL, it looks like in this situation (where CanDraw is
false but we're visible), we'll continue to foreground tick (since ticking has
moved to BeginFrameOnImplThread), so we won't need to background tick.

+  // background ticking.
+  UpdateBackgroundAnimateTicking();
+
   // SetVisible kicks off the next scheduler action, so this must be last.
   scheduler_on_impl_thread_->SetVisible(layer_tree_host_impl_->visible());
 }
@@ -1035,16 +1061,6 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
   if (!layer_tree_host_impl_->renderer())
     return result;
 
-  base::TimeTicks monotonic_time =
-      layer_tree_host_impl_->CurrentFrameTimeTicks();
-  base::Time wall_clock_time = layer_tree_host_impl_->CurrentFrameTime();
-
-  // TODO(enne): This should probably happen post-animate.
-  if (layer_tree_host_impl_->pending_tree())
-    layer_tree_host_impl_->pending_tree()->UpdateDrawProperties();
-  layer_tree_host_impl_->Animate(monotonic_time, wall_clock_time);
-  UpdateBackgroundAnimateTicking();
-
   base::TimeTicks start_time = base::TimeTicks::HighResNow();
   base::TimeDelta draw_duration_estimate = DrawDurationEstimate();
   base::AutoReset<bool> mark_inside(&inside_draw_, true);
@@ -1060,15 +1076,12 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
   // DrawLayers() depends on the result of PrepareToDraw(), it is guarded on
   // CanDraw() as well.
 
-  // readback_request_on_impl_thread_ may be for the pending tree, do
-  // not perform the readback unless explicitly requested.
   bool drawing_for_readback =
       readback_requested && !!readback_request_on_impl_thread_;
   bool can_do_readback = layer_tree_host_impl_->renderer()->CanReadPixels();
 
   LayerTreeHostImpl::FrameData frame;
   bool draw_frame = false;
-  bool start_ready_animations = true;
 
   if (layer_tree_host_impl_->CanDraw() &&
       (!drawing_for_readback || can_do_readback)) {
@@ -1077,15 +1090,13 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
     if (drawing_for_readback)
       readback_rect = readback_request_on_impl_thread_->rect;
 
-    // Do not start animations if we skip drawing the frame to avoid
-    // checkerboarding.
     if (layer_tree_host_impl_->PrepareToDraw(&frame, readback_rect) ||
         forced_draw)
       draw_frame = true;
-    else
-      start_ready_animations = false;
   }
 
+  frame_did_draw_ = draw_frame;
+
   if (draw_frame) {
     layer_tree_host_impl_->DrawLayers(
         &frame,
@@ -1093,7 +1104,6 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
     result.did_draw = true;
   }
   layer_tree_host_impl_->DidDrawAllLayers(frame);
-  layer_tree_host_impl_->UpdateAnimationState(start_ready_animations);
 
   // Check for a pending CompositeAndReadback.
   if (drawing_for_readback) {
@@ -1233,9 +1243,8 @@ DrawSwapReadbackResult ThreadProxy::ScheduledActionDrawAndReadback() {
 
 void ThreadProxy::DidAnticipatedDrawTimeChange(base::TimeTicks time) {
   if (current_resource_update_controller_on_impl_thread_)
-    current_resource_update_controller_on_impl_thread_
-        ->PerformMoreUpdates(time);
-  layer_tree_host_impl_->ResetCurrentFrameTimeForNextFrame();
+    current_resource_update_controller_on_impl_thread_->PerformMoreUpdates(
+        time);
 }
 
 base::TimeDelta ThreadProxy::DrawDurationEstimate() {
@@ -1256,6 +1265,14 @@ base::TimeDelta ThreadProxy::CommitToActivateDurationEstimate() {
       kCommitAndActivationDurationEstimationPercentile);
 }
 
+void ThreadProxy::PostBeginFrameDeadline(const base::Closure& closure,
+                                         base::TimeTicks deadline) {
+  base::TimeDelta delta = deadline - base::TimeTicks::Now();
+  if (delta <= base::TimeDelta())
+    delta = base::TimeDelta();
+  Proxy::ImplThreadTaskRunner()->PostDelayedTask(FROM_HERE, closure, delta);
+}
+
 void ThreadProxy::ReadyToFinalizeTextureUpdates() {
   DCHECK(IsImplThread());
   scheduler_on_impl_thread_->FinishCommit();
@@ -1326,6 +1343,8 @@ void ThreadProxy::InitializeImplOnImplThread(CompletionEvent* completion) {
   layer_tree_host_impl_ = layer_tree_host_->CreateLayerTreeHostImpl(this);
   const LayerTreeSettings& settings = layer_tree_host_->settings();
   SchedulerSettings scheduler_settings;
+  scheduler_settings.deadline_scheduling_enabled =
+      settings.deadline_scheduling_enabled;
   scheduler_settings.impl_side_painting = settings.impl_side_painting;
   scheduler_settings.timeout_and_draw_when_animation_checkerboards =
       settings.timeout_and_draw_when_animation_checkerboards;