Plumb gfx::SwapTimings out to Display

Note: based on crrev.com/c/1648953

In order to get swap timing in the renderer process, we'll need to send
down additional information via BeginFrame. crrev.com/c/1639559 adds a
new FrameTimingDetails struct that will be sent down, instead of just
the PresentationFeedback. We'll be adding to that struct in a future CL
that plumbs out swap information.

This struct add the first bit of plumbing from OutputSurface to Display.
OutputSurfaceClient::DidReceiveSwapBuffersAck now takes a SwapTimings
parameter, which will then be plubmed out to Surface then SurfaceClient
(CompositorFrameSinkSupport) once dependent changes all land.

Bug: 971712
Change-Id: Ibdd49356ce94261aec4a695b22f96dec1ed5e0ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1650224
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670223}

android_webview/browser/gfx/surfaces_instance.cc

@@ -381,7 +381,9 @@ void SurfacesInstance::DrawAndSwap(const gfx::Size& viewport,
   }
   display_->Resize(viewport);
   display_->DrawAndSwap();
-  display_->DidReceiveSwapBuffersAck();
+  // TODO(dlibby): Consider sending real swap timings here for webview (or
+  // prove why it is not needed).
+  display_->DidReceiveSwapBuffersAck(gfx::SwapTimings());
   gl_surface_->MaybeDidPresent(gfx::PresentationFeedback(
       base::TimeTicks::Now(), base::TimeDelta(), 0 /* flags */));
 }

cc/test/fake_output_surface_client.cc

