cc: Make SingleThreadProxy::CompositeImmediately match scheduler path.

cc/trees/single_thread_proxy.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/trees/single_thread_proxy.h"
 
 #include "base/auto_reset.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/debug/benchmark_instrumentation.h"
@@ skipping 27 matching lines @@
 SingleThreadProxy::SingleThreadProxy(
     LayerTreeHost* layer_tree_host,
     LayerTreeHostSingleThreadClient* client,
     scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
     scoped_ptr<BeginFrameSource> external_begin_frame_source)
     : Proxy(main_task_runner, NULL),
       layer_tree_host_(layer_tree_host),
       client_(client),
       timing_history_(layer_tree_host->rendering_stats_instrumentation()),
       next_frame_is_newly_committed_frame_(false),
+      inside_impl_frame_(false),
       inside_draw_(false),
       defer_commits_(false),
       animate_requested_(false),
       commit_requested_(false),
       inside_synchronous_composite_(false),
       output_surface_creation_requested_(false),
       weak_factory_(this) {
   TRACE_EVENT0("cc", "SingleThreadProxy::SingleThreadProxy");
   DCHECK(Proxy::IsMainThread());
   DCHECK(layer_tree_host);
@@ skipping 521 matching lines @@
   layer_tree_host_->DidCompleteSwapBuffers();
 }
 
 void SingleThreadProxy::OnDrawForOutputSurface() {
   NOTREACHED() << "Implemented by ThreadProxy for synchronous compositor.";
 }
 
 void SingleThreadProxy::CompositeImmediately(base::TimeTicks frame_begin_time) {
   TRACE_EVENT0("cc,benchmark", "SingleThreadProxy::CompositeImmediately");
   DCHECK(Proxy::IsMainThread());
+  DCHECK(!inside_impl_frame_);
   base::AutoReset<bool> inside_composite(&inside_synchronous_composite_, true);
 
   if (layer_tree_host_->output_surface_lost()) {
     RequestNewOutputSurface();
     // RequestNewOutputSurface could have synchronously created an output
     // surface, so check again before returning.
     if (layer_tree_host_->output_surface_lost())
       return;
   }
 
+  BeginFrameArgs begin_frame_args(BeginFrameArgs::Create(
+      BEGINFRAME_FROM_HERE, frame_begin_time, base::TimeTicks(),
+      BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL));
+
+  // Start the impl frame.
   {
-    BeginFrameArgs begin_frame_args(BeginFrameArgs::Create(
-        BEGINFRAME_FROM_HERE, frame_begin_time, base::TimeTicks(),
-        BeginFrameArgs::DefaultInterval(), BeginFrameArgs::NORMAL));
+    DebugScopedSetImplThread impl(const_cast<SingleThreadProxy*>(this));

[sunnyps, 2015/05/01 00:46:38]

Why do you need a const_cast here? 'this' is not const in a non-const method,
right? If this is not needed, can you change the other uses of const_cast in
this method too?

[mithro-old, 2015/05/01 03:08:54]

Done.

+    WillBeginImplFrame(begin_frame_args);
+  }
+
+  // Run the "main thread" and get it to commit.
+  {
+    DCHECK(inside_impl_frame_);

[sunnyps, 2015/05/01 00:46:38]

nit: move this DCHECK to immediately under the WillBeginImplFrame.

[mithro-old, 2015/05/01 03:08:54]

This DCHCEK is checking that when you call DoBeginMainFrame we are inside an
impl frame. This means it should be logically in the same scope as the
DoBeginMainFrame (IE if you were to split the stuff out into it's own function
then you'd take the DCHECK with it).

     DoBeginMainFrame(begin_frame_args);
     DoCommit();
 
     DCHECK_EQ(0u, layer_tree_host_->num_queued_swap_promises())
         << "Commit should always succeed and transfer promises.";
   }
 
+  // Finish the impl frame.
   {
     DebugScopedSetImplThread impl(const_cast<SingleThreadProxy*>(this));
     if (layer_tree_host_impl_->settings().impl_side_painting) {
       layer_tree_host_impl_->ActivateSyncTree();
       DCHECK(!layer_tree_host_impl_->active_tree()
                   ->needs_update_draw_properties());
       layer_tree_host_impl_->PrepareTiles();
       layer_tree_host_impl_->SynchronouslyInitializeAllTiles();
     }
 
     DoAnimate();
 
     LayerTreeHostImpl::FrameData frame;
     DoComposite(frame_begin_time, &frame);
 
     // DoComposite could abort, but because this is a synchronous composite
     // another draw will never be scheduled, so break remaining promises.
     layer_tree_host_impl_->active_tree()->BreakSwapPromises(
         SwapPromise::SWAP_FAILS);
+
+    DidBeginImplFrameDeadline();
   }
 }
 
 void SingleThreadProxy::ForceSerializeOnSwapBuffers() {
   {
     DebugScopedSetImplThread impl(this);
     if (layer_tree_host_impl_->renderer()) {
       DCHECK(!layer_tree_host_->output_surface_lost());
       layer_tree_host_impl_->renderer()->DoNoOp();
     }
@@ skipping 70 matching lines @@
             "461509 SingleThreadProxy::DoComposite4"));
     layer_tree_host_impl_->DidDrawAllLayers(*frame);
 
     bool start_ready_animations = draw_frame;
     // TODO(robliao): Remove ScopedTracker below once https://crbug.com/461509
     // is fixed.
     tracked_objects::ScopedTracker tracking_profile5(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "461509 SingleThreadProxy::DoComposite5"));
     layer_tree_host_impl_->UpdateAnimationState(start_ready_animations);
