Add SwapPromise support to LayerTreeHost and LayerTreeImpl

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 4f2f488eae8e5805ee2caff3dbda557bcc8709da..ae5a02564e4173ce68f5baba9baccb4971c91f86 100644
--- a/cc/trees/thread_proxy.cc
+++ b/cc/trees/thread_proxy.cc
@@ -5,6 +5,7 @@
 #include "cc/trees/thread_proxy.h"
 
 #include <string>
+#include <vector>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ -24,6 +25,8 @@
 #include "cc/trees/layer_tree_impl.h"
 #include "ui/gfx/frame_time.h"
 
+namespace {
+
 // Measured in seconds.
 const double kSmoothnessTakesPriorityExpirationDelay = 0.25;
 
@@ -32,6 +35,22 @@ const double kCommitAndActivationDurationEstimationPercentile = 50.0;
 const double kDrawDurationEstimationPercentile = 100.0;
 const int kDrawDurationEstimatePaddingInMicroseconds = 0;
 
+class SwapPromiseChecker {
+ public:
+  explicit SwapPromiseChecker(cc::LayerTreeHost* layer_tree_host)
+        : layer_tree_host_(layer_tree_host) {}
+
+  ~SwapPromiseChecker() {
+    if (layer_tree_host_->HasQueuedSwapPromise())
+      layer_tree_host_->BreakSwapPromise("COMMIT ABORTED");
+  }
+
+ private:
+  cc::LayerTreeHost* layer_tree_host_;
+};
+
+}  // namespace
+
 namespace cc {
 
 struct ThreadProxy::ReadbackRequest {
@@ -730,6 +749,12 @@ void ThreadProxy::BeginMainFrame(
     return;
   }
 
+  // If the commit finishes, LTH will transfer its swap promises to LTI.
+  // The dtor of SwapPromiseChecker will check LTH's swap promises, and
+  // call the swap aborted callback of the swap promises if the commit
+  // ever aborts.
+  SwapPromiseChecker swap_promise_checker(layer_tree_host_);

[jamesr, 2013/11/12 03:15:04]

it looks like this will invoke the aborted callback on the main thread

[Yufeng Shen (Slow to review), 2013/11/13 21:05:04]

See comments below.

+
   // 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
@@ -1109,6 +1134,10 @@ DrawSwapReadbackResult ThreadProxy::DrawSwapReadbackInternal(
   } else if (draw_frame) {
     DCHECK(swap_requested);
     result.did_swap = layer_tree_host_impl_->SwapBuffers(frame);
+    if (result.did_swap)
+      layer_tree_host_impl_->active_tree()->FinishSwapPromise();
+    else
+      layer_tree_host_impl_->active_tree()->BreakSwapPromise("DID NOT SWAP");

[jamesr, 2013/11/12 03:15:04]

whereas these calls will invoke callbacks from the compositor thread. is the
caller supposed to deal with this themselves? that seems like an extremely
error-prone interface if the callback is run and bound arguments destroyed from
different threads depending on what happens

what if we never make it to this point? for example, what if we lose the
OutputSurface in between the commit and the draw, or become invisible, or any
other case where the scheduler decides not to advance to drawing?

[Yufeng Shen (Slow to review), 2013/11/13 21:05:04]

Right.  I have in the new patch set changed the interface of
SwapPromise so that SwapPromise is now an abstract class that
has virtual interface  DidSwap() and DidNotSwap() which can
be called accordingly when swap buffer happens and fails.

I also put comments/warning that DidNotSwap() can happen
on main/impl thread. So it is better to have the subclass of
SwapPromise to carry the thread-safe member data (e.g. in
LatencyInfo case, copy the LatencyInfo into the a subclass
InputLatencySwapPromise), and so that when DidSwap/DidNotSwap()
are operating on the member data, it does not matter which thread
it is on.
An email thread "Swap Promises" was started for discussing this.

 
     // We don't know if we have incomplete tiles if we didn't actually swap.
     if (result.did_swap) {