android: Add draw completion for CompositorView

content/browser/renderer_host/compositor_impl_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/compositor_impl_android.h"
 
 #include <android/bitmap.h>
 #include <android/native_window_jni.h>
 #include <stdint.h>
 #include <unordered_set>
@@ skipping 168 matching lines @@
     attributes.red_size = 5;
     attributes.green_size = 6;
     attributes.blue_size = 5;
   }
 
   return attributes;
 }
 
 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()),
         weak_ptr_factory_(this) {
     capabilities_.max_frames_pending = kMaxDisplaySwapBuffers;
   }
 
   ~AndroidOutputSurface() override = default;
 
   void SwapBuffers(cc::OutputSurfaceFrame frame) override {
@@ skipping 59 matching lines @@
     DCHECK(command_buffer_proxy);
     return command_buffer_proxy;
   }
 
   void OnSwapBuffersCompleted(
       const std::vector<ui::LatencyInfo>& latency_info,
       gfx::SwapResult result,
       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_;
 };
 
 #if defined(ENABLE_VULKAN)
 class VulkanOutputSurface : public cc::OutputSurface {
  public:
   explicit VulkanOutputSurface(
       scoped_refptr<cc::VulkanContextProvider> vulkan_context_provider,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner)
@@ skipping 390 matching lines @@
           GetCompositorContextAttributes(has_transparent_background_),
           shared_context,
           ui::command_buffer_metrics::DISPLAY_COMPOSITOR_ONSCREEN_CONTEXT);
   if (!context_provider->BindToCurrentThread()) {
     LOG(ERROR) << "Failed to init ContextProvider for compositor.";
     LOG_IF(FATAL, ++num_successive_context_creation_failures_ >= 2)
         << "Too many context creation failures. Giving up... ";
     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(
     std::unique_ptr<cc::OutputSurface> display_output_surface,
     scoped_refptr<cc::VulkanContextProvider> vulkan_context_provider,
     scoped_refptr<cc::ContextProvider> context_provider) {
   DCHECK(compositor_frame_sink_request_pending_);
 
   pending_frames_ = 0;
   num_successive_context_creation_failures_ = 0;
@@ skipping 25 matching lines @@
           : base::MakeUnique<cc::DirectCompositorFrameSink>(
                 frame_sink_id_, manager, display_.get(), context_provider,
                 nullptr, BrowserGpuMemoryBufferManager::current(),
                 HostSharedBitmapManager::current());
 
   display_->SetVisible(true);
   display_->Resize(size_);
   host_->SetCompositorFrameSink(std::move(compositor_frame_sink));
 }
 
+void CompositorImpl::DidSwapBuffers() {
+  client_->DidSwapBuffers();
+}
+
 cc::UIResourceId CompositorImpl::CreateUIResource(
     cc::UIResourceClient* client) {
   TRACE_EVENT0("compositor", "CompositorImpl::CreateUIResource");
   return host_->GetUIResourceManager()->CreateUIResource(client);
 }
 
 void CompositorImpl::DeleteUIResource(cc::UIResourceId resource_id) {
   TRACE_EVENT0("compositor", "CompositorImpl::DeleteUIResource");
   host_->GetUIResourceManager()->DeleteUIResource(resource_id);
 }
@@ skipping 64 matching lines @@
   }
   GetSurfaceManager()->UnregisterFrameSinkHierarchy(frame_sink_id_,
                                                     frame_sink_id);
 }
 
 bool CompositorImpl::HavePendingReadbacks() {
   return !readback_layer_tree_->children().empty();
 }
 
 }  // namespace content