Add SoftwareOutputDeviceWinProxyTest

This CL moves the implementation of Windows SoftwareOutputDevice
class declarations into the header so they can be tested. It also adds
unit tests for SoftwareOutputDeviceWinProxy.

Change-Id: I2c4173dadc24965f70b4d94663849a5555d8225c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124298
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756013}

components/viz/service/BUILD.gn

@@ -526,6 +526,7 @@ viz_source_set("unit_tests") {
     sources += [
       "display/overlay_dc_unittest.cc",
       "display_embedder/output_device_backing_unittest.cc",
+      "display_embedder/software_output_device_win_unittest.cc",
     ]
   }
 

components/viz/service/display_embedder/software_output_device_win.cc

@@ -4,66 +4,35 @@
 
 #include "components/viz/service/display_embedder/software_output_device_win.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "base/memory/unsafe_shared_memory_region.h"
-#include "base/threading/thread_checker.h"
 #include "base/trace_event/trace_event.h"
 #include "base/win/windows_version.h"
 #include "components/viz/common/display/use_layered_window.h"
 #include "components/viz/common/resources/resource_sizes.h"
-#include "components/viz/service/display_embedder/output_device_backing.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
-#include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 #include "services/viz/privileged/mojom/compositing/layered_window_updater.mojom.h"
 #include "skia/ext/platform_canvas.h"
 #include "skia/ext/skia_utils_win.h"
