cc: Make OutputSurface::BindToClient pure virtual and not return bool

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>
 #include <utility>
 
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
-#include "base/cancelable_callback.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/sys_info.h"
 #include "base/threading/simple_thread.h"
@@ skipping 178 matching lines @@
   last_begin_frame_args_ =
       cc::BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, frame_time, deadline,
                                  vsync_period, cc::BeginFrameArgs::NORMAL);
   std::unordered_set<cc::BeginFrameObserver*> observers(observers_);
   for (auto* obs : observers)
     obs->OnBeginFrame(last_begin_frame_args_);
 }
 
 class AndroidOutputSurface : public cc::OutputSurface {
  public:
-  AndroidOutputSurface(
-      scoped_refptr<ContextProviderCommandBuffer> context_provider,
-      const base::Callback<void(gpu::Capabilities)>&
-          populate_gpu_capabilities_callback)
+  explicit AndroidOutputSurface(
+      scoped_refptr<ContextProviderCommandBuffer> context_provider)
       : cc::OutputSurface(std::move(context_provider)),
-        populate_gpu_capabilities_callback_(populate_gpu_capabilities_callback),
-        swap_buffers_completion_callback_(
-            base::Bind(&AndroidOutputSurface::OnSwapBuffersCompleted,
-                       base::Unretained(this))),
         overlay_candidate_validator_(
             new display_compositor::
-                CompositorOverlayCandidateValidatorAndroid()) {
+                CompositorOverlayCandidateValidatorAndroid()),
+        weak_ptr_factory_(this) {
     capabilities_.max_frames_pending = kMaxDisplaySwapBuffers;
   }
 
   ~AndroidOutputSurface() override = default;
 
   void SwapBuffers(cc::OutputSurfaceFrame frame) override {
     GetCommandBufferProxy()->SetLatencyInfo(frame.latency_info);
     if (frame.sub_buffer_rect.IsEmpty()) {
       context_provider_->ContextSupport()->CommitOverlayPlanes();
     } else {
       DCHECK(frame.sub_buffer_rect == gfx::Rect(frame.size));
       context_provider_->ContextSupport()->Swap();
     }
   }
 
-  bool BindToClient(cc::OutputSurfaceClient* client) override {
-    if (!OutputSurface::BindToClient(client))
-      return false;
-
+  void BindToClient(cc::OutputSurfaceClient* client) override {
+    DCHECK(client);
+    DCHECK(!client_);
+    client_ = client;
     GetCommandBufferProxy()->SetSwapBuffersCompletionCallback(
-        swap_buffers_completion_callback_.callback());
-
-    populate_gpu_capabilities_callback_.Run(
-        context_provider_->ContextCapabilities());
-    return true;
+        base::Bind(&AndroidOutputSurface::OnSwapBuffersCompleted,
+                   weak_ptr_factory_.GetWeakPtr()));
   }
 
   void EnsureBackbuffer() override {}
 
   void DiscardBackbuffer() override {
     context_provider()->ContextGL()->DiscardBackbufferCHROMIUM();
   }
 
   void BindFramebuffer() override {
     context_provider()->ContextGL()->BindFramebuffer(GL_FRAMEBUFFER, 0);
@@ skipping 35 matching lines @@
 
   void OnSwapBuffersCompleted(
       const std::vector<ui::LatencyInfo>& latency_info,
       gfx::SwapResult result,
       const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac) {
     RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
     client_->DidReceiveSwapBuffersAck();
   }
 
  private:
-  base::Callback<void(gpu::Capabilities)> populate_gpu_capabilities_callback_;
-  base::CancelableCallback<void(
-      const std::vector<ui::LatencyInfo>&,
-      gfx::SwapResult,
-      const gpu::GpuProcessHostedCALayerTreeParamsMac* params_mac)>
-      swap_buffers_completion_callback_;
+  cc::OutputSurfaceClient* client_ = nullptr;
   std::unique_ptr<cc::OverlayCandidateValidator> overlay_candidate_validator_;
+  base::WeakPtrFactory<AndroidOutputSurface> weak_ptr_factory_;

[piman, 2016/10/25 00:47:44]

I /think/ it's not necessary because AndroidOutputSurface is the sole owner of
the ContextProvider/CommandBufferProxyImpl, but I guess it doesn't really hurt?

[danakj, 2016/10/25 00:50:19]

Ya I agree, but refptrs scare me.

 };
 
 #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)
       : OutputSurface(std::move(vulkan_context_provider)),
         task_runner_(std::move(task_runner)),
@@ skipping 367 matching lines @@
               ->CreateDisplayContextProvider(
                   surface_handle_, GetCompositorContextSharedMemoryLimits(),
                   GetCompositorContextAttributes(has_transparent_background_),
                   false /*support_locking*/, false /*automatic_flushes*/,
                   std::move(gpu_channel_host));
 
       scoped_refptr<ContextProviderCommandBuffer>
           context_provider_command_buffer =
               static_cast<ContextProviderCommandBuffer*>(
                   context_provider.get());
+      context_provider_command_buffer->BindToCurrentThread();
+      gpu_capabilities_ =
+          context_provider_command_buffer->ContextCapabilities();
+
       auto display_output_surface = base::MakeUnique<AndroidOutputSurface>(
-          context_provider_command_buffer,
-          base::Bind(&CompositorImpl::PopulateGpuCapabilities,
-                     base::Unretained(this)));
+          std::move(context_provider_command_buffer));
       InitializeDisplay(std::move(display_output_surface), nullptr,
                         std::move(context_provider));
       break;
   }
 }
 
 void CompositorImpl::InitializeDisplay(
     std::unique_ptr<cc::OutputSurface> display_output_surface,
     scoped_refptr<cc::VulkanContextProvider> vulkan_context_provider,
     scoped_refptr<cc::ContextProvider> context_provider) {
@@ skipping 23 matching lines @@
                                     vulkan_context_provider)
                               : base::MakeUnique<cc::DirectCompositorFrameSink>(
                                     frame_sink_id_, manager, display_.get(),
                                     context_provider, nullptr);
 
   display_->SetVisible(true);
   display_->Resize(size_);
   host_->SetCompositorFrameSink(std::move(compositor_frame_sink));
 }
 
-void CompositorImpl::PopulateGpuCapabilities(
-    gpu::Capabilities gpu_capabilities) {
-  gpu_capabilities_ = gpu_capabilities;
-}
-
 void CompositorImpl::AddObserver(VSyncObserver* observer) {
   observer_list_.AddObserver(observer);
 }
 
 void CompositorImpl::RemoveObserver(VSyncObserver* observer) {
   observer_list_.RemoveObserver(observer);
 }
 
 cc::UIResourceId CompositorImpl::CreateUIResource(
     cc::UIResourceClient* client) {
@@ skipping 68 matching lines @@
 
 cc::FrameSinkId CompositorImpl::GetFrameSinkId() {
   return frame_sink_id_;
 }
 
 bool CompositorImpl::HavePendingReadbacks() {
   return !readback_layer_tree_->children().empty();
 }
 
 }  // namespace content