cc: Control defer_commits logic by Scheduler

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/debug/trace_event.h"
 #include "cc/debug/benchmark_instrumentation.h"
 #include "cc/output/context_provider.h"
@@ skipping 26 matching lines @@
     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_draw_(false),
       defer_commits_(false),
-      commit_was_deferred_(false),
       commit_requested_(false),
       inside_synchronous_composite_(false),
       output_surface_creation_requested_(false),
       external_begin_frame_source_(external_begin_frame_source.Pass()),
       weak_factory_(this) {
   TRACE_EVENT0("cc", "SingleThreadProxy::SingleThreadProxy");
   DCHECK(Proxy::IsMainThread());
   DCHECK(layer_tree_host);
 }
 
@@ skipping 235 matching lines @@
     return;
   if (defer_commits_ == defer_commits)
     return;
 
   if (defer_commits)
     TRACE_EVENT_ASYNC_BEGIN0("cc", "SingleThreadProxy::SetDeferCommits", this);
   else
     TRACE_EVENT_ASYNC_END0("cc", "SingleThreadProxy::SetDeferCommits", this);
 
   defer_commits_ = defer_commits;
-  if (!defer_commits_ && commit_was_deferred_) {
-    commit_was_deferred_ = false;
-    BeginMainFrame();
-  }
+  scheduler_on_impl_thread_->SetDeferCommits(defer_commits);
 }
 
 bool SingleThreadProxy::CommitRequested() const {
   DCHECK(Proxy::IsMainThread());
   return commit_requested_;
 }
 
 bool SingleThreadProxy::BeginMainFrameRequested() const {
   DCHECK(Proxy::IsMainThread());
   // If there is no scheduler, then there can be no pending begin frame,
@@ skipping 336 matching lines @@
   // fall on the next.  Doing it asynchronously instead matches the semantics of
   // ThreadProxy::SetNeedsCommit where SetNeedsCommit will not cause a
   // synchronous commit.
   MainThreadTaskRunner()->PostTask(
       FROM_HERE,
       base::Bind(&SingleThreadProxy::BeginMainFrame,
                  weak_factory_.GetWeakPtr()));
 }
 
 void SingleThreadProxy::BeginMainFrame() {
-  if (defer_commits_) {
-    DCHECK(!commit_was_deferred_);
-    commit_was_deferred_ = true;
-    layer_tree_host_->DidDeferCommit();
-    return;
-  }
-
   // This checker assumes NotifyReadyToCommit in this stack causes a synchronous
   // commit.
   ScopedAbortRemainingSwapPromises swap_promise_checker(layer_tree_host_);
 
-  if (!layer_tree_host_->visible()) {
+  bool need_to_abort_begin_main_frame = true;
+  if (defer_commits_) {

[danakj, 2014/11/19 16:31:34]

nit: i mildly prefer a few early outs/returns instead of a elseif elseif else
chain.

[simonhong, 2014/11/26 15:52:53]

Done.

+    TRACE_EVENT_INSTANT0("cc", "EarlyOut_DeferCommits",
+                         TRACE_EVENT_SCOPE_THREAD);
+    layer_tree_host_->DidDeferCommit();

[brianderson, 2014/11/19 19:55:07]

It looks like LTH::DidDeferCommit() is only used by tests.

After this patch, I think it'll be racy whether or not DidDeferCommit gets
called anymore, since the scheduler may never send a BeginMainFrame.

Should we get rid of LTH::DidDeferCommit and make tests rely on some other way
of aborting commits, such as setting the visibility instead?

LayerTreeHostTestDeferCommits will also need to change. Probably with some kind
of timeout that verifies WillBeginMainFrame isn't called after commits are
deferred.

[simonhong, 2014/11/26 15:52:53]

This change breaks below three tests.
* LayerTreeHostTestDeferCommits
* LayerTreeHostTestBreakSwapPromiseForVisibilityAbortedCommit
* LayerTreeHostTestBreakSwapPromiseForContextAbortedCommit

I think we don't need some tests for defer commit in here because all defer
commit logic is moved to Scheduler.
I removed above two tests(*BreakSwap*) and I added some test in SchedulerTest
and SchedulerStateMachineTest.
Does this make sense?

[brianderson, 2014/12/03 02:12:10]

The scheduler tests are good, but I think we should keep the LTH tests too. They
cover different things.

+  } else if (!layer_tree_host_->visible()) {
     TRACE_EVENT_INSTANT0("cc", "EarlyOut_NotVisible", TRACE_EVENT_SCOPE_THREAD);
+  } else if (layer_tree_host_->output_surface_lost()) {
+    TRACE_EVENT_INSTANT0(
+        "cc", "EarlyOut_OutputSurfaceLost", TRACE_EVENT_SCOPE_THREAD);
+  } else {
+    need_to_abort_begin_main_frame = false;
+  }
+
+  if (need_to_abort_begin_main_frame) {
     BeginMainFrameAbortedOnImplThread();
     return;
   }
-
-  if (layer_tree_host_->output_surface_lost()) {
-    TRACE_EVENT_INSTANT0(
-        "cc", "EarlyOut_OutputSurfaceLost", TRACE_EVENT_SCOPE_THREAD);
-    BeginMainFrameAbortedOnImplThread();
-    return;
-  }
 
   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);
@@ skipping 101 matching lines @@
 
 base::TimeDelta SingleThreadProxy::CommitToActivateDurationEstimate() {
   return timing_history_.CommitToActivateDurationEstimate();
 }
 
 void SingleThreadProxy::DidBeginImplFrameDeadline() {
   layer_tree_host_impl_->ResetCurrentBeginFrameArgsForNextFrame();
 }
 
 }  // namespace cc