@@ -6,7 +6,8 @@
 
 namespace cc {
 
-void FakeOutputSurfaceClient::DidReceiveSwapBuffersAck() {
+void FakeOutputSurfaceClient::DidReceiveSwapBuffersAck(
+    const gfx::SwapTimings&) {
   swap_count_++;
 }
 

cc/test/fake_output_surface_client.h

@@ -14,7 +14,7 @@ class FakeOutputSurfaceClient : public viz::OutputSurfaceClient {
   FakeOutputSurfaceClient() = default;
 
   void SetNeedsRedrawRect(const gfx::Rect& damage_rect) override {}
-  void DidReceiveSwapBuffersAck() override;
+  void DidReceiveSwapBuffersAck(const gfx::SwapTimings& timings) override;
   void DidReceiveTextureInUseResponses(
       const gpu::TextureInUseResponses& responses) override {}
   void DidReceiveCALayerParams(

cc/test/pixel_test_output_surface.cc

@@ -77,7 +77,7 @@ void PixelTestOutputSurface::SwapBuffers(viz::OutputSurfaceFrame frame) {
 }
 
 void PixelTestOutputSurface::SwapBuffersCallback() {
-  client_->DidReceiveSwapBuffersAck();
+  client_->DidReceiveSwapBuffersAck(gfx::SwapTimings());
   client_->DidReceivePresentationFeedback(
       gfx::PresentationFeedback(base::TimeTicks::Now(), base::TimeDelta(), 0));
 }

components/viz/service/display/display.cc

@@ -39,6 +39,7 @@
 #include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/overlay_transform_utils.h"
 #include "ui/gfx/presentation_feedback.h"
+#include "ui/gfx/swap_result.h"
 
 namespace viz {
 
@@ -599,7 +600,7 @@ bool Display::DrawAndSwap() {
   return true;
 }
 
-void Display::DidReceiveSwapBuffersAck() {
+void Display::DidReceiveSwapBuffersAck(const gfx::SwapTimings& timings) {
   if (scheduler_) {
     scheduler_->DidReceiveSwapBuffersAck();
     if (no_pending_swaps_callback_ && scheduler_->pending_swaps() == 0)
@@ -608,6 +609,19 @@ void Display::DidReceiveSwapBuffersAck() {
 
   if (renderer_)
     renderer_->SwapBuffersComplete();
+
+  // Adding to pending_presented_callbacks_ must have been done in DrawAndSwap,
+  // and should not be popped until DidReceivePresentationFeedback. Therefore
+  // we must not have an empty list when getting the SwapBuffers ACK (this is
+  // required to happen between those two events).
+  DCHECK(!pending_presented_callbacks_.empty());
+
+  // Check that the swap timings correspond with the timestamp from when
+  // the swap was triggered. Note that not all output surfaces provide timing
+  // information, hence the check for a valid swap_start.
+  const auto swap_time = pending_presented_callbacks_.front().first;
+  if (!timings.swap_start.is_null())
+    DCHECK_LE(swap_time, timings.swap_start);
 }
 
 void Display::DidReceiveTextureInUseResponses(

components/viz/service/display/display.h

@@ -126,7 +126,7 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
 
   // OutputSurfaceClient implementation.
   void SetNeedsRedrawRect(const gfx::Rect& damage_rect) override;
-  void DidReceiveSwapBuffersAck() override;
+  void DidReceiveSwapBuffersAck(const gfx::SwapTimings& timings) override;
   void DidReceiveTextureInUseResponses(
       const gpu::TextureInUseResponses& responses) override;
   void DidReceiveCALayerParams(

components/viz/service/display/output_surface_client.h

@@ -18,6 +18,7 @@
 namespace gfx {
 struct CALayerParams;
 struct PresentationFeedback;
+struct SwapTimings;
 }  // namespace gfx
 
 namespace viz {
@@ -25,8 +26,9 @@ namespace viz {
 class VIZ_SERVICE_EXPORT OutputSurfaceClient {
  public:
   // A notification that the swap of the backbuffer to the hardware is complete
-  // and is now visible to the user.
-  virtual void DidReceiveSwapBuffersAck() = 0;
+  // and is now visible to the user, along with timing information on when the
+  // swapping of the backbuffer started and completed.
+  virtual void DidReceiveSwapBuffersAck(const gfx::SwapTimings& timings) = 0;
 
   // For surfaceless/ozone implementations to create damage for the next frame.
   virtual void SetNeedsRedrawRect(const gfx::Rect& damage_rect) = 0;

components/viz/service/display_embedder/gl_output_surface.cc

@@ -154,8 +154,9 @@ bool GLOutputSurface::HasExternalStencilTest() const {
 
 void GLOutputSurface::ApplyExternalStencil() {}
 
-void GLOutputSurface::DidReceiveSwapBuffersAck(gfx::SwapResult result) {
-  client_->DidReceiveSwapBuffersAck();
+void GLOutputSurface::DidReceiveSwapBuffersAck(
+    const gfx::SwapResponse& response) {
+  client_->DidReceiveSwapBuffersAck(response.timings);
 }
 
 void GLOutputSurface::HandlePartialSwap(
@@ -176,7 +177,7 @@ void GLOutputSurface::OnGpuSwapBuffersCompleted(
     client_->DidReceiveTextureInUseResponses(params.texture_in_use_responses);
   if (!params.ca_layer_params.is_empty)
     client_->DidReceiveCALayerParams(params.ca_layer_params);
-  DidReceiveSwapBuffersAck(params.swap_response.result);
+  DidReceiveSwapBuffersAck(params.swap_response);
 
   UpdateLatencyInfoOnSwap(params.swap_response, &latency_info);
   latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);

components/viz/service/display_embedder/gl_output_surface.h

@@ -59,7 +59,7 @@ class GLOutputSurface : public OutputSurface {
   ui::LatencyTracker* latency_tracker() { return &latency_tracker_; }
 
   // Called when a swap completion is signaled from ImageTransportSurface.
-  virtual void DidReceiveSwapBuffersAck(gfx::SwapResult result);
+  virtual void DidReceiveSwapBuffersAck(const gfx::SwapResponse& response);
 
   // Called in SwapBuffers() when a swap is determined to be partial. Subclasses
   // might override this method because different platforms handle partial swaps

components/viz/service/display_embedder/gl_output_surface_buffer_queue.cc

@@ -102,18 +102,17 @@ gfx::BufferFormat GLOutputSurfaceBufferQueue::GetOverlayBufferFormat() const {
 }
 
 void GLOutputSurfaceBufferQueue::DidReceiveSwapBuffersAck(
-    gfx::SwapResult result) {
+    const gfx::SwapResponse& response) {
   bool force_swap = false;
-  if (result == gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
+  if (response.result == gfx::SwapResult::SWAP_NAK_RECREATE_BUFFERS) {
     // Even through the swap failed, this is a fixable error so we can pretend
     // it succeeded to the rest of the system.
-    result = gfx::SwapResult::SWAP_ACK;
     buffer_queue_->RecreateBuffers();
     force_swap = true;
   }
 
   buffer_queue_->PageFlipComplete();
-  client()->DidReceiveSwapBuffersAck();
+  client()->DidReceiveSwapBuffersAck(response.timings);
 
   if (force_swap)
     client()->SetNeedsRedrawRect(gfx::Rect(swap_size_));

components/viz/service/display_embedder/gl_output_surface_buffer_queue.h

@@ -62,7 +62,7 @@ class GLOutputSurfaceBufferQueue : public GLOutputSurface {
   void SetDrawRectangle(const gfx::Rect& damage) override;
 
   // GLOutputSurface:
-  void DidReceiveSwapBuffersAck(gfx::SwapResult result) override;
+  void DidReceiveSwapBuffersAck(const gfx::SwapResponse& response) override;
 
   std::unique_ptr<BufferQueue> buffer_queue_;
 

components/viz/service/display_embedder/gl_output_surface_offscreen.cc

@@ -117,7 +117,9 @@ void GLOutputSurfaceOffscreen::SwapBuffers(OutputSurfaceFrame frame) {
 void GLOutputSurfaceOffscreen::OnSwapBuffersComplete(
     std::vector<ui::LatencyInfo> latency_info) {
   latency_tracker()->OnGpuSwapBuffersCompleted(latency_info);
-  client()->DidReceiveSwapBuffersAck();
+  // Swap timings are not available since for offscreen there is no Swap, just
+  // a SignalSyncToken.
+  client()->DidReceiveSwapBuffersAck(gfx::SwapTimings());
   client()->DidReceivePresentationFeedback(gfx::PresentationFeedback());
 }
 

components/viz/service/display_embedder/skia_output_device.cc

@@ -46,6 +46,9 @@ gfx::SwapResponse SkiaOutputDevice::FinishSwapBuffers(gfx::SwapResult result) {
 
   params_->swap_response.result = result;
   params_->swap_response.timings.swap_end = base::TimeTicks::Now();
+  did_swap_buffer_complete_callback_.Run(
+      *params_, gfx::Size(draw_surface_->width(), draw_surface_->height()));
+
   if (feedback_) {
     std::move(*feedback_)
         .Run(gfx::PresentationFeedback(
@@ -55,8 +58,6 @@ gfx::SwapResponse SkiaOutputDevice::FinishSwapBuffers(gfx::SwapResult result) {
                 ? 0
                 : gfx::PresentationFeedback::Flags::kFailure));
   }
-  did_swap_buffer_complete_callback_.Run(
-      *params_, gfx::Size(draw_surface_->width(), draw_surface_->height()));
 
   feedback_.reset();
   auto response = params_->swap_response;

components/viz/service/display_embedder/skia_output_surface_impl.cc

@@ -573,7 +573,7 @@ void SkiaOutputSurfaceImpl::DidSwapBuffersComplete(
     client_->DidReceiveTextureInUseResponses(params.texture_in_use_responses);
   if (!params.ca_layer_params.is_empty)
     client_->DidReceiveCALayerParams(params.ca_layer_params);
-  client_->DidReceiveSwapBuffersAck();
+  client_->DidReceiveSwapBuffersAck(params.swap_response.timings);
   if (needs_swap_size_notifications_)
     client_->DidSwapWithSize(pixel_size);
 }

components/viz/service/display_embedder/skia_output_surface_impl_non_ddl.cc

@@ -252,11 +252,14 @@ void SkiaOutputSurfaceImplNonDDL::ReleaseCachedResources(
 
 void SkiaOutputSurfaceImplNonDDL::SkiaSwapBuffers(OutputSurfaceFrame frame) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  gfx::SwapTimings swap_timings;
+  swap_timings.swap_start = base::TimeTicks::Now();
   gl_surface_->SwapBuffers(
       base::BindOnce(&SkiaOutputSurfaceImplNonDDL::BufferPresented,
                      weak_ptr_factory_.GetWeakPtr()));
+  swap_timings.swap_end = base::TimeTicks::Now();
   if (need_swapbuffers_ack_)
-    client_->DidReceiveSwapBuffersAck();
+    client_->DidReceiveSwapBuffersAck(swap_timings);
 }
 
 SkCanvas* SkiaOutputSurfaceImplNonDDL::BeginPaintRenderPass(

components/viz/service/display_embedder/software_output_surface.cc

@@ -16,6 +16,7 @@
 #include "components/viz/service/display/output_surface_frame.h"
 #include "components/viz/service/display/software_output_device.h"
 #include "ui/gfx/presentation_feedback.h"
+#include "ui/gfx/swap_result.h"
 #include "ui/gfx/vsync_provider.h"
 #include "ui/latency/latency_info.h"
 
@@ -71,8 +72,9 @@ void SoftwareOutputSurface::SwapBuffers(OutputSurfaceFrame frame) {
       << "arrive before the previous latency info is processed.";
   stored_latency_info_ = std::move(frame.latency_info);
 
-  software_device()->OnSwapBuffers(base::BindOnce(
-      &SoftwareOutputSurface::SwapBuffersCallback, weak_factory_.GetWeakPtr()));
+  software_device()->OnSwapBuffers(
+      base::BindOnce(&SoftwareOutputSurface::SwapBuffersCallback,
+                     weak_factory_.GetWeakPtr(), swap_time));
 
   gfx::VSyncProvider* vsync_provider = software_device()->GetVSyncProvider();
   if (vsync_provider && update_vsync_parameters_callback_) {
@@ -106,11 +108,12 @@ uint32_t SoftwareOutputSurface::GetFramebufferCopyTextureFormat() {
   return 0;
 }
 
-void SoftwareOutputSurface::SwapBuffersCallback(const gfx::Size& pixel_size) {
+void SoftwareOutputSurface::SwapBuffersCallback(base::TimeTicks swap_time,
+                                                const gfx::Size& pixel_size) {
   latency_tracker_.OnGpuSwapBuffersCompleted(stored_latency_info_);
   client_->DidFinishLatencyInfo(stored_latency_info_);
   std::vector<ui::LatencyInfo>().swap(stored_latency_info_);
-  client_->DidReceiveSwapBuffersAck();
+  client_->DidReceiveSwapBuffersAck({swap_time, swap_time});
 
   base::TimeTicks now = base::TimeTicks::Now();
   base::TimeDelta interval_to_next_refresh =

components/viz/service/display_embedder/software_output_surface.h

@@ -53,7 +53,8 @@ class VIZ_SERVICE_EXPORT SoftwareOutputSurface : public OutputSurface {
 #endif
 
  private:
-  void SwapBuffersCallback(const gfx::Size& pixel_size);
+  void SwapBuffersCallback(base::TimeTicks swap_time,
+                           const gfx::Size& pixel_size);
   void UpdateVSyncParameters(base::TimeTicks timebase,
                              base::TimeDelta interval);
 

components/viz/test/fake_output_surface.cc

@@ -54,9 +54,9 @@ void FakeOutputSurface::SwapBuffers(OutputSurfaceFrame frame) {
 }
 
 void FakeOutputSurface::SwapBuffersAck() {
-  client_->DidReceiveSwapBuffersAck();
-  client_->DidReceivePresentationFeedback(
-      {base::TimeTicks::Now(), base::TimeDelta(), 0});
+  base::TimeTicks now = base::TimeTicks::Now();
+  client_->DidReceiveSwapBuffersAck({now, now});
+  client_->DidReceivePresentationFeedback({now, base::TimeDelta(), 0});
 }
 
 void FakeOutputSurface::BindFramebuffer() {

components/viz/test/fake_skia_output_surface.cc

@@ -21,6 +21,7 @@
 #include "third_party/skia/include/core/SkPixelRef.h"
 #include "third_party/skia/include/gpu/GrBackendSurface.h"
 #include "third_party/skia/include/gpu/gl/GrGLTypes.h"
+#include "ui/gfx/swap_result.h"
 #include "ui/gl/gl_utils.h"
 
 namespace viz {
@@ -330,9 +331,9 @@ bool FakeSkiaOutputSurface::GetGrBackendTexture(
 }
 
 void FakeSkiaOutputSurface::SwapBuffersAck() {
-  client_->DidReceiveSwapBuffersAck();
-  client_->DidReceivePresentationFeedback(
-      {base::TimeTicks::Now(), base::TimeDelta(), 0});
+  base::TimeTicks now = base::TimeTicks::Now();
+  client_->DidReceiveSwapBuffersAck({now, now});
+  client_->DidReceivePresentationFeedback({now, base::TimeDelta(), 0});
 }
 
 }  // namespace viz

content/browser/compositor/gpu_browser_compositor_output_surface.cc

@@ -50,7 +50,7 @@ void GpuBrowserCompositorOutputSurface::OnGpuSwapBuffersCompleted(
     client_->DidReceiveCALayerParams(params.ca_layer_params);
   if (!params.texture_in_use_responses.empty())
     client_->DidReceiveTextureInUseResponses(params.texture_in_use_responses);
-  client_->DidReceiveSwapBuffersAck();
+  client_->DidReceiveSwapBuffersAck(params.swap_response.timings);
   UpdateLatencyInfoOnSwap(params.swap_response, &latency_info);
   latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);
 }

content/browser/compositor/offscreen_browser_compositor_output_surface.cc

@@ -186,7 +186,9 @@ void OffscreenBrowserCompositorOutputSurface::OnReflectorChanged() {
 void OffscreenBrowserCompositorOutputSurface::OnSwapBuffersComplete(
     const std::vector<ui::LatencyInfo>& latency_info) {
   latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);
-  client_->DidReceiveSwapBuffersAck();
+  // Swap timings are not available since for offscreen there is no Swap, just
+  // a SignalSyncToken.
+  client_->DidReceiveSwapBuffersAck(gfx::SwapTimings());
   client_->DidReceivePresentationFeedback(gfx::PresentationFeedback());
 }
 

content/browser/compositor/software_browser_compositor_output_surface.cc

@@ -81,14 +81,15 @@ void SoftwareBrowserCompositorOutputSurface::SwapBuffers(
 
   software_device()->OnSwapBuffers(base::BindOnce(
       &SoftwareBrowserCompositorOutputSurface::SwapBuffersCallback,
-      weak_factory_.GetWeakPtr(), frame.latency_info));
+      weak_factory_.GetWeakPtr(), frame.latency_info, swap_time));
 }
 
 void SoftwareBrowserCompositorOutputSurface::SwapBuffersCallback(
     const std::vector<ui::LatencyInfo>& latency_info,
+    const base::TimeTicks& swap_time,
     const gfx::Size& pixel_size) {
   latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);
-  client_->DidReceiveSwapBuffersAck();
+  client_->DidReceiveSwapBuffersAck({swap_time, swap_time});
 #if defined(USE_X11)
   if (needs_swap_size_notifications_)
     client_->DidSwapWithSize(pixel_size);

content/browser/compositor/software_browser_compositor_output_surface.h

@@ -46,6 +46,7 @@ class CONTENT_EXPORT SoftwareBrowserCompositorOutputSurface
 
  private:
   void SwapBuffersCallback(const std::vector<ui::LatencyInfo>& latency_info,
+                           const base::TimeTicks& swap_time,
                            const gfx::Size& pixel_size);
   void UpdateVSyncCallback(const base::TimeTicks timebase,
                            const base::TimeDelta interval);

content/browser/renderer_host/compositor_impl_android.cc

@@ -315,7 +315,7 @@ class AndroidOutputSurface : public viz::OutputSurface {
   void OnSwapBuffersCompleted(std::vector<ui::LatencyInfo> latency_info,
                               gfx::Size swap_size,
                               const gpu::SwapBuffersCompleteParams& params) {
-    client_->DidReceiveSwapBuffersAck();
+    client_->DidReceiveSwapBuffersAck(params.swap_response.timings);
     swap_buffers_callback_.Run(swap_size);
     UpdateLatencyInfoOnSwap(params.swap_response, &latency_info);
     latency_tracker_.OnGpuSwapBuffersCompleted(latency_info);

ui/compositor/test/in_process_context_factory.cc

@@ -134,7 +134,7 @@ class DirectOutputSurface : public viz::OutputSurface {
 
  private:
   void OnSwapBuffersComplete() {
-    client_->DidReceiveSwapBuffersAck();
+    client_->DidReceiveSwapBuffersAck(gfx::SwapTimings());
     client_->DidReceivePresentationFeedback(gfx::PresentationFeedback());
   }