Remove GpuHostMsg_FrameDrawn and replace with client channel swap ack

This also makes it unnecessary to signal a sync point
callback for swap completion for browser
compositor surfaces which is skipped now.

BUG=417945

Review URL: https://codereview.chromium.org/628703005

Cr-Commit-Position: refs/heads/master@{#298972}

cc/output/output_surface.cc

@@ -223,6 +223,8 @@ void OutputSurface::BindFramebuffer() {
 }
 
 void OutputSurface::SwapBuffers(CompositorFrame* frame) {
+  // TODO(sievers): Make OutputSurface::SwapBuffers() pure virtual.
+  // The implementation here is used for tests only.
   if (frame->software_frame_data) {
     PostSwapBuffersComplete();
     client_->DidSwapBuffers();

content/browser/compositor/gpu_browser_compositor_output_surface.cc

@@ -5,8 +5,12 @@
 #include "content/browser/compositor/gpu_browser_compositor_output_surface.h"
 
 #include "cc/output/compositor_frame.h"
+#include "cc/output/output_surface_client.h"
 #include "content/browser/compositor/reflector_impl.h"
+#include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/common/gpu/client/context_provider_command_buffer.h"
+#include "content/public/browser/browser_thread.h"
+#include "gpu/command_buffer/client/context_support.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 
 namespace content {
@@ -20,22 +24,41 @@ GpuBrowserCompositorOutputSurface::GpuBrowserCompositorOutputSurface(
     : BrowserCompositorOutputSurface(context,
                                      surface_id,
                                      output_surface_map,
-                                     vsync_manager) {
+                                     vsync_manager),
+      swap_buffers_completion_callback_(
+          base::Bind(&GpuBrowserCompositorOutputSurface::OnSwapBuffersCompleted,
+                     base::Unretained(this))) {
   overlay_candidate_validator_ = overlay_candidate_validator.Pass();
 }
 
 GpuBrowserCompositorOutputSurface::~GpuBrowserCompositorOutputSurface() {}
 
-void GpuBrowserCompositorOutputSurface::SwapBuffers(
-    cc::CompositorFrame* frame) {
-  DCHECK(frame->gl_frame_data);
-
+CommandBufferProxyImpl*
+GpuBrowserCompositorOutputSurface::GetCommandBufferProxy() {
   ContextProviderCommandBuffer* provider_command_buffer =
-      static_cast<ContextProviderCommandBuffer*>(context_provider_.get());
+      static_cast<content::ContextProviderCommandBuffer*>(
+          context_provider_.get());
   CommandBufferProxyImpl* command_buffer_proxy =
       provider_command_buffer->GetCommandBufferProxy();
   DCHECK(command_buffer_proxy);
-  command_buffer_proxy->SetLatencyInfo(frame->metadata.latency_info);
+  return command_buffer_proxy;
+}
+
+bool GpuBrowserCompositorOutputSurface::BindToClient(
+    cc::OutputSurfaceClient* client) {
+  if (!BrowserCompositorOutputSurface::BindToClient(client))
+    return false;
+
+  GetCommandBufferProxy()->SetSwapBuffersCompletionCallback(
+      swap_buffers_completion_callback_.callback());
+  return true;
+}
+
+void GpuBrowserCompositorOutputSurface::SwapBuffers(
+    cc::CompositorFrame* frame) {
+  DCHECK(frame->gl_frame_data);
+
+  GetCommandBufferProxy()->SetLatencyInfo(frame->metadata.latency_info);
 
   if (reflector_.get()) {
     if (frame->gl_frame_data->sub_buffer_rect ==
@@ -45,13 +68,32 @@ void GpuBrowserCompositorOutputSurface::SwapBuffers(
       reflector_->OnPostSubBuffer(frame->gl_frame_data->sub_buffer_rect);
   }
 
-  OutputSurface::SwapBuffers(frame);
+  if (frame->gl_frame_data->sub_buffer_rect ==
+      gfx::Rect(frame->gl_frame_data->size)) {
+    context_provider_->ContextSupport()->Swap();
+  } else {
+    context_provider_->ContextSupport()->PartialSwapBuffers(
+        frame->gl_frame_data->sub_buffer_rect);
+  }
+
+  client_->DidSwapBuffers();
 }
 
-void GpuBrowserCompositorOutputSurface::OnSwapBuffersComplete() {
+void GpuBrowserCompositorOutputSurface::OnSwapBuffersCompleted(
+    const std::vector<ui::LatencyInfo>& latency_info) {
+#if defined(OS_MACOSX)
   // On Mac, delay acknowledging the swap to the output surface client until
-  // it has been drawn.
-#if !defined(OS_MACOSX)
+  // it has been drawn, see OnSurfaceDisplayed();
+  NOTREACHED();
+#else
+  if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
+  } else {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&RenderWidgetHostImpl::CompositorFrameDrawn, latency_info));
+  }
   cc::OutputSurface::OnSwapBuffersComplete();
 #endif
 }

content/browser/compositor/gpu_browser_compositor_output_surface.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_BROWSER_COMPOSITOR_GPU_BROWSER_COMPOSITOR_OUTPUT_SURFACE_H_
 #define CONTENT_BROWSER_COMPOSITOR_GPU_BROWSER_COMPOSITOR_OUTPUT_SURFACE_H_
 
+#include "base/cancelable_callback.h"
 #include "content/browser/compositor/browser_compositor_output_surface.h"
 
 namespace ui {
@@ -16,6 +17,7 @@ class OverlayCandidateValidator;
 }
 
 namespace content {
+class CommandBufferProxyImpl;
 
 // Adapts a WebGraphicsContext3DCommandBufferImpl into a
 // cc::OutputSurface that also handles vsync parameter updates
@@ -35,12 +37,18 @@ class GpuBrowserCompositorOutputSurface
  protected:
   // cc::OutputSurface implementation.
   virtual void SwapBuffers(cc::CompositorFrame* frame) override;
-  virtual void OnSwapBuffersComplete() override;
+  virtual bool BindToClient(cc::OutputSurfaceClient* client) override;
 
 #if defined(OS_MACOSX)
   virtual void OnSurfaceDisplayed() override;
 #endif
 
+  CommandBufferProxyImpl* GetCommandBufferProxy();
+  void OnSwapBuffersCompleted(const std::vector<ui::LatencyInfo>& latency_info);
+
+  base::CancelableCallback<void(const std::vector<ui::LatencyInfo>&)>
+      swap_buffers_completion_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(GpuBrowserCompositorOutputSurface);
 };
 

content/browser/gpu/gpu_process_host_ui_shim.cc

@@ -212,7 +212,6 @@ bool GpuProcessHostUIShim::OnControlMessageReceived(
                         OnGraphicsInfoCollected)
     IPC_MESSAGE_HANDLER(GpuHostMsg_VideoMemoryUsageStats,
                         OnVideoMemoryUsageStatsReceived);
-    IPC_MESSAGE_HANDLER(GpuHostMsg_FrameDrawn, OnFrameDrawn)
 
     IPC_MESSAGE_UNHANDLED_ERROR()
   IPC_END_MESSAGE_MAP()
@@ -274,14 +273,6 @@ void GpuProcessHostUIShim::OnAcceleratedSurfaceBuffersSwapped(
 #endif
 }
 
-void GpuProcessHostUIShim::OnFrameDrawn(
-    const std::vector<ui::LatencyInfo>& latency_info) {
-  if (!ui::LatencyInfo::Verify(latency_info,
-                               "GpuProcessHostUIShim::OnFrameDrawn"))
-    return;
-  RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
-}
-
 void GpuProcessHostUIShim::OnVideoMemoryUsageStatsReceived(
     const GPUVideoMemoryUsageStats& video_memory_usage_stats) {
   GpuDataManagerImpl::GetInstance()->UpdateVideoMemoryUsageStats(

content/browser/gpu/gpu_process_host_ui_shim.h

@@ -93,7 +93,6 @@ class GpuProcessHostUIShim : public IPC::Listener,
       const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params);
   void OnVideoMemoryUsageStatsReceived(
       const GPUVideoMemoryUsageStats& video_memory_usage_stats);
-  void OnFrameDrawn(const std::vector<ui::LatencyInfo>& latency_info);
 
   // The serial number of the GpuProcessHost / GpuProcessHostUIShim pair.
   int host_id_;

content/browser/renderer_host/compositor_impl_android.cc

@@ -10,6 +10,7 @@
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.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"
@@ -25,10 +26,12 @@
 #include "cc/output/compositor_frame.h"
 #include "cc/output/context_provider.h"
 #include "cc/output/output_surface.h"
+#include "cc/output/output_surface_client.h"
 #include "cc/trees/layer_tree_host.h"
 #include "content/browser/android/child_process_launcher_android.h"
 #include "content/browser/gpu/browser_gpu_channel_host_factory.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
+#include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/common/gpu/client/command_buffer_proxy_impl.h"
 #include "content/common/gpu/client/context_provider_command_buffer.h"
 #include "content/common/gpu/client/gl_helper.h"
@@ -37,6 +40,7 @@
 #include "content/common/gpu/gpu_process_launch_causes.h"
 #include "content/common/host_shared_bitmap_manager.h"
 #include "content/public/browser/android/compositor_client.h"
+#include "gpu/command_buffer/client/context_support.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
@@ -49,6 +53,8 @@
 #include "webkit/common/gpu/context_provider_in_process.h"
 #include "webkit/common/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h"
 
+namespace content {
+
 namespace {
 
 const unsigned int kMaxSwapBuffers = 2U;
@@ -56,10 +62,13 @@ const unsigned int kMaxSwapBuffers = 2U;
 // Used to override capabilities_.adjust_deadline_for_parent to false
 class OutputSurfaceWithoutParent : public cc::OutputSurface {
  public:
-  OutputSurfaceWithoutParent(const scoped_refptr<
-      content::ContextProviderCommandBuffer>& context_provider,
-      base::WeakPtr<content::CompositorImpl> compositor_impl)
-      : cc::OutputSurface(context_provider) {
+  OutputSurfaceWithoutParent(
+      const scoped_refptr<ContextProviderCommandBuffer>& context_provider,
+      base::WeakPtr<CompositorImpl> compositor_impl)
+      : cc::OutputSurface(context_provider),
+        swap_buffers_completion_callback_(
+            base::Bind(&OutputSurfaceWithoutParent::OnSwapBuffersCompleted,
+                       base::Unretained(this))) {
     capabilities_.adjust_deadline_for_parent = false;
     compositor_impl_ = compositor_impl;
     main_thread_ = base::MessageLoopProxy::current();
@@ -71,32 +80,51 @@ class OutputSurfaceWithoutParent : public cc::OutputSurface {
           ui::INPUT_EVENT_BROWSER_SWAP_BUFFER_COMPONENT, 0, 0);
     }
 
-    content::ContextProviderCommandBuffer* provider_command_buffer =
-        static_cast<content::ContextProviderCommandBuffer*>(
-            context_provider_.get());
-    content::CommandBufferProxyImpl* command_buffer_proxy =
-        provider_command_buffer->GetCommandBufferProxy();
-    DCHECK(command_buffer_proxy);
-    command_buffer_proxy->SetLatencyInfo(frame->metadata.latency_info);
-
-    OutputSurface::SwapBuffers(frame);
+    GetCommandBufferProxy()->SetLatencyInfo(frame->metadata.latency_info);
+    DCHECK(frame->gl_frame_data->sub_buffer_rect ==
+           gfx::Rect(frame->gl_frame_data->size));
+    context_provider_->ContextSupport()->Swap();
+    client_->DidSwapBuffers();
   }
 
   virtual bool BindToClient(cc::OutputSurfaceClient* client) override {
     if (!OutputSurface::BindToClient(client))
       return false;
 
+    GetCommandBufferProxy()->SetSwapBuffersCompletionCallback(
+        swap_buffers_completion_callback_.callback());
+
     main_thread_->PostTask(
         FROM_HERE,
-        base::Bind(&content::CompositorImpl::PopulateGpuCapabilities,
+        base::Bind(&CompositorImpl::PopulateGpuCapabilities,
                    compositor_impl_,
                    context_provider_->ContextCapabilities().gpu));
 
     return true;
   }
 
+ private:
+  CommandBufferProxyImpl* GetCommandBufferProxy() {
+    ContextProviderCommandBuffer* provider_command_buffer =
+        static_cast<content::ContextProviderCommandBuffer*>(
+            context_provider_.get());
+    CommandBufferProxyImpl* command_buffer_proxy =
+        provider_command_buffer->GetCommandBufferProxy();
+    DCHECK(command_buffer_proxy);
+    return command_buffer_proxy;
+  }
+
+  void OnSwapBuffersCompleted(
+      const std::vector<ui::LatencyInfo>& latency_info) {
+    RenderWidgetHostImpl::CompositorFrameDrawn(latency_info);
+    OutputSurface::OnSwapBuffersComplete();
+  }
+
+  base::CancelableCallback<void(const std::vector<ui::LatencyInfo>&)>
+      swap_buffers_completion_callback_;
+
   scoped_refptr<base::MessageLoopProxy> main_thread_;
-  base::WeakPtr<content::CompositorImpl> compositor_impl_;
+  base::WeakPtr<CompositorImpl> compositor_impl_;
 };
 
 class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
@@ -130,7 +158,7 @@ class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
     SurfaceTextureMapKey key(surface_texture_id, child_process_id);
     DCHECK(surface_textures_.find(key) == surface_textures_.end());
     surface_textures_[key] = surface_texture;
-    content::RegisterChildProcessSurfaceTexture(
+    RegisterChildProcessSurfaceTexture(
         surface_texture_id,
         child_process_id,
         surface_texture->j_surface_texture().obj());
@@ -143,8 +171,7 @@ class SurfaceTextureTrackerImpl : public gfx::SurfaceTextureTracker {
     SurfaceTextureMap::iterator it = surface_textures_.begin();
     while (it != surface_textures_.end()) {
       if (it->first.second == child_process_id) {
-        content::UnregisterChildProcessSurfaceTexture(it->first.first,
-                                                      it->first.second);
+        UnregisterChildProcessSurfaceTexture(it->first.first, it->first.second);
         surface_textures_.erase(it++);
       } else {
         ++it;
@@ -169,8 +196,6 @@ static bool g_initialized = false;
 
 } // anonymous namespace
 
-namespace content {
-
 // static
 Compositor* Compositor::Create(CompositorClient* client,
                                gfx::NativeWindow root_window) {
@@ -402,7 +427,7 @@ void CompositorImpl::SetSurface(jobject surface) {
 
   // First, cleanup any existing surface references.
   if (surface_id_)
-    content::UnregisterViewSurface(surface_id_);
+    UnregisterViewSurface(surface_id_);
   SetWindowSurface(NULL);
 
   // Now, set the new surface if we have one.
@@ -417,7 +442,7 @@ void CompositorImpl::SetSurface(jobject surface) {
   if (window) {
     SetWindowSurface(window);
     ANativeWindow_release(window);
-    content::RegisterViewSurface(surface_id_, j_surface.obj());
+    RegisterViewSurface(surface_id_, j_surface.obj());
   }
 }
 

content/common/gpu/client/command_buffer_proxy_impl.cc

@@ -47,6 +47,8 @@ bool CommandBufferProxyImpl::OnMessageReceived(const IPC::Message& message) {
                         OnSetMemoryAllocation);
     IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SignalSyncPointAck,
                         OnSignalSyncPointAck);
+    IPC_MESSAGE_HANDLER(GpuCommandBufferMsg_SwapBuffersCompleted,
+                        OnSwapBuffersCompleted);
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -192,6 +194,11 @@ void CommandBufferProxyImpl::SetLatencyInfo(
     latency_info_.push_back(latency_info[i]);
 }
 
+void CommandBufferProxyImpl::SetSwapBuffersCompletionCallback(
+    const SwapBuffersCompletionCallback& callback) {
+  swap_buffers_completion_callback_ = callback;
+}
+
 void CommandBufferProxyImpl::WaitForTokenInRange(int32 start, int32 end) {
   TRACE_EVENT2("gpu",
                "CommandBufferProxyImpl::WaitForToken",
@@ -504,4 +511,16 @@ gpu::CommandBufferSharedState* CommandBufferProxyImpl::shared_state() const {
       shared_state_shm_->memory());
 }
 
+void CommandBufferProxyImpl::OnSwapBuffersCompleted(
+    const std::vector<ui::LatencyInfo>& latency_info) {
+  if (!swap_buffers_completion_callback_.is_null()) {
+    if (!ui::LatencyInfo::Verify(
+            latency_info, "CommandBufferProxyImpl::OnSwapBuffersCompleted")) {
+      swap_buffers_completion_callback_.Run(std::vector<ui::LatencyInfo>());
+      return;
+    }
+    swap_buffers_completion_callback_.Run(latency_info);
+  }
+}
+
 }  // namespace content

content/common/gpu/client/command_buffer_proxy_impl.h

@@ -131,6 +131,10 @@ class CommandBufferProxyImpl
       const GpuConsoleMessageCallback& callback);
 
   void SetLatencyInfo(const std::vector<ui::LatencyInfo>& latency_info);
+  typedef base::Callback<void(const std::vector<ui::LatencyInfo>& latency_info)>
+      SwapBuffersCompletionCallback;
+  void SetSwapBuffersCompletionCallback(
+      const SwapBuffersCompletionCallback& callback);
 
   // TODO(apatrick): this is a temporary optimization while skia is calling
   // ContentGLContext::MakeCurrent prior to every GL call. It saves returning 6
@@ -161,6 +165,7 @@ class CommandBufferProxyImpl
   void OnConsoleMessage(const GPUCommandBufferConsoleMessage& message);
   void OnSetMemoryAllocation(const gpu::MemoryAllocation& allocation);
   void OnSignalSyncPointAck(uint32 id);
+  void OnSwapBuffersCompleted(const std::vector<ui::LatencyInfo>& latency_info);
 
   // Try to read an updated copy of the state from shared memory.
   void TryUpdateState();
@@ -201,6 +206,8 @@ class CommandBufferProxyImpl
 
   std::vector<ui::LatencyInfo> latency_info_;
 
+  SwapBuffersCompletionCallback swap_buffers_completion_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(CommandBufferProxyImpl);
 };
 

content/common/gpu/gpu_command_buffer_stub.cc

@@ -1096,4 +1096,9 @@ uint64 GpuCommandBufferStub::GetMemoryUsage() const {
   return GetMemoryManager()->GetClientMemoryUsage(this);
 }
 
+void GpuCommandBufferStub::SwapBuffersCompleted(
+    const std::vector<ui::LatencyInfo>& latency_info) {
+  Send(new GpuCommandBufferMsg_SwapBuffersCompleted(route_id_, latency_info));
+}
+
 }  // namespace content

content/common/gpu/gpu_command_buffer_stub.h

@@ -144,6 +144,8 @@ class GpuCommandBufferStub
 
   uint64 GetMemoryUsage() const;
 
+  void SwapBuffersCompleted(const std::vector<ui::LatencyInfo>& latency_info);
+
  private:
   GpuMemoryManager* GetMemoryManager() const;
   bool MakeCurrent();

content/common/gpu/gpu_messages.h

@@ -378,11 +378,6 @@ IPC_MESSAGE_CONTROL2(GpuHostMsg_AcceleratedSurfaceInitialized,
                      int32 /* surface_id */,
                      int32 /* route_id */)
 
-// Tells the browser that a frame with the specific latency info was drawn to
-// the screen
-IPC_MESSAGE_CONTROL1(GpuHostMsg_FrameDrawn,
-                     std::vector<ui::LatencyInfo> /* latency_info */)
-
 // Same as above with a rect of the part of the surface that changed.
 IPC_MESSAGE_CONTROL1(GpuHostMsg_AcceleratedSurfaceBuffersSwapped,
                      GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params)
@@ -548,6 +543,10 @@ IPC_SYNC_MESSAGE_ROUTED5_1(GpuCommandBufferMsg_CreateVideoEncoder,
 IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_Destroyed,
                     gpu::error::ContextLostReason /* reason */)
 
+// Tells the browser that SwapBuffers returned and passes latency info
+IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_SwapBuffersCompleted,
+                    std::vector<ui::LatencyInfo> /* latency_info */)
+
 // Send to stub on surface visibility change.
 IPC_MESSAGE_ROUTED1(GpuCommandBufferMsg_SetSurfaceVisible, bool /* visible */)
 

content/common/gpu/image_transport_surface.cc

@@ -120,9 +120,9 @@ void ImageTransportHelper::SendUpdateVSyncParameters(
                                                       interval));
 }
 
-void ImageTransportHelper::SendLatencyInfo(
+void ImageTransportHelper::SwapBuffersCompleted(
     const std::vector<ui::LatencyInfo>& latency_info) {
-  manager_->Send(new GpuHostMsg_FrameDrawn(latency_info));
+  stub_->SwapBuffersCompleted(latency_info);
 }
 
 void ImageTransportHelper::SetScheduled(bool is_scheduled) {
@@ -245,7 +245,7 @@ bool PassThroughImageTransportSurface::SwapBuffers() {
         ui::INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT, 0, 0);
   }
 
-  helper_->SendLatencyInfo(latency_info_);
+  helper_->SwapBuffersCompleted(latency_info_);
   latency_info_.clear();
   return result;
 }
@@ -259,7 +259,7 @@ bool PassThroughImageTransportSurface::PostSubBuffer(
         ui::INPUT_EVENT_LATENCY_TERMINATED_FRAME_SWAP_COMPONENT, 0, 0);
   }
 
-  helper_->SendLatencyInfo(latency_info_);
+  helper_->SwapBuffersCompleted(latency_info_);
   latency_info_.clear();
   return result;
 }

content/common/gpu/image_transport_surface.h

@@ -127,7 +127,7 @@ class ImageTransportHelper
   void SendUpdateVSyncParameters(
       base::TimeTicks timebase, base::TimeDelta interval);
 
-  void SendLatencyInfo(const std::vector<ui::LatencyInfo>& latency_info);
+  void SwapBuffersCompleted(const std::vector<ui::LatencyInfo>& latency_info);
 
   // Whether or not we should execute more commands.
   void SetScheduled(bool is_scheduled);