cc: Allow the main thread to cancel commits

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 5e94ebc8f4ab3291fe3b540a6a3a2ffc0369757b..a29b9340dd5c7da8117f0303b3d591aa2f2d841d 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -89,6 +89,7 @@ ThreadProxy::ThreadProxy(
       using_synchronous_renderer_compositor_(
           layer_tree_host->settings().using_synchronous_renderer_compositor),
       inside_draw_(false),
+      can_cancel_commit_(false),
       defer_commits_(false),
       renew_tree_priority_on_impl_thread_pending_(false),
       draw_duration_history_(kDrawDurationHistorySize) {
@@ -125,10 +126,15 @@ bool ThreadProxy::CompositeAndReadback(void* pixels, gfx::Rect rect) {
                    &begin_frame_sent_to_main_thread_completion));
     begin_frame_sent_to_main_thread_completion.Wait();
   }
+
   in_composite_and_readback_ = true;
   BeginFrameOnMainThread(scoped_ptr<BeginFrameAndCommitState>());
   in_composite_and_readback_ = false;
 
+  // Composite and readback requires a second commit to undo any changes
+  // that it made.
+  can_cancel_commit_ = false;
+
   // Perform a synchronous readback.
   ReadbackRequest request;
   request.rect = rect;
@@ -298,6 +304,17 @@ void ThreadProxy::OnOutputSurfaceInitializeAttempted(
   }
 }
 
+void ThreadProxy::SendCommitRequestToImplThreadIfNeeded() {

[brianderson, 2013/07/13 01:26:42]

While you are adding this method, it looks like commit_requested and
commit_request_sent_to_imple_thread_ are redundant. Do you mind removing one?

[enne (OOO), 2013/07/15 22:23:28]

It doesn't look like one can be removed.  commit_requested_ (unlike
commit_request_sent_to_impl_thread_) is also used for
ThreadProxy::CommitRequested and indirectly for LayerTreeHost::CommitRequested.

See https://bugs.webkit.org/show_bug.cgi?id=95393 for why this was introduced in
the first place.

+  DCHECK(IsMainThread());
+  if (commit_request_sent_to_impl_thread_)
+    return;
+  commit_request_sent_to_impl_thread_ = true;
+  Proxy::ImplThreadTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&ThreadProxy::SetNeedsCommitOnImplThread,
+                 impl_thread_weak_ptr_));
+}
+
 const RendererCapabilities& ThreadProxy::GetRendererCapabilities() const {
   DCHECK(IsMainThread());
   DCHECK(!layer_tree_host_->output_surface_lost());
@@ -311,30 +328,28 @@ void ThreadProxy::SetNeedsAnimate() {
 
   TRACE_EVENT0("cc", "ThreadProxy::SetNeedsAnimate");
   animate_requested_ = true;
+  can_cancel_commit_ = false;
+  SendCommitRequestToImplThreadIfNeeded();
+}
 
-  if (commit_request_sent_to_impl_thread_)
-    return;
-  commit_request_sent_to_impl_thread_ = true;
-  Proxy::ImplThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ThreadProxy::SetNeedsCommitOnImplThread,
-                 impl_thread_weak_ptr_));
+void ThreadProxy::SetNeedsUpdateLayers() {
+  DCHECK(IsMainThread());
+  SendCommitRequestToImplThreadIfNeeded();
 }
 
 void ThreadProxy::SetNeedsCommit() {
   DCHECK(IsMainThread());
+  // This needs to unconditionally be set here, because the during

[brianderson, 2013/07/13 01:26:42]

Nit: -the

[enne (OOO), 2013/07/15 22:23:28]

Done.

+  // begin frame commit_requested_ is set unconditionally to true
+  // to suppress extra commits, however we don't want to suppress cancels.

[brianderson, 2013/07/13 01:26:42]

Triple negative made that last part of the comment hard to parse. How about:
", however we still want to be able to prevent cancels."

[enne (OOO), 2013/07/15 22:23:28]

Cleaned up this comment in general.

+  can_cancel_commit_ = false;
+
   if (commit_requested_)
     return;
   TRACE_EVENT0("cc", "ThreadProxy::SetNeedsCommit");
   commit_requested_ = true;
 
-  if (commit_request_sent_to_impl_thread_)
-    return;
-  commit_request_sent_to_impl_thread_ = true;
-  Proxy::ImplThreadTaskRunner()->PostTask(
-      FROM_HERE,
-      base::Bind(&ThreadProxy::SetNeedsCommitOnImplThread,
-                 impl_thread_weak_ptr_));
+  SendCommitRequestToImplThreadIfNeeded();
 }
 
 void ThreadProxy::DidLoseOutputSurfaceOnImplThread() {
@@ -684,10 +699,12 @@ void ThreadProxy::BeginFrameOnMainThread(
     commit_request_sent_to_impl_thread_ = false;
 
     TRACE_EVENT0("cc", "EarlyOut_NotVisible");
+    bool cancel_commit = false;
     Proxy::ImplThreadTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(&ThreadProxy::BeginFrameAbortedByMainThreadOnImplThread,
-                   impl_thread_weak_ptr_));
+                   impl_thread_weak_ptr_,
+                   cancel_commit));
     return;
   }
 
