Change swap ack with software compositing.

This CL moves the responsibility for acknowledging that swap has
occurred from SoftwareOutputSurface to SoftwareOutputDevice. This is to
allow handling asynchronous swap better. This CL should have no change
in behaviour but a follow up CL will add an overridden implementation of
SoftwareOutputDevice::OnSwapBuffers().

The main change is that instead of SoftwareOutputSurface holding a
TaskRunner the SoftwareOutputDevice does. Only the macOS
SoftwareOutputDevice needs a specific TaskRunner provided. All other
SoftwareOutputDevice implementations just grab the current one on
construction.

Bug: 826633
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I97f487db90fe18f4182d2210539f61f2cb694c6a
Reviewed-on: https://chromium-review.googlesource.com/1055476Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557910}

components/viz/service/display/software_output_device.cc

@@ -5,12 +5,19 @@
 #include "components/viz/service/display/software_output_device.h"
 
 #include "base/logging.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/gfx/vsync_provider.h"
 
 namespace viz {
 
-SoftwareOutputDevice::SoftwareOutputDevice() = default;
+SoftwareOutputDevice::SoftwareOutputDevice()
+    : SoftwareOutputDevice(base::SequencedTaskRunnerHandle::Get()) {}
+
+SoftwareOutputDevice::SoftwareOutputDevice(
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : task_runner_(std::move(task_runner)) {}
+
 SoftwareOutputDevice::~SoftwareOutputDevice() = default;
 
 void SoftwareOutputDevice::BindToClient(SoftwareOutputDeviceClient* client) {
@@ -42,4 +49,8 @@ gfx::VSyncProvider* SoftwareOutputDevice::GetVSyncProvider() {
   return vsync_provider_.get();
 }
 
+void SoftwareOutputDevice::OnSwapBuffers(base::OnceClosure swap_ack_callback) {
+  task_runner_->PostTask(FROM_HERE, std::move(swap_ack_callback));
+}
+
 }  // namespace viz

components/viz/service/display/software_output_device.h

@@ -7,7 +7,9 @@
 
 #include <memory>
 
+#include "base/callback.h"
 #include "base/macros.h"
+#include "base/sequenced_task_runner.h"
 #include "components/viz/service/display/software_output_device_client.h"
 #include "components/viz/service/viz_service_export.h"
 #include "third_party/skia/include/core/SkSurface.h"
@@ -29,7 +31,10 @@ class SoftwareOutputDeviceClient;
 // OutputSurface, such as to a platform-provided window framebuffer.
 class VIZ_SERVICE_EXPORT SoftwareOutputDevice {
  public:
+  // Uses TaskRunner returned from SequencedTaskRunnerHandle::Get().
   SoftwareOutputDevice();
+  explicit SoftwareOutputDevice(
+      scoped_refptr<base::SequencedTaskRunner> task_runner);
   virtual ~SoftwareOutputDevice();
 
   // This may be called only once, and requires a non-nullptr argument.
@@ -61,7 +66,14 @@ class VIZ_SERVICE_EXPORT SoftwareOutputDevice {
   // hardware vsync. Return null if a provider doesn't exist.
   virtual gfx::VSyncProvider* GetVSyncProvider();
 
+  // Called from OutputSurface::SwapBuffers(). The default implementation will
+  // immediately run |swap_ack_callback| via PostTask. If swap isn't synchronous
+  // this can be overriden so that |swap_ack_callback| is run after swap
+  // completes.
+  virtual void OnSwapBuffers(base::OnceClosure swap_ack_callback);
+
  protected:
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
   SoftwareOutputDeviceClient* client_ = nullptr;
   gfx::Size viewport_pixel_size_;
   gfx::Rect damage_rect_;

components/viz/service/display_embedder/gpu_display_provider.cc

@@ -114,7 +114,7 @@ std::unique_ptr<Display> GpuDisplayProvider::CreateDisplay(
 
   if (!gpu_compositing) {
     output_surface = std::make_unique<SoftwareOutputSurface>(
-        CreateSoftwareOutputDeviceForPlatform(surface_handle), task_runner_);
+        CreateSoftwareOutputDeviceForPlatform(surface_handle));
   } else if (renderer_settings.use_skia_renderer &&
              renderer_settings.use_skia_deferred_display_list) {
 #if defined(OS_MACOSX) || defined(OS_WIN)
@@ -208,7 +208,7 @@ GpuDisplayProvider::CreateSoftwareOutputDeviceForPlatform(
 #if defined(OS_WIN)
   return CreateSoftwareOutputDeviceWin(surface_handle, &output_device_backing_);
 #elif defined(OS_MACOSX)
-  return std::make_unique<SoftwareOutputDeviceMac>();
+  return std::make_unique<SoftwareOutputDeviceMac>(task_runner_);
 #elif defined(OS_ANDROID)
   // Android does not do software compositing, so we can't get here.
   NOTREACHED();

components/viz/service/display_embedder/software_output_device_mac.cc

@@ -16,7 +16,9 @@ namespace viz {
 SoftwareOutputDeviceMac::Buffer::Buffer() = default;
 SoftwareOutputDeviceMac::Buffer::~Buffer() = default;
 
-SoftwareOutputDeviceMac::SoftwareOutputDeviceMac() {}
+SoftwareOutputDeviceMac::SoftwareOutputDeviceMac(
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : SoftwareOutputDevice(std::move(task_runner)) {}
 
 SoftwareOutputDeviceMac::~SoftwareOutputDeviceMac() {}
 

components/viz/service/display_embedder/software_output_device_mac.h

@@ -21,7 +21,8 @@ namespace viz {
 
 class VIZ_SERVICE_EXPORT SoftwareOutputDeviceMac : public SoftwareOutputDevice {
  public:
-  SoftwareOutputDeviceMac();
+  explicit SoftwareOutputDeviceMac(
+      scoped_refptr<base::SequencedTaskRunner> task_runner);
   ~SoftwareOutputDeviceMac() override;
 
   // SoftwareOutputDevice implementation.

components/viz/service/display_embedder/software_output_device_mac_unittest.mm

@@ -4,6 +4,7 @@
 
 #include "components/viz/service/display_embedder/software_output_device_mac.h"
 
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/skia_util.h"
 
@@ -12,7 +13,8 @@ namespace viz {
 namespace {
 
 TEST(SoftwareOutputDeviceMacTest, Basics) {
-  auto device = std::make_unique<SoftwareOutputDeviceMac>();
+  auto device = std::make_unique<SoftwareOutputDeviceMac>(
+      base::SequencedTaskRunnerHandle::Get());
   gfx::Size pixel_size(512, 512);
   float scale_factor = 1;
 

components/viz/service/display_embedder/software_output_surface.cc

@@ -9,7 +9,6 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
-#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/viz/service/display/output_surface_client.h"
@@ -22,11 +21,8 @@
 namespace viz {
 
 SoftwareOutputSurface::SoftwareOutputSurface(
-    std::unique_ptr<SoftwareOutputDevice> software_device,
-    scoped_refptr<base::SequencedTaskRunner> task_runner)
-    : OutputSurface(std::move(software_device)),
-      task_runner_(std::move(task_runner)),
-      weak_factory_(this) {}
+    std::unique_ptr<SoftwareOutputDevice> software_device)
+    : OutputSurface(std::move(software_device)), weak_factory_(this) {}
 
 SoftwareOutputSurface::~SoftwareOutputSurface() = default;
 
@@ -84,9 +80,9 @@ void SoftwareOutputSurface::SwapBuffers(OutputSurfaceFrame frame) {
   // Update refresh_interval_ as well.
 
   ++swap_id_;
-  task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(&SoftwareOutputSurface::SwapBuffersCallback,
-                                weak_factory_.GetWeakPtr(), swap_id_));
+  software_device()->OnSwapBuffers(
+      base::BindOnce(&SoftwareOutputSurface::SwapBuffersCallback,
+                     weak_factory_.GetWeakPtr(), swap_id_));
 }
 
 bool SoftwareOutputSurface::IsDisplayedAsOverlayPlane() const {

components/viz/service/display_embedder/software_output_surface.h

@@ -6,8 +6,6 @@
 #define COMPONENTS_VIZ_SERVICE_DISPLAY_EMBEDDER_SOFTWARE_OUTPUT_SURFACE_H_
 
 #include "base/memory/weak_ptr.h"
-#include "base/sequenced_task_runner.h"
-#include "base/single_thread_task_runner.h"
 #include "components/viz/service/display/output_surface.h"
 #include "components/viz/service/viz_service_export.h"
 #include "ui/latency/latency_info.h"
@@ -18,8 +16,8 @@ class SoftwareOutputDevice;
 
 class VIZ_SERVICE_EXPORT SoftwareOutputSurface : public OutputSurface {
  public:
-  SoftwareOutputSurface(std::unique_ptr<SoftwareOutputDevice> software_device,
-                        scoped_refptr<base::SequencedTaskRunner> task_runner);
+  explicit SoftwareOutputSurface(
+      std::unique_ptr<SoftwareOutputDevice> software_device);
   ~SoftwareOutputSurface() override;
 
   // OutputSurface implementation.
@@ -49,7 +47,6 @@ class VIZ_SERVICE_EXPORT SoftwareOutputSurface : public OutputSurface {
   void SwapBuffersCallback(uint64_t swap_id);
 
   OutputSurfaceClient* client_ = nullptr;
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
   base::TimeDelta refresh_interval_;
   uint64_t swap_id_ = 0;
   std::vector<ui::LatencyInfo> stored_latency_info_;

content/browser/compositor/gpu_process_transport_factory.cc

@@ -229,7 +229,8 @@ GpuProcessTransportFactory::~GpuProcessTransportFactory() {
 
 std::unique_ptr<viz::SoftwareOutputDevice>
 GpuProcessTransportFactory::CreateSoftwareOutputDevice(
-    gfx::AcceleratedWidget widget) {
+    gfx::AcceleratedWidget widget,
+    scoped_refptr<base::SequencedTaskRunner> task_runner) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kHeadless))
     return base::WrapUnique(new viz::SoftwareOutputDevice);
@@ -255,7 +256,7 @@ GpuProcessTransportFactory::CreateSoftwareOutputDevice(
 #elif defined(USE_X11)
   return std::make_unique<viz::SoftwareOutputDeviceX11>(widget);
 #elif defined(OS_MACOSX)
-  return std::make_unique<viz::SoftwareOutputDeviceMac>();
+  return std::make_unique<viz::SoftwareOutputDeviceMac>(std::move(task_runner));
 #else
   NOTREACHED();
   return std::unique_ptr<viz::SoftwareOutputDevice>();
@@ -506,8 +507,9 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
       }
       display_output_surface =
           std::make_unique<SoftwareBrowserCompositorOutputSurface>(
-              CreateSoftwareOutputDevice(compositor->widget()),
-              std::move(vsync_callback), compositor->task_runner());
+              CreateSoftwareOutputDevice(compositor->widget(),
+                                         compositor->task_runner()),
+              std::move(vsync_callback));
     } else {
       DCHECK(context_provider);
       const auto& capabilities = context_provider->ContextCapabilities();

content/browser/compositor/gpu_process_transport_factory.h

@@ -111,7 +111,8 @@ class GpuProcessTransportFactory : public ui::ContextFactory,
 
   PerCompositorData* CreatePerCompositorData(ui::Compositor* compositor);
   std::unique_ptr<viz::SoftwareOutputDevice> CreateSoftwareOutputDevice(
-      gfx::AcceleratedWidget widget);
+      gfx::AcceleratedWidget widget,
+      scoped_refptr<base::SequencedTaskRunner> task_runner);
   void EstablishedGpuChannel(
       base::WeakPtr<ui::Compositor> compositor,
       bool use_gpu_compositing,

content/browser/compositor/software_browser_compositor_output_surface.cc

@@ -9,7 +9,6 @@
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
-#include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/viz/service/display/output_surface_client.h"
@@ -23,11 +22,9 @@ namespace content {
 
 SoftwareBrowserCompositorOutputSurface::SoftwareBrowserCompositorOutputSurface(
     std::unique_ptr<viz::SoftwareOutputDevice> software_device,
-    const UpdateVSyncParametersCallback& update_vsync_parameters_callback,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner)
+    const UpdateVSyncParametersCallback& update_vsync_parameters_callback)
     : BrowserCompositorOutputSurface(std::move(software_device),
                                      update_vsync_parameters_callback),
-      task_runner_(std::move(task_runner)),
       weak_factory_(this) {}
 
 SoftwareBrowserCompositorOutputSurface::
@@ -88,11 +85,9 @@ void SoftwareBrowserCompositorOutputSurface::SwapBuffers(
   }
 
   ++swap_id_;
-  task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &SoftwareBrowserCompositorOutputSurface::SwapBuffersCallback,
-          weak_factory_.GetWeakPtr(), swap_id_, frame.latency_info));
+  software_device()->OnSwapBuffers(base::BindOnce(
+      &SoftwareBrowserCompositorOutputSurface::SwapBuffersCallback,
+      weak_factory_.GetWeakPtr(), swap_id_, frame.latency_info));
 }
 
 void SoftwareBrowserCompositorOutputSurface::SwapBuffersCallback(

content/browser/compositor/software_browser_compositor_output_surface.h

@@ -7,17 +7,12 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/single_thread_task_runner.h"
 #include "build/build_config.h"
 #include "content/browser/compositor/browser_compositor_output_surface.h"
 #include "content/common/content_export.h"
 #include "gpu/vulkan/buildflags.h"
 #include "ui/latency/latency_tracker.h"
 
-namespace cc {
-class SoftwareOutputDevice;
-}
-
 namespace content {
 
 class CONTENT_EXPORT SoftwareBrowserCompositorOutputSurface
@@ -25,8 +20,7 @@ class CONTENT_EXPORT SoftwareBrowserCompositorOutputSurface
  public:
   SoftwareBrowserCompositorOutputSurface(
       std::unique_ptr<viz::SoftwareOutputDevice> software_device,
-      const UpdateVSyncParametersCallback& update_vsync_parameters_callback,
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+      const UpdateVSyncParametersCallback& update_vsync_parameters_callback);
 
   ~SoftwareBrowserCompositorOutputSurface() override;
 
@@ -57,7 +51,6 @@ class CONTENT_EXPORT SoftwareBrowserCompositorOutputSurface
                            const base::TimeDelta interval);
 
   viz::OutputSurfaceClient* client_ = nullptr;
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   uint64_t swap_id_ = 0;
   base::TimeDelta refresh_interval_;
   ui::LatencyTracker latency_tracker_;

content/browser/compositor/software_browser_compositor_output_surface_unittest.cc

@@ -130,8 +130,7 @@ SoftwareBrowserCompositorOutputSurfaceTest::CreateSurface(
       std::move(device),
       base::Bind(
           &SoftwareBrowserCompositorOutputSurfaceTest::UpdateVSyncParameters,
-          base::Unretained(this)),
-      base::ThreadTaskRunnerHandle::Get());
+          base::Unretained(this)));
 }
 
 void SoftwareBrowserCompositorOutputSurfaceTest::UpdateVSyncParameters(