Make Android CompositorImpl use scheduled SingleThreadProxy

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 f471fbc83171651a0667f2e181c49aed9e9f0ef5..7930407421446aebdfb0639987454c3daf0b16be 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -194,13 +194,7 @@ CompositorImpl::CompositorImpl(CompositorClient* client,
       surface_id_(0),
       client_(client),
       root_window_(root_window),
-      did_post_swapbuffers_(false),
-      ignore_schedule_composite_(false),
-      needs_composite_(false),
-      needs_animate_(false),
-      will_composite_immediately_(false),
-      composite_on_vsync_trigger_(DO_NOT_COMPOSITE),
-      pending_swapbuffers_(0U),
+      pending_swapbuffers_(0),
       weak_factory_(this) {
   DCHECK(client);
   DCHECK(root_window);
@@ -215,124 +209,9 @@ CompositorImpl::~CompositorImpl() {
   SetSurface(NULL);
 }
 
-void CompositorImpl::PostComposite(CompositingTrigger trigger) {
-  DCHECK(needs_composite_);
-  DCHECK(trigger == COMPOSITE_IMMEDIATELY || trigger == COMPOSITE_EVENTUALLY);
-
-  if (will_composite_immediately_ ||
-      (trigger == COMPOSITE_EVENTUALLY && WillComposite())) {
-    // We will already composite soon enough.
-    DCHECK(WillComposite());
-    return;
-  }
-
-  if (DidCompositeThisFrame()) {
-    DCHECK(!WillCompositeThisFrame());
-    if (composite_on_vsync_trigger_ != COMPOSITE_IMMEDIATELY) {
-      composite_on_vsync_trigger_ = trigger;
-      root_window_->RequestVSyncUpdate();
-    }
-    DCHECK(WillComposite());
-    return;
-  }
-
-  base::TimeDelta delay;
-  if (trigger == COMPOSITE_IMMEDIATELY) {
-    will_composite_immediately_ = true;
-    composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
-  } else {
-    DCHECK(!WillComposite());
-    const base::TimeDelta estimated_composite_time = vsync_period_ / 4;
-    const base::TimeTicks now = base::TimeTicks::Now();
-
-    if (!last_vsync_.is_null() && (now - last_vsync_) < vsync_period_) {
-      base::TimeTicks next_composite =
-          last_vsync_ + vsync_period_ - estimated_composite_time;
-      if (next_composite < now) {
-        // It's too late, we will reschedule composite as needed on the next
-        // vsync.
-        composite_on_vsync_trigger_ = COMPOSITE_EVENTUALLY;
-        root_window_->RequestVSyncUpdate();
-        DCHECK(WillComposite());
-        return;
-      }
-
-      delay = next_composite - now;
-    }
-  }
-  TRACE_EVENT2("cc", "CompositorImpl::PostComposite",
-               "trigger", trigger,
-               "delay", delay.InMillisecondsF());
-
-  DCHECK(composite_on_vsync_trigger_ == DO_NOT_COMPOSITE);
-  if (current_composite_task_)
-    current_composite_task_->Cancel();
-
-  // Unretained because we cancel the task on shutdown.
-  current_composite_task_.reset(new base::CancelableClosure(
-      base::Bind(&CompositorImpl::Composite, base::Unretained(this), trigger)));
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE, current_composite_task_->callback(), delay);
-}
-
-void CompositorImpl::Composite(CompositingTrigger trigger) {
-  BrowserGpuChannelHostFactory* factory =
-      BrowserGpuChannelHostFactory::instance();
-  if (!factory->GetGpuChannel() || factory->GetGpuChannel()->IsLost()) {
-    CauseForGpuLaunch cause =
-        CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE;
-    factory->EstablishGpuChannel(
-        cause,
-        base::Bind(&CompositorImpl::OnGpuChannelEstablished,
-                   weak_factory_.GetWeakPtr()));
-    return;
-  }
-
-  DCHECK(host_);
-  DCHECK(trigger == COMPOSITE_IMMEDIATELY || trigger == COMPOSITE_EVENTUALLY);
-  DCHECK(needs_composite_);
-  DCHECK(!DidCompositeThisFrame());
-
-  if (trigger == COMPOSITE_IMMEDIATELY)
-    will_composite_immediately_ = false;
-
-  DCHECK_LE(pending_swapbuffers_, kMaxSwapBuffers);
-  if (pending_swapbuffers_ == kMaxSwapBuffers) {
-    TRACE_EVENT0("compositor", "CompositorImpl_SwapLimit");
-    return;
-  }
-
-  // Reset state before Layout+Composite since that might create more
-  // requests to Composite that we need to respect.
-  needs_composite_ = false;
-
-  // Only allow compositing once per vsync.
-  current_composite_task_->Cancel();
-  DCHECK(DidCompositeThisFrame() && !WillComposite());
-
-  // Ignore ScheduleComposite() from layer tree changes during layout and
-  // animation updates that will already be reflected in the current frame
-  // we are about to draw.
-  ignore_schedule_composite_ = true;
-
-  const base::TimeTicks frame_time = gfx::FrameTime::Now();
-  if (needs_animate_) {
-    needs_animate_ = false;
-    root_window_->Animate(frame_time);
-  }
-  ignore_schedule_composite_ = false;
-
-  did_post_swapbuffers_ = false;
-  host_->Composite(frame_time);
-  if (did_post_swapbuffers_)
-    pending_swapbuffers_++;
-
-  // Need to track vsync to avoid compositing more than once per frame.
-  root_window_->RequestVSyncUpdate();
-}
-
 void CompositorImpl::OnGpuChannelEstablished() {
-  ScheduleComposite();
+  if (host_)
+    host_->SetNeedsCommit();

[brianderson, 2014/06/24 23:50:12]

Should this be SetNeedsRedraw? If this is meant to trigger the first commit, the
scheduler will do that automatically when there is a new OutputSurface.

[danakj, 2014/06/25 17:28:54]

This should probably just go away?

 }
 
 UIResourceProvider& CompositorImpl::GetUIResourceProvider() {
@@ -394,14 +273,9 @@ void CompositorImpl::SetSurface(jobject surface) {
 
 void CompositorImpl::SetVisible(bool visible) {
   if (!visible) {
-    if (WillComposite())
-      CancelComposite();
     ui_resource_provider_.SetLayerTreeHost(NULL);
     host_.reset();
   } else if (!host_) {
-    DCHECK(!WillComposite());
-    needs_composite_ = false;
-    needs_animate_ = false;
     pending_swapbuffers_ = 0;
     cc::LayerTreeSettings settings;
     settings.refresh_rate = 60.0;
@@ -416,8 +290,6 @@ void CompositorImpl::SetVisible(bool visible) {
         command_line->HasSwitch(cc::switches::kEnableGpuBenchmarking));
     settings.initial_debug_state.show_fps_counter =
         command_line->HasSwitch(cc::switches::kUIShowFPSCounter);
-    // TODO(enne): Update this this compositor to use the scheduler.
-    settings.single_thread_proxy_scheduler = false;
 
     host_ = cc::LayerTreeHost::CreateSingleThreaded(
         this, this, HostSharedBitmapManager::current(), settings);
@@ -429,6 +301,17 @@ void CompositorImpl::SetVisible(bool visible) {
     host_->set_has_transparent_background(has_transparent_background_);
     host_->SetDeviceScaleFactor(device_scale_factor_);
     ui_resource_provider_.SetLayerTreeHost(host_.get());
+
+    BrowserGpuChannelHostFactory* factory =
+        BrowserGpuChannelHostFactory::instance();
+    if (!factory->GetGpuChannel() || factory->GetGpuChannel()->IsLost()) {
+      CauseForGpuLaunch cause =
+          CAUSE_FOR_GPU_LAUNCH_WEBGRAPHICSCONTEXT3DCOMMANDBUFFERIMPL_INITIALIZE;
+      factory->EstablishGpuChannel(
+          cause,
+          base::Bind(&CompositorImpl::OnGpuChannelEstablished,
+                     weak_factory_.GetWeakPtr()));

[no sievers, 2014/06/19 17:13:26]

We then need to defer the host_->SetLayerTreeHostClientReady() from line 299 to
OnGpuChannelEstablished() also or factory->GetGpuChannel() in
CreateOutputSurface() races with this and might return NULL.

Also it wouldn't work when we lost the channel.
Unfortunately on Android we also cannot use EstablishGpuChannelSync when you run
the GPU thread in its own process, since it would deadlock (opening the channel
needs to synchronously go back from the gpu thread to the main UI thread here).

That's why I had the bail-out hack in Composite(), but now it seems we need to
propagate this to cc to not try to create the OutputSurface after we lost the
context and until we are ready again.

+    }
   }
 }
 
@@ -457,10 +340,7 @@ void CompositorImpl::SetHasTransparentBackground(bool flag) {
 void CompositorImpl::SetNeedsComposite() {
   if (!host_.get())
     return;
-  DCHECK(!needs_composite_ || WillComposite());
-
-  needs_composite_ = true;
-  PostComposite(COMPOSITE_IMMEDIATELY);
+  host_->SetNeedsCommit();
 }
 
 static scoped_ptr<WebGraphicsContext3DCommandBufferImpl>
@@ -496,9 +376,9 @@ CreateGpuProcessViewContext(
 }
 
 void CompositorImpl::Layout() {
-  ignore_schedule_composite_ = true;
+  host_->SetDeferCommits(true);
   client_->Layout();
-  ignore_schedule_composite_ = false;
+  host_->SetDeferCommits(false);
 }
 
 scoped_ptr<cc::OutputSurface> CompositorImpl::CreateOutputSurface(
@@ -534,42 +414,19 @@ void CompositorImpl::OnLostResources() {
   ui_resource_provider_.UIResourcesAreInvalid();
 }
 
-void CompositorImpl::ScheduleComposite() {
-  DCHECK(!needs_composite_ || WillComposite());
-  if (ignore_schedule_composite_)
-    return;
-
-  needs_composite_ = true;
-  // We currently expect layer tree invalidations at most once per frame
-  // during normal operation and therefore try to composite immediately
-  // to minimize latency.
-  PostComposite(COMPOSITE_IMMEDIATELY);
-}
-
-void CompositorImpl::ScheduleAnimation() {
-  DCHECK(!needs_animate_ || needs_composite_);
-  DCHECK(!needs_composite_ || WillComposite());
-  needs_animate_ = true;
-
-  if (needs_composite_)
-    return;
-
-  TRACE_EVENT0("cc", "CompositorImpl::ScheduleAnimation");
-  needs_composite_ = true;
-  PostComposite(COMPOSITE_EVENTUALLY);
-}
-
 void CompositorImpl::DidPostSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidPostSwapBuffers");
-  did_post_swapbuffers_ = true;
+  pending_swapbuffers_++;
 }
 
 void CompositorImpl::DidCompleteSwapBuffers() {
   TRACE_EVENT0("compositor", "CompositorImpl::DidCompleteSwapBuffers");
   DCHECK_GT(pending_swapbuffers_, 0U);
-  if (pending_swapbuffers_-- == kMaxSwapBuffers && needs_composite_)
-    PostComposite(COMPOSITE_IMMEDIATELY);
+  pending_swapbuffers_--;
   client_->OnSwapBuffersCompleted(pending_swapbuffers_);

[brianderson, 2014/06/24 23:50:12]

Can we get rid of pending_swapbuffers_? Implementations of
OnSwapBuffersCompleted all seem to ignore the argument anyway.

Then we can get rid of CompositorImpl::DidPostSwapBuffers().

Also, do we need to call client_->OnSwapBuffersCompleted() here? It looks like
Android only uses it to call mSurfaceView.SetBackgroundResource(0):
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/and...

@sievers: What is that for? Can it be triggered any other way?

[no sievers, 2014/06/25 00:03:26]

There is one call site in Chrome for Android, where it checks
pending_swapbuffers_ to be 0 to defer and throttle native Android view
invalidations. However, I think we can do that better now without having to look
at swap completion.

+
+  // TODO(enne): where does this go?
+  root_window_->RequestVSyncUpdate();

[enne (OOO), 2014/06/19 01:09:27]

This maybe could be in a better place.

[no sievers, 2014/06/19 17:49:21]

...then line 429 can go away here...

 }
 
 void CompositorImpl::DidAbortSwapBuffers() {
@@ -577,7 +434,6 @@ void CompositorImpl::DidAbortSwapBuffers() {
   // This really gets called only once from
   // SingleThreadProxy::DidLoseOutputSurfaceOnImplThread() when the
   // context was lost.
-  ScheduleComposite();
   client_->OnSwapBuffersCompleted(0);
 }
 
@@ -596,25 +452,7 @@ void CompositorImpl::RequestCopyOfOutputOnRootLayer(
 
 void CompositorImpl::OnVSync(base::TimeTicks frame_time,
                              base::TimeDelta vsync_period) {
-  vsync_period_ = vsync_period;
-  last_vsync_ = frame_time;
-
-  if (WillCompositeThisFrame()) {
-    // We somehow missed the last vsync interval, so reschedule for deadline.
-    // We cannot schedule immediately, or will get us out-of-phase with new
-    // renderer frames.
-    CancelComposite();
-    composite_on_vsync_trigger_ = COMPOSITE_EVENTUALLY;
-  } else {
-    current_composite_task_.reset();
-  }
-
-  DCHECK(!DidCompositeThisFrame() && !WillCompositeThisFrame());
-  if (composite_on_vsync_trigger_ != DO_NOT_COMPOSITE) {
-    CompositingTrigger trigger = composite_on_vsync_trigger_;
-    composite_on_vsync_trigger_ = DO_NOT_COMPOSITE;
-    PostComposite(trigger);
-  }
+  // TODO(enne): Does this need to get piped to output surface somehow?

[enne (OOO), 2014/06/19 01:09:27]

brianderson: I'm a little unclear about vsync parameters.  Is this something
that this compositor should be responsible for somehow routing to
Scheduler::CommitVSyncParameters, or does that happen elsewhere that I'm not
seeing?

[no sievers, 2014/06/19 17:49:21]

...and it would take care of the vsync scheduling. When OnVSync happens and
|needs_begin_frame| is true, then we should call
OutputSurfaceClient(LTHImpl)::BeginFrame().

Brian, does that make sense at all?

[brianderson, 2014/06/24 23:50:12]

Yes, that makes sense and is how the scheduler expects the OutputSurface to
behave.

root_window_->RequestVSyncUpdate() should be called in two cases:
SetNeedsBeginFrame goes from false to true.
Immediately from the vsync callback when if SetNeedsBeginFrame is still true.
The scheduler will automatically handle BeginFrames that pile up.

 }
 
 void CompositorImpl::SetNeedsAnimate() {