Android: Move scheduling logic to CompositorImpl

content/browser/renderer_host/compositor_impl_android.cc

Index: content/browser/renderer_host/compositor_impl_android.cc
diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc
index 2a168a89c55a45ce1cd0d9bd8ee1b7a8f0df146d..3a59a6e63468ae39679270f661afd436f8aaf7db 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -238,7 +238,12 @@ CompositorImpl::CompositorImpl(CompositorClient* client,
       window_(NULL),
       surface_id_(0),
       client_(client),
-      root_window_(root_window) {
+      root_window_(root_window),
+      did_post_swapbuffers_(false),
+      need_to_composite_(false),
+      can_composite_outside_vsync_(true),
+      pending_swapbuffers_(0U),
+      weak_factory_(this) {
   DCHECK(client);
   DCHECK(root_window);
   ImageTransportFactoryAndroid::AddObserver(this);
@@ -252,9 +257,36 @@ CompositorImpl::~CompositorImpl() {
   SetSurface(NULL);
 }
 
-void CompositorImpl::Composite() {
-  if (host_)
-    host_->Composite(gfx::FrameTime::Now());
+void CompositorImpl::Composite(bool is_vsync) {
+  if (!host_)
+    return;
+
+  if (!need_to_composite_)
+    return;
+
+  if (!is_vsync && !can_composite_outside_vsync_) {
+    TRACE_EVENT0("compositor", "CompositorImpl_DeferComposite");
+    root_window_->RequestVSyncUpdate();
+    return;
+  }
+
+  const unsigned int kMaxSwapBuffers = 2U;
+  DCHECK_LE(pending_swapbuffers_, kMaxSwapBuffers);
+  if (pending_swapbuffers_ == kMaxSwapBuffers) {
+    TRACE_EVENT0("compositor", "CompositorImpl_SwapLimit");
+    root_window_->RequestVSyncUpdate();
+    return;
+  }
+
+  // Don't allow compositing immediately more than once per vsync.
+  can_composite_outside_vsync_ = false;
+
+  did_post_swapbuffers_ = false;
+  host_->Composite(gfx::FrameTime::Now());
+  if (did_post_swapbuffers_)
+    pending_swapbuffers_++;
+
+  need_to_composite_ = false;
 }
 
 void CompositorImpl::SetRootLayer(scoped_refptr<cc::Layer> root_layer) {
@@ -465,6 +497,10 @@ CreateGpuProcessViewContext(
                                                 NULL));
 }
 
+void CompositorImpl::Layout() {
+  client_->Layout();
+}
+
 scoped_ptr<cc::OutputSurface> CompositorImpl::CreateOutputSurface(
     bool fallback) {
   blink::WebGraphicsContext3D::Attributes attrs;
@@ -498,12 +534,16 @@ scoped_refptr<cc::ContextProvider> CompositorImpl::OffscreenContextProvider() {
   return NULL;
 }
 
-void CompositorImpl::DidCompleteSwapBuffers() {
-  client_->OnSwapBuffersCompleted();
-}
-
 void CompositorImpl::ScheduleComposite() {
-  client_->ScheduleComposite();
+  // We already scheduled something.

[Sami, 2014/04/16 15:16:30]

The consumePendingRendererFrame() logic made sure this only took the fast path
if there was a new frame to composite. IIRC I had to add it because something in
the browser (loading spinner, tab switcher, dunno) was often scheduling
compositing and kicking the renderer into high latency mode. It would be good to
double check this doesn't happen with this patch.

+  if (need_to_composite_)
+    return;
+
+  need_to_composite_ = true;
+  base::MessageLoop::current()->PostTask(

[no sievers, 2014/04/16 02:17:28]

It definitely loses the 'post to front of queue' behavior you had added here for
an issue. Is it still necessary?
If so we need to figure out how we handle this with the cc scheduler.

[Sami, 2014/04/16 15:16:30]

I *think* it's still an issue (here's the original description for reference:
https://gerrit-int.chromium.org/43697). It should be fairly obvious from a trace
when it happens. Basically the browser compositor for frame N is delayed by
input for frame N+1. Maybe we could throttle the input to avoid that situation.

[no sievers, 2014/05/09 00:30:05]

Do you mind if I follow up on this separately? Handling this the other way
around by deferring input or so sounds good.

+      FROM_HERE,
+      base::Bind(
+          &CompositorImpl::Composite, weak_factory_.GetWeakPtr(), false));
 }
 
 void CompositorImpl::ScheduleAnimation() {
@@ -513,11 +553,19 @@ void CompositorImpl::ScheduleAnimation() {
 void CompositorImpl::DidPostSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidPostSwapBuffers");
   client_->OnSwapBuffersPosted();
+  did_post_swapbuffers_ = true;
+}
+
+void CompositorImpl::DidCompleteSwapBuffers() {
+  TRACE_EVENT0("compositor", "CompositorImpl::DidCompleteSwapBuffers");
+  DCHECK_GT(pending_swapbuffers_, 0U);
+  pending_swapbuffers_--;
 }
 
 void CompositorImpl::DidAbortSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidAbortSwapBuffers");
-  client_->OnSwapBuffersCompleted();
+  DCHECK_GT(pending_swapbuffers_, 0U);
+  pending_swapbuffers_--;
 }
 
 void CompositorImpl::DidCommit() {
@@ -528,4 +576,10 @@ void CompositorImpl::AttachLayerForReadback(scoped_refptr<cc::Layer> layer) {
   root_layer_->AddChild(layer);
 }
 
+void CompositorImpl::OnVSync(base::TimeTicks frame_time,
+                             base::TimeDelta vsync_period) {
+  Composite(true);
+  can_composite_outside_vsync_ = true;
+}
+
 }  // namespace content