-#include "third_party/skia/include/core/SkCanvas.h"
 #include "ui/gfx/gdi_util.h"
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/win/hwnd_util.h"
 #include "ui/gl/vsync_provider_win.h"
 
 namespace viz {
-namespace {
-
-// Shared base class for Windows SoftwareOutputDevice implementations.
-class SoftwareOutputDeviceWinBase : public SoftwareOutputDevice {
- public:
-  explicit SoftwareOutputDeviceWinBase(HWND hwnd) : hwnd_(hwnd) {
-    vsync_provider_ = std::make_unique<gl::VSyncProviderWin>(hwnd);
-  }
-
-  ~SoftwareOutputDeviceWinBase() override {
-    DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-    DCHECK(!in_paint_);
-  }
-
-  HWND hwnd() const { return hwnd_; }
-
-  // SoftwareOutputDevice implementation.
-  void Resize(const gfx::Size& viewport_pixel_size,
-              float scale_factor) override;
-  SkCanvas* BeginPaint(const gfx::Rect& damage_rect) override;
-  void EndPaint() override;
-
-  // Called from Resize() if |viewport_pixel_size_| has changed.
-  virtual void ResizeDelegated() = 0;
 
-  // Called from BeginPaint() and should return an SkCanvas.
-  virtual SkCanvas* BeginPaintDelegated() = 0;
-
-  // Called from EndPaint() if there is damage.
-  virtual void EndPaintDelegated(const gfx::Rect& damage_rect) = 0;
-
- private:
-  const HWND hwnd_;
-  bool in_paint_ = false;
-
-  THREAD_CHECKER(thread_checker_);
+SoftwareOutputDeviceWinBase::SoftwareOutputDeviceWinBase(HWND hwnd)
+    : hwnd_(hwnd) {
+  vsync_provider_ = std::make_unique<gl::VSyncProviderWin>(hwnd);
+}
 
-  DISALLOW_COPY_AND_ASSIGN(SoftwareOutputDeviceWinBase);
-};
+SoftwareOutputDeviceWinBase::~SoftwareOutputDeviceWinBase() {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK(!in_paint_);
+}
 
 void SoftwareOutputDeviceWinBase::Resize(const gfx::Size& viewport_pixel_size,
                                          float scale_factor) {
@@ -101,32 +70,6 @@ void SoftwareOutputDeviceWinBase::EndPaint() {
   EndPaintDelegated(intersected_damage_rect);
 }
 
-// SoftwareOutputDevice implementation that draws directly to the provided HWND.
-// The backing buffer for paint is shared for all instances of this class.
-class SoftwareOutputDeviceWinDirect : public SoftwareOutputDeviceWinBase,
-                                      public OutputDeviceBacking::Client {
- public:
-  SoftwareOutputDeviceWinDirect(HWND hwnd, OutputDeviceBacking* backing);
-  ~SoftwareOutputDeviceWinDirect() override;
-
-  // SoftwareOutputDeviceWinBase implementation.
-  void ResizeDelegated() override;
-  SkCanvas* BeginPaintDelegated() override;
-  void EndPaintDelegated(const gfx::Rect& damage_rect) override;
-
-  // OutputDeviceBacking::Client implementation.
-  const gfx::Size& GetViewportPixelSize() const override {
-    return viewport_pixel_size_;
-  }
-  void ReleaseCanvas() override { canvas_.reset(); }
-
- private:
-  OutputDeviceBacking* const backing_;
-  std::unique_ptr<SkCanvas> canvas_;
-
-  DISALLOW_COPY_AND_ASSIGN(SoftwareOutputDeviceWinDirect);
-};
-
 SoftwareOutputDeviceWinDirect::SoftwareOutputDeviceWinDirect(
     HWND hwnd,
     OutputDeviceBacking* backing)
@@ -173,38 +116,13 @@ void SoftwareOutputDeviceWinDirect::EndPaintDelegated(
   ::ReleaseDC(hwnd(), hdc);
 }
 
-// SoftwareOutputDevice implementation that uses layered window API to draw
-// indirectly. Since UpdateLayeredWindow() is blocked by the GPU sandbox an
-// implementation of mojom::LayeredWindowUpdater in the browser process handles
-// calling UpdateLayeredWindow. Pixel backing is in a SharedMemoryRegion so no
-// copying between processes is required.
-class SoftwareOutputDeviceWinProxy : public SoftwareOutputDeviceWinBase {
- public:
-  SoftwareOutputDeviceWinProxy(
-      HWND hwnd,
-      mojo::PendingRemote<mojom::LayeredWindowUpdater> layered_window_updater);
-  ~SoftwareOutputDeviceWinProxy() override = default;
-
-  // SoftwareOutputDevice implementation.
-  void OnSwapBuffers(SwapBuffersCallback swap_ack_callback) override;
-
-  // SoftwareOutputDeviceWinBase implementation.
-  void ResizeDelegated() override;
-  SkCanvas* BeginPaintDelegated() override;
-  void EndPaintDelegated(const gfx::Rect& rect) override;
-
- private:
-  // Runs |swap_ack_callback_| after draw has happened.
-  void DrawAck();
-
-  mojo::Remote<mojom::LayeredWindowUpdater> layered_window_updater_;
-
-  std::unique_ptr<SkCanvas> canvas_;
-  bool waiting_on_draw_ack_ = false;
-  base::OnceClosure swap_ack_callback_;
+const gfx::Size& SoftwareOutputDeviceWinDirect::GetViewportPixelSize() const {
+  return viewport_pixel_size_;
+}
 
-  DISALLOW_COPY_AND_ASSIGN(SoftwareOutputDeviceWinProxy);
-};
+void SoftwareOutputDeviceWinDirect::ReleaseCanvas() {
+  canvas_.reset();
+}
 
 SoftwareOutputDeviceWinProxy::SoftwareOutputDeviceWinProxy(
     HWND hwnd,
@@ -214,6 +132,8 @@ SoftwareOutputDeviceWinProxy::SoftwareOutputDeviceWinProxy(
   DCHECK(layered_window_updater_.is_bound());
 }
 
+SoftwareOutputDeviceWinProxy::~SoftwareOutputDeviceWinProxy() = default;
+
 void SoftwareOutputDeviceWinProxy::OnSwapBuffers(
     SwapBuffersCallback swap_ack_callback) {
   DCHECK(swap_ack_callback_.is_null());
@@ -288,8 +208,6 @@ void SoftwareOutputDeviceWinProxy::DrawAck() {
     std::move(swap_ack_callback_).Run();
 }
 
-}  // namespace
-
 std::unique_ptr<SoftwareOutputDevice> CreateSoftwareOutputDeviceWin(
     HWND hwnd,
     OutputDeviceBacking* backing,

components/viz/service/display_embedder/software_output_device_win.h

@@ -9,13 +9,106 @@
 
 #include <memory>
 
+#include "base/threading/thread_checker.h"
 #include "components/viz/service/display/software_output_device.h"
+#include "components/viz/service/display_embedder/output_device_backing.h"
 #include "components/viz/service/viz_service_export.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/viz/privileged/mojom/compositing/display_private.mojom.h"
+#include "services/viz/privileged/mojom/compositing/layered_window_updater.mojom.h"
+#include "third_party/skia/include/core/SkCanvas.h"
 
 namespace viz {
 
-class OutputDeviceBacking;
+// Shared base class for Windows SoftwareOutputDevice implementations.
+class VIZ_SERVICE_EXPORT SoftwareOutputDeviceWinBase
+    : public SoftwareOutputDevice {
+ public:
+  explicit SoftwareOutputDeviceWinBase(HWND hwnd);
+  SoftwareOutputDeviceWinBase(const SoftwareOutputDeviceWinBase& other) =
+      delete;
+  SoftwareOutputDeviceWinBase& operator=(
+      const SoftwareOutputDeviceWinBase& other) = delete;
+  ~SoftwareOutputDeviceWinBase() override;
+
+  HWND hwnd() const { return hwnd_; }
+
+  // SoftwareOutputDevice implementation.
+  void Resize(const gfx::Size& viewport_pixel_size,
+              float scale_factor) override;
+  SkCanvas* BeginPaint(const gfx::Rect& damage_rect) override;
+  void EndPaint() override;
+
+  // Called from Resize() if |viewport_pixel_size_| has changed.
+  virtual void ResizeDelegated() = 0;
+
+  // Called from BeginPaint() and should return an SkCanvas.
+  virtual SkCanvas* BeginPaintDelegated() = 0;
+
+  // Called from EndPaint() if there is damage.
+  virtual void EndPaintDelegated(const gfx::Rect& damage_rect) = 0;
+
+ private:
+  const HWND hwnd_;
+  bool in_paint_ = false;
+
+  THREAD_CHECKER(thread_checker_);
+};
+
+// SoftwareOutputDevice implementation that draws directly to the provided HWND.
+// The backing buffer for paint is shared for all instances of this class.
+class VIZ_SERVICE_EXPORT SoftwareOutputDeviceWinDirect
+    : public SoftwareOutputDeviceWinBase,
+      public OutputDeviceBacking::Client {
+ public:
+  SoftwareOutputDeviceWinDirect(HWND hwnd, OutputDeviceBacking* backing);
+  ~SoftwareOutputDeviceWinDirect() override;
+
+  // SoftwareOutputDeviceWinBase implementation.
+  void ResizeDelegated() override;
+  SkCanvas* BeginPaintDelegated() override;
+  void EndPaintDelegated(const gfx::Rect& damage_rect) override;
+
+  // OutputDeviceBacking::Client implementation.
+  const gfx::Size& GetViewportPixelSize() const override;
+  void ReleaseCanvas() override;
+
+ private:
+  OutputDeviceBacking* const backing_;
+  std::unique_ptr<SkCanvas> canvas_;
+};
+
+// SoftwareOutputDevice implementation that uses layered window API to draw
+// indirectly. Since UpdateLayeredWindow() is blocked by the GPU sandbox an
+// implementation of mojom::LayeredWindowUpdater in the browser process handles
+// calling UpdateLayeredWindow. Pixel backing is in a SharedMemoryRegion so no
+// copying between processes is required.
+class VIZ_SERVICE_EXPORT SoftwareOutputDeviceWinProxy
+    : public SoftwareOutputDeviceWinBase {
+ public:
+  SoftwareOutputDeviceWinProxy(
+      HWND hwnd,
+      mojo::PendingRemote<mojom::LayeredWindowUpdater> layered_window_updater);
+  ~SoftwareOutputDeviceWinProxy() override;
+
+  // SoftwareOutputDevice implementation.
+  void OnSwapBuffers(SwapBuffersCallback swap_ack_callback) override;
+
+  // SoftwareOutputDeviceWinBase implementation.
+  void ResizeDelegated() override;
+  SkCanvas* BeginPaintDelegated() override;
+  void EndPaintDelegated(const gfx::Rect& rect) override;
+
+ private:
+  // Runs |swap_ack_callback_| after draw has happened.
+  void DrawAck();
+
+  mojo::Remote<mojom::LayeredWindowUpdater> layered_window_updater_;
+
+  std::unique_ptr<SkCanvas> canvas_;
+  bool waiting_on_draw_ack_ = false;
+  base::OnceClosure swap_ack_callback_;
+};
 
 // Creates an appropriate SoftwareOutputDevice implementation.
 VIZ_SERVICE_EXPORT std::unique_ptr<SoftwareOutputDevice>

components/viz/service/display_embedder/software_output_device_win_unittest.cc

+// Copyright 2020 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 "components/viz/service/display_embedder/software_output_device_win.h"
+
+#include "base/memory/unsafe_shared_memory_region.h"
+#include "base/test/gmock_callback_support.h"
+#include "components/viz/common/resources/resource_sizes.h"
+#include "services/viz/privileged/mojom/compositing/layered_window_updater.mojom.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using base::test::RunOnceClosure;
+using testing::_;
+
+namespace viz {
+namespace {
+
+constexpr gfx::Size kDefaultSize(100, 100);
+
+class MockLayeredWindowUpdater : public mojom::LayeredWindowUpdater {
+ public:
+  MockLayeredWindowUpdater() = default;
+  ~MockLayeredWindowUpdater() override = default;
+
+  mojo::PendingRemote<mojom::LayeredWindowUpdater> BindNewPipeAndPassRemote() {
+    return receiver_.BindNewPipeAndPassRemote();
+  }
+
+  void FlushForTesting() { receiver_.FlushForTesting(); }
+
+  // mojom::LayeredWindowUpdater implementation.
+  MOCK_METHOD(void,
+              OnAllocatedSharedMemory,
+              (const gfx::Size& size,
+               base::UnsafeSharedMemoryRegion memory_region),
+              (override));
+  MOCK_METHOD(void, Draw, (DrawCallback), (override));
+
+ private:
+  mojo::Receiver<mojom::LayeredWindowUpdater> receiver_{this};
+};
+
+}  // namespace
+
+class SoftwareOutputDeviceWinProxyTest : public testing::Test {
+ public:
+  SoftwareOutputDeviceWinProxyTest()
+      : device_(0, updater_.BindNewPipeAndPassRemote()) {}
+
+  void SetUp() override {
+    // Check that calling Resize() results in the allocation of shared memory
+    // and triggers the OnAllocatedSharedMemory() IPC.
+    EXPECT_CALL(updater_, OnAllocatedSharedMemory(kDefaultSize, _))
+        .WillOnce(
+            testing::WithArg<1>([](base::UnsafeSharedMemoryRegion region) {
+              EXPECT_TRUE(region.IsValid());
+              size_t required_bytes = ResourceSizes::CheckedSizeInBytes<size_t>(
+                  kDefaultSize, ResourceFormat::RGBA_8888);
+              EXPECT_GE(region.GetSize(), required_bytes);
+            }));
+    device_.Resize(kDefaultSize, 1.0f);
+    updater_.FlushForTesting();
+    testing::Mock::VerifyAndClearExpectations(&updater_);
+  }
+
+ protected:
+  MockLayeredWindowUpdater updater_;
+  SoftwareOutputDeviceWinProxy device_;
+};
+
+TEST_F(SoftwareOutputDeviceWinProxyTest, DrawWithSwap) {
+  // Verify that calling BeginPaint() then EndPaint() triggers the Draw() IPC
+  // and then run the DrawCallback.
+  device_.BeginPaint(gfx::Rect(kDefaultSize));
+  EXPECT_CALL(updater_, Draw(_)).WillOnce(RunOnceClosure<0>());
+  device_.EndPaint();
+  updater_.FlushForTesting();
+  testing::Mock::VerifyAndClearExpectations(&updater_);
+
+  // OnSwapBuffers() is called before DrawAck() so the swap buffers callback
+  // shouldn't run yet.
+  bool called = false;
+  device_.OnSwapBuffers(base::BindOnce(
+      [](bool* val, const gfx::Size& size) { *val = true; }, &called));
+  EXPECT_FALSE(called);
+
+  // Verify that DrawAck() runs the swap buffers callback.
+  updater_.FlushForTesting();
+  EXPECT_TRUE(called);
+}
+
+TEST_F(SoftwareOutputDeviceWinProxyTest, DrawNoSwap) {
+  // Verify that calling BeginPaint() then EndPaint() triggers the Draw() IPC
+  // and then run the DrawCallback.
+  device_.BeginPaint(gfx::Rect(kDefaultSize));
+  EXPECT_CALL(updater_, Draw(_)).WillOnce(RunOnceClosure<0>());
+  device_.EndPaint();
+  updater_.FlushForTesting();
+  testing::Mock::VerifyAndClearExpectations(&updater_);
+
+  // DrawAck() will be triggered after this second flush when the IPC response
+  // arrives. OnSwapBuffers() was never called which is allowed.
+  updater_.FlushForTesting();
+}
+
+}  // namespace viz