-    // TODO(robliao): Remove ScopedTracker below once https://crbug.com/461509
-    // is fixed.
-    tracked_objects::ScopedTracker tracking_profile6(

[sunnyps, 2015/05/01 00:46:38]

This is related to the other CL right?
https://codereview.chromium.org/1111743002/

[mithro-old, 2015/05/01 03:08:55]

Yes, this patch is dependent on that CL. I uploaded it to test on the try bots,
you should be looking at patchset #3 ->
https://codereview.chromium.org/1099703004/#ps40001

Reitveld / Commit Queue doesn't deal with dependent patches :(

-        FROM_HERE_WITH_EXPLICIT_FUNCTION(
-            "461509 SingleThreadProxy::DoComposite6"));
-    layer_tree_host_impl_->ResetCurrentBeginFrameArgsForNextFrame();
 
     // TODO(robliao): Remove ScopedTracker below once https://crbug.com/461509
     // is fixed.
     tracked_objects::ScopedTracker tracking_profile7(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "461509 SingleThreadProxy::DoComposite7"));
     timing_history_.DidFinishDrawing();
   }
 
   if (draw_frame) {
@@ skipping 49 matching lines @@
   scheduler_on_impl_thread_->SetChildrenNeedBeginFrames(
       children_need_begin_frames);
 }
 
 void SingleThreadProxy::SetAuthoritativeVSyncInterval(
     const base::TimeDelta& interval) {
   scheduler_on_impl_thread_->SetAuthoritativeVSyncInterval(interval);
 }
 
 void SingleThreadProxy::WillBeginImplFrame(const BeginFrameArgs& args) {
+  DCHECK(!inside_impl_frame_)
+      << "WillBeginImplFrame called while already inside an impl frame!";
+  inside_impl_frame_ = true;
   layer_tree_host_impl_->WillBeginImplFrame(args);
 }
 
 void SingleThreadProxy::ScheduledActionSendBeginMainFrame() {
   TRACE_EVENT0("cc", "SingleThreadProxy::ScheduledActionSendBeginMainFrame");
   // Although this proxy is single-threaded, it's problematic to synchronously
   // have BeginMainFrame happen after ScheduledActionSendBeginMainFrame.  This
   // could cause a commit to occur in between a series of SetNeedsCommit calls
   // (i.e. property modifications) causing some to fall on one frame and some to
   // fall on the next.  Doing it asynchronously instead matches the semantics of
   // ThreadProxy::SetNeedsCommit where SetNeedsCommit will not cause a
   // synchronous commit.
+  DCHECK(inside_impl_frame_)

[sunnyps, 2015/05/01 00:46:38]

We're pretty sure this is true for SingleThreadProxy, right?

[mithro-old, 2015/05/01 03:08:54]

We'd hope so, but as SRE says "hope is not a strategy" so we check. Then when
someone breaks the assumption this will fail before they commit it.

+      << "BeginMainFrame should only be sent inside a BeginImplFrame";
+  const BeginFrameArgs& begin_frame_args =
+      layer_tree_host_impl_->CurrentBeginFrameArgs();
+
   MainThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&SingleThreadProxy::BeginMainFrame,
-                 weak_factory_.GetWeakPtr()));
+      FROM_HERE, base::Bind(&SingleThreadProxy::BeginMainFrame,
+                            weak_factory_.GetWeakPtr(), begin_frame_args));
 }
 
 void SingleThreadProxy::SendBeginMainFrameNotExpectedSoon() {
   layer_tree_host_->BeginMainFrameNotExpectedSoon();
 }
 