@@ -714,13 +731,27 @@ void ThreadProxy::BeginFrameOnMainThread(
   // UpdateLayers.
   commit_requested_ = false;
   commit_request_sent_to_impl_thread_ = false;
+  bool can_cancel_this_commit =
+      can_cancel_commit_ && !in_composite_and_readback_;
+  can_cancel_commit_ = true;
 
   scoped_ptr<ResourceUpdateQueue> queue =
       make_scoped_ptr(new ResourceUpdateQueue);
-  layer_tree_host_->UpdateLayers(
+  bool updated = layer_tree_host_->UpdateLayers(
       queue.get(),
-      begin_frame_state ?
-          begin_frame_state->memory_allocation_limit_bytes : 0u);
+      begin_frame_state ? begin_frame_state->memory_allocation_limit_bytes
+                        : 0u);
+
+  if (!updated && can_cancel_this_commit) {

[brianderson, 2013/07/13 01:26:42]

If "updated" is false, what else is there to commit after a readback,
SetNeedsAnimate, and SetNeedsCommit? Are there any more cases where we can
cancel the commit?

[enne (OOO), 2013/07/15 22:23:28]

The low-hanging fruit here are the cases I've added in this patch (unhandled
invalidations, page scale, scrolling on the compositor thread).

Follow-up to this patch could be being smarter with SetNeedsAnimate or not
setting SetNeedsCommit when a layer !DrawsContent().

+    TRACE_EVENT0("cc", "EarlyOut_NoUpdates");
+    bool cancel_commit = true;
+    Proxy::ImplThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&ThreadProxy::BeginFrameAbortedByMainThreadOnImplThread,
+                   impl_thread_weak_ptr_,
+                   cancel_commit));
+    return;
+  }
 
   // Once single buffered layers are committed, they cannot be modified until
   // they are drawn by the impl thread.
@@ -827,13 +858,17 @@ void ThreadProxy::StartCommitOnImplThread(
       scheduler_on_impl_thread_->AnticipatedDrawTime());
 }
 
-void ThreadProxy::BeginFrameAbortedByMainThreadOnImplThread() {
+void ThreadProxy::BeginFrameAbortedByMainThreadOnImplThread(
+    bool cancel_commit) {
   TRACE_EVENT0("cc", "ThreadProxy::BeginFrameAbortedByMainThreadOnImplThread");
   DCHECK(IsImplThread());
   DCHECK(scheduler_on_impl_thread_);
   DCHECK(scheduler_on_impl_thread_->CommitPending());
 
-  scheduler_on_impl_thread_->BeginFrameAbortedByMainThread();
+  // The scroll and scale set was applied by the main thread, so the active tree
+  // needs to be updated as if these sent values were applied and committed.
+  layer_tree_host_impl_->active_tree()->ApplySentScrollAndScaleDeltas();
+  scheduler_on_impl_thread_->BeginFrameAbortedByMainThread(cancel_commit);
 }
 
 void ThreadProxy::ScheduledActionCommit() {