android: Add draw completion for CompositorView

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 96dcc517578cc152c9298e9a5e90f67da7d741c5..da2cc35e8b36755073c4f714c0a48dd853374bd6 100644
--- a/content/browser/renderer_host/compositor_impl_android.cc
+++ b/content/browser/renderer_host/compositor_impl_android.cc
@@ -186,9 +186,11 @@ gpu::gles2::ContextCreationAttribHelper GetCompositorContextAttributes(
 
 class AndroidOutputSurface : public cc::OutputSurface {
  public:
-  explicit AndroidOutputSurface(
-      scoped_refptr<ui::ContextProviderCommandBuffer> context_provider)
+  AndroidOutputSurface(
+      scoped_refptr<ui::ContextProviderCommandBuffer> context_provider,
+      base::Closure swap_buffers_callback)
       : cc::OutputSurface(std::move(context_provider)),
+        swap_buffers_callback_(std::move(swap_buffers_callback)),
         overlay_candidate_validator_(
             new display_compositor::
                 CompositorOverlayCandidateValidatorAndroid()),
@@ -268,10 +270,12 @@ class AndroidOutputSurface : public cc::OutputSurface {
       const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
     RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
     client_->DidReceiveSwapBuffersAck();
+    swap_buffers_callback_.Run();
   }
 
  private:
   cc::OutputSurfaceClient* client_ = nullptr;
+  base::Closure swap_buffers_callback_;
   std::unique_ptr<cc::OverlayCandidateValidator> overlay_candidate_validator_;
   base::WeakPtrFactory<AndroidOutputSurface> weak_ptr_factory_;
 };
@@ -682,10 +686,12 @@ void CompositorImpl::OnGpuChannelEstablished(
     HandlePendingCompositorFrameSinkRequest();
   }
 
-  auto display_output_surface =
-      base::MakeUnique<AndroidOutputSurface>(context_provider);
-  InitializeDisplay(std::move(display_output_surface), nullptr,
-                    std::move(context_provider));
+  // Unretained is safe this owns cc::Display which owns OutputSurface.
+  InitializeDisplay(
+      base::MakeUnique<AndroidOutputSurface>(
+          context_provider,

[boliu, 2017/03/09 20:24:09]

ahh, for anyone wondering why this is x86 crash only, this is the problem.

std::move(context_provider) and context_provider in the same argument list. c++
does not guarantee argument evaluation order, and looks like on x86, it's
backwards!

std::move happens first, so by the time it gets to here, context_provider is
null already

I'll reland, and then merge after other m58 issues have been settled.

[Peter Wen, 2017/03/13 14:07:55]

Thanks for getting to the root of this bug and the explanation, Bo! It makes
sense now why it's just x86.

+          base::Bind(&CompositorImpl::DidSwapBuffers, base::Unretained(this))),
+      nullptr, std::move(context_provider));
 }
 
 void CompositorImpl::InitializeDisplay(
@@ -731,6 +737,10 @@ void CompositorImpl::InitializeDisplay(
   host_->SetCompositorFrameSink(std::move(compositor_frame_sink));
 }
 
+void CompositorImpl::DidSwapBuffers() {
+  client_->DidSwapBuffers();
+}
+
 cc::UIResourceId CompositorImpl::CreateUIResource(
     cc::UIResourceClient* client) {
   TRACE_EVENT0("compositor", "CompositorImpl::CreateUIResource");