-void SingleThreadProxy::BeginMainFrame() {
+void SingleThreadProxy::BeginMainFrame(const BeginFrameArgs& begin_frame_args) {
   commit_requested_ = false;
   animate_requested_ = false;
 
   if (defer_commits_) {
     TRACE_EVENT_INSTANT0("cc", "EarlyOut_DeferCommit",
                          TRACE_EVENT_SCOPE_THREAD);
     BeginMainFrameAbortedOnImplThread(
         CommitEarlyOutReason::ABORTED_DEFERRED_COMMIT);
     return;
   }
@@ skipping 15 matching lines @@
     BeginMainFrameAbortedOnImplThread(
         CommitEarlyOutReason::ABORTED_OUTPUT_SURFACE_LOST);
     return;
   }
 
   // Prevent new commits from being requested inside DoBeginMainFrame.
   // Note: We do not want to prevent SetNeedsAnimate from requesting
   // a commit here.
   commit_requested_ = true;
 
-  const BeginFrameArgs& begin_frame_args =
-      layer_tree_host_impl_->CurrentBeginFrameArgs();
   DoBeginMainFrame(begin_frame_args);
 }
 
 void SingleThreadProxy::DoBeginMainFrame(
     const BeginFrameArgs& begin_frame_args) {
   layer_tree_host_->WillBeginMainFrame();
   layer_tree_host_->BeginMainFrame(begin_frame_args);
   layer_tree_host_->AnimateLayers(begin_frame_args.frame_time);
   layer_tree_host_->Layout();
 
@@ skipping 98 matching lines @@
 base::TimeDelta SingleThreadProxy::BeginMainFrameToCommitDurationEstimate() {
   return timing_history_.BeginMainFrameToCommitDurationEstimate();
 }
 
 base::TimeDelta SingleThreadProxy::CommitToActivateDurationEstimate() {
   return timing_history_.CommitToActivateDurationEstimate();
 }
 
 void SingleThreadProxy::DidBeginImplFrameDeadline() {
   layer_tree_host_impl_->ResetCurrentBeginFrameArgsForNextFrame();
+  DCHECK(inside_impl_frame_)
+      << "DidBeginImplFrameDeadline called while not inside an impl frame!";
+  inside_impl_frame_ = false;
 }
 
 void SingleThreadProxy::SendBeginFramesToChildren(const BeginFrameArgs& args) {
   layer_tree_host_->SendBeginFramesToChildren(args);
 }
 
 }  // namespace cc