cc: Control defer_commits logic by Scheduler

cc/trees/single_thread_proxy.cc

Index: cc/trees/single_thread_proxy.cc
diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc
index f15c7ec269256105167466e691e5d94d306d60a4..486260d8476a6978ee9b82ddbf053e0160dc90a6 100644
--- a/cc/trees/single_thread_proxy.cc
+++ b/cc/trees/single_thread_proxy.cc
@@ -44,7 +44,6 @@ SingleThreadProxy::SingleThreadProxy(
       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),
@@ -300,10 +299,7 @@ void SingleThreadProxy::SetDeferCommits(bool defer_commits) {
     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 {
@@ -660,26 +656,25 @@ void SingleThreadProxy::ScheduledActionSendBeginMainFrame() {
 }
 
 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);
-    BeginMainFrameAbortedOnImplThread();
-    return;
-  }
-
-  if (layer_tree_host_->output_surface_lost()) {
+  } 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;
   }