cc: Control defer_commits logic by Scheduler

cc/trees/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/thread_proxy.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 419 matching lines @@
   DCHECK(IsMainThread());
   if (main().defer_commits == defer_commits)
     return;
 
   main().defer_commits = defer_commits;
   if (main().defer_commits)
     TRACE_EVENT_ASYNC_BEGIN0("cc", "ThreadProxy::SetDeferCommits", this);
   else
     TRACE_EVENT_ASYNC_END0("cc", "ThreadProxy::SetDeferCommits", this);
 
-  if (!main().defer_commits && main().pending_deferred_commit) {
-    Proxy::MainThreadTaskRunner()->PostTask(
-        FROM_HERE,
-        base::Bind(&ThreadProxy::BeginMainFrame,
-                   main_thread_weak_ptr_,
-                   base::Passed(&main().pending_deferred_commit)));
-  }
+  Proxy::ImplThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&ThreadProxy::SetDeferCommitsOnImplThread,
+                 impl_thread_weak_ptr_,
+                 defer_commits));
+}
+
+void ThreadProxy::SetDeferCommitsOnImplThread(bool defer_commits) const {
+  DCHECK(IsImplThread());
+  impl().scheduler->SetDeferCommits(defer_commits);
 }
 
 bool ThreadProxy::CommitRequested() const {
   DCHECK(IsMainThread());
   return main().commit_requested;
 }
 
 bool ThreadProxy::BeginMainFrameRequested() const {
   DCHECK(IsMainThread());
   return main().commit_request_sent_to_impl_thread;
@@ skipping 238 matching lines @@
 }
 
 void ThreadProxy::BeginMainFrame(
     scoped_ptr<BeginMainFrameAndCommitState> begin_main_frame_state) {
   benchmark_instrumentation::ScopedBeginFrameTask begin_frame_task(
       benchmark_instrumentation::kDoBeginFrame,
       begin_main_frame_state->begin_frame_id);
   TRACE_EVENT_SYNTHETIC_DELAY_BEGIN("cc.BeginMainFrame");
   DCHECK(IsMainThread());
 
-  if (main().defer_commits) {
-    main().pending_deferred_commit = begin_main_frame_state.Pass();
-    layer_tree_host()->DidDeferCommit();
-    TRACE_EVENT_INSTANT0(
-        "cc", "EarlyOut_DeferCommits", TRACE_EVENT_SCOPE_THREAD);
-    return;
-  }
-
   // If the commit finishes, LayerTreeHost will transfer its swap promises to
   // LayerTreeImpl. The destructor of ScopedSwapPromiseChecker aborts the
   // remaining swap promises.
   ScopedAbortRemainingSwapPromises swap_promise_checker(layer_tree_host());

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

This patch reorders the deferred commit early out WRT the swap promise checker,
meaning swap promises would be aborted on a deferred commit when before they
would not be. And now it would send a BeginFrameAborted, as well. Is this
intentional? Is it the right thing to do?

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

We should keep the original behavior and not fulfill the swap promises on
deferred commit. My understanding is that the promises are used to track input
latency. If we abort because of a deferred commit, we are deferring input as
well. The other reasons we abort are because we aren't going to draw.

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

As brian said, Aborting the BeginMainFrame when deferring commit is true is
intentional.
I want to handle deferring BeginMainFrame by aborting it and recreating new
BeginMainFrame
when deferring is canceled.
And I didn't think much about re-ordering swap promise.
It should work like before.

 
   main().commit_requested = false;
   main().commit_request_sent_to_impl_thread = false;
   main().animate_requested = false;
 
-  if (!layer_tree_host()->visible()) {
+  bool need_to_abort_begin_main_frame = true;
+  if (main().defer_commits) {
+    layer_tree_host()->DidDeferCommit();
+    TRACE_EVENT_INSTANT0("cc",
+                         "EarlyOut_DeferCommits",
+                         TRACE_EVENT_SCOPE_THREAD);

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

If we abort a commit because we are deferring commits, we should probably call
SetNeedsCommit to make sure we eventually get another BeginMainFrame. Otherwise
we are assuming blink will SetNeedsCommit when it stops deferring commits, which
I don't think is a safe assumption to make. Even if it is a true assumption
today, it may not be in the future.

Same for the SingleThreadProxy.

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

SetNeedsCommit() is added when SetDeferCommits(false) is called (when changed
from true to false).

+  } 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) {
     bool did_handle = false;
     Proxy::ImplThreadTaskRunner()->PostTask(
         FROM_HERE,
-        base::Bind(&ThreadProxy::BeginMainFrameAbortedOnImplThread,
-                   impl_thread_weak_ptr_,
-                   did_handle));
-    return;
-  }
-
-  if (layer_tree_host()->output_surface_lost()) {
-    TRACE_EVENT_INSTANT0(
-        "cc", "EarlyOut_OutputSurfaceLost", TRACE_EVENT_SCOPE_THREAD);
-    bool did_handle = false;
-    Proxy::ImplThreadTaskRunner()->PostTask(
-        FROM_HERE,
         base::Bind(&ThreadProxy::BeginMainFrameAbortedOnImplThread,
                    impl_thread_weak_ptr_,
                    did_handle));
     return;
   }
 
   // Do not notify the impl thread of commit requests that occur during
   // the apply/animate/layout part of the BeginMainFrameAndCommit process since
   // those commit requests will get painted immediately. Once we have done
   // the paint, main().commit_requested will be set to false to allow new commit
@@ skipping 614 matching lines @@
 
   impl().timing_history.DidActivateSyncTree();
 }
 
 void ThreadProxy::DidManageTiles() {
   DCHECK(IsImplThread());
   impl().scheduler->DidManageTiles();
 }
 
 }  // namespace cc