From d8b65122a180cf8391e21b694d83b1a8de833465 Mon Sep 17 00:00:00 2001
From: Sunny Sachanandani <sunnyps@chromium.org>
Date: Wed, 08 Apr 2026 10:36:37 -0700
Subject: [PATCH] [viz] Clamp DComp overlay content rect

SkiaOutputDeviceDComp::ScheduleOverlays calculated content_rect without
clamping it to the actual image size, which could lead to out-of-bounds
memory access if the UV rect was out of bounds.

This CL adds clamping to the content_rect to ensure it stays within the
image bounds. It also handles cases where resource_size_in_pixels is
larger than the actual image size by clamping to the image dimensions.

Also, refactored SkiaOutputDeviceDComp to take GpuDriverBugWorkarounds
instead of FeatureInfo to simplify dependencies and make it easier to
test.

Added unit tests to verify the clamping behavior.

Bug: 497487755
Test: SkiaOutputDeviceDCompTest.*
Change-Id: Ie62e7504935ca27630d8b4fc8cc7629d6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7738282
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1611628}
---

diff --git a/components/viz/service/BUILD.gn b/components/viz/service/BUILD.gn
index 42a96534..668242b6 100644
--- a/components/viz/service/BUILD.gn
+++ b/components/viz/service/BUILD.gn
@@ -736,6 +736,7 @@
     sources += [
       "display/overlay_dc_unittest.cc",
       "display_embedder/output_device_backing_unittest.cc",
+      "display_embedder/skia_output_device_dcomp_unittest.cc",
       "display_embedder/software_output_device_win_swapchain_unittest.cc",
       "display_embedder/software_output_device_win_unittest.cc",
       "display_embedder/test_support/dcomp_mocks.cc",
diff --git a/components/viz/service/display_embedder/skia_output_device.cc b/components/viz/service/display_embedder/skia_output_device.cc
index 03ec61b..15e4d30 100644
--- a/components/viz/service/display_embedder/skia_output_device.cc
+++ b/components/viz/service/display_embedder/skia_output_device.cc
@@ -104,11 +104,14 @@
     CHECK(!graphite_shared_context);
     capabilities_.max_render_target_size = gr_context->maxRenderTargetSize();
     capabilities_.max_texture_size = gr_context->maxTextureSize();
-  } else {
-    CHECK(graphite_shared_context);
+  } else if (graphite_shared_context) {
     capabilities_.max_render_target_size =
         graphite_shared_context->maxTextureSize();
     capabilities_.max_texture_size = graphite_shared_context->maxTextureSize();
+  } else {
+    // Unit tests may create SkiaOutputDevice without a GPU context.
+    capabilities_.max_render_target_size = 8192;
+    capabilities_.max_texture_size = 8192;
   }
 }
 
diff --git a/components/viz/service/display_embedder/skia_output_device_dcomp.cc b/components/viz/service/display_embedder/skia_output_device_dcomp.cc
index 140ec190..c21a5319 100644
--- a/components/viz/service/display_embedder/skia_output_device_dcomp.cc
+++ b/components/viz/service/display_embedder/skia_output_device_dcomp.cc
@@ -18,7 +18,6 @@
 #include "components/viz/common/switches.h"
 #include "components/viz/service/display_embedder/skia_output_surface_dependency.h"
 #include "gpu/command_buffer/common/mailbox.h"
-#include "gpu/command_buffer/service/feature_info.h"
 #include "gpu/command_buffer/service/gl_utils.h"
 #include "gpu/command_buffer/service/shared_context_state.h"
 #include "gpu/command_buffer/service/shared_image/shared_image_backing.h"
@@ -26,6 +25,7 @@
 #include "gpu/command_buffer/service/shared_image/shared_image_representation.h"
 #include "gpu/command_buffer/service/skia_utils.h"
 #include "gpu/command_buffer/service/texture_manager.h"
+#include "gpu/config/gpu_driver_bug_workarounds.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/gpu/ganesh/GrDirectContext.h"
@@ -121,7 +121,7 @@
     gpu::SharedImageRepresentationFactory* shared_image_representation_factory,
     gpu::SharedContextState* context_state,
     scoped_refptr<gl::Presenter> presenter,
-    scoped_refptr<gpu::gles2::FeatureInfo> feature_info,
+    const gpu::GpuDriverBugWorkarounds& workarounds,
     gpu::MemoryTracker* memory_tracker,
     DidSwapBufferCompleteCallback did_swap_buffer_complete_callback)
     : SkiaOutputDevice(context_state->gr_context(),
@@ -131,8 +131,7 @@
       shared_image_representation_factory_(shared_image_representation_factory),
       context_state_(context_state),
       presenter_(std::move(presenter)) {
-  DCHECK(!feature_info->workarounds()
-              .disable_post_sub_buffers_for_onscreen_surfaces);
+  DCHECK(!workarounds.disable_post_sub_buffers_for_onscreen_surfaces);
   capabilities_.uses_default_gl_framebuffer = true;
   capabilities_.output_surface_origin = gfx::SurfaceOrigin::kTopLeft;
   capabilities_.number_of_buffers =
@@ -143,7 +142,7 @@
     // when the feature |DCompTripleBufferRootSwapChain| is enabled.
     capabilities_.number_of_buffers = 2;
   }
-  if (feature_info->workarounds().supports_two_yuv_hardware_overlays) {
+  if (workarounds.supports_two_yuv_hardware_overlays) {
     capabilities_.allowed_yuv_overlay_count = 2;
   }
   if (base::FeatureList::IsEnabled(
@@ -169,9 +168,6 @@
       !IsBufferQueueSupportedAndEnabled(capabilities_.dc_support_level);
 
   DCHECK(context_state_);
-  DCHECK(context_state_->gr_context() ||
-         context_state_->graphite_shared_context());
-  DCHECK(context_state_->context());
   DCHECK(presenter_);
 
   // SRGB
@@ -320,6 +316,9 @@
     params.content_rect = gfx::ScaleRect(
         dc_layer.uv_rect, dc_layer.resource_size_in_pixels.width(),
         dc_layer.resource_size_in_pixels.height());
+    if (params.overlay_image) {
+      params.content_rect.Intersect(gfx::RectF(params.overlay_image->size()));
+    }
 
     params.quad_rect = gfx::ToRoundedRect(dc_layer.display_rect);
     CHECK(std::holds_alternative<gfx::Transform>(dc_layer.transform));
diff --git a/components/viz/service/display_embedder/skia_output_device_dcomp.h b/components/viz/service/display_embedder/skia_output_device_dcomp.h
index 6d9ef9e7..22dd6af7 100644
--- a/components/viz/service/display_embedder/skia_output_device_dcomp.h
+++ b/components/viz/service/display_embedder/skia_output_device_dcomp.h
@@ -12,6 +12,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "components/viz/service/display_embedder/skia_output_device.h"
+#include "components/viz/service/viz_service_export.h"
 #include "ui/gl/presenter.h"
 
 namespace gl {
@@ -21,23 +22,20 @@
 namespace gpu {
 class SharedContextState;
 class SharedImageRepresentationFactory;
-
-namespace gles2 {
-class FeatureInfo;
-}  // namespace gles2
+class GpuDriverBugWorkarounds;
 }  // namespace gpu
 
 namespace viz {
 
 // Base class for DComp-backed OutputDevices.
-class SkiaOutputDeviceDComp : public SkiaOutputDevice {
+class VIZ_SERVICE_EXPORT SkiaOutputDeviceDComp : public SkiaOutputDevice {
  public:
   SkiaOutputDeviceDComp(
       gpu::SharedImageRepresentationFactory*
           shared_image_representation_factory,
       gpu::SharedContextState* context_state,
       scoped_refptr<gl::Presenter> presenter,
-      scoped_refptr<gpu::gles2::FeatureInfo> feature_info,
+      const gpu::GpuDriverBugWorkarounds& workarounds,
       gpu::MemoryTracker* memory_tracker,
       DidSwapBufferCompleteCallback did_swap_buffer_complete_callback);
 
@@ -56,12 +54,13 @@
       std::vector<GrBackendSemaphore>* end_semaphores) override;
   void EndPaint() override;
 
+ protected:
+  virtual std::optional<gl::DCLayerOverlayImage> BeginOverlayAccess(
+      const gpu::Mailbox& mailbox);
+
  private:
   class OverlayData;
 
-  std::optional<gl::DCLayerOverlayImage> BeginOverlayAccess(
-      const gpu::Mailbox& mailbox);
-
   void CreateSkSurface();
 
   // Mailboxes of overlays scheduled in the current frame.
diff --git a/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc b/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc
new file mode 100644
index 0000000..0db5ef1
--- /dev/null
+++ b/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc
@@ -0,0 +1,178 @@
+// Copyright 2026 The Chromium Authors
+// 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/skia_output_device_dcomp.h"
+
+#include <memory>
+#include <utility>

diff --git a/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc b/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc
new file mode 100644
index 0000000..0db5ef1
--- /dev/null
+++ b/components/viz/service/display_embedder/skia_output_device_dcomp_unittest.cc
@@ -0,0 +1,178 @@
+// Copyright 2026 The Chromium Authors
+// 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/skia_output_device_dcomp.h"
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/containers/flat_map.h"
+#include "base/functional/callback_helpers.h"
+#include "base/memory/scoped_refptr.h"
+#include "components/viz/service/display/overlay_candidate.h"
+#include "gpu/config/gpu_driver_bug_workarounds.h"
+#include "gpu/command_buffer/service/shared_context_state.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gl/dc_layer_overlay_image.h"
+#include "ui/gl/dc_layer_overlay_params.h"
+#include "ui/gl/presenter.h"
+#include "ui/gl/gl_context_stub.h"
+#include "ui/gl/gl_surface_stub.h"
+
+using ::testing::_;
+using ::testing::Invoke;
+
+namespace viz {
+namespace {
+
+class MockPresenter : public gl::Presenter {
+ public:
+  MockPresenter() = default;
+
+  void Present(SwapCompletionCallback completion_callback,
+               PresentationCallback presentation_callback,
+               gfx::FrameData data) override {
+    std::move(completion_callback)
+        .Run(gfx::SwapCompletionResult(gfx::SwapResult::SWAP_ACK));
+    std::move(presentation_callback).Run({});
+  }
+
+  bool ScheduleOverlayPlane(
+      gl::OverlayImage image,
+      std::unique_ptr<gfx::GpuFence> gpu_fence,
+      const gfx::OverlayPlaneData& overlay_plane_data) override {
+    return true;
+  }
+
+  MOCK_METHOD1(ScheduleDCLayers, void(std::vector<gl::DCLayerOverlayParams>));
+
+  bool SupportsDelegatedInk() override { return false; }
+  HWND GetWindow() const override { return nullptr; }
+  bool DestroyDCLayerTree() override { return true; }
+
+ protected:
+  ~MockPresenter() override = default;
+};
+
+class TestSkiaOutputDeviceDComp : public SkiaOutputDeviceDComp {
+ public:
+  TestSkiaOutputDeviceDComp(scoped_refptr<gl::Presenter> presenter,
+                            gpu::SharedContextState* context_state)
+      : SkiaOutputDeviceDComp(
+            /*shared_image_representation_factory=*/nullptr,
+            context_state,
+            std::move(presenter),
+            /*workarounds=*/gpu::GpuDriverBugWorkarounds(),
+            /*memory_tracker=*/nullptr,
+            /*did_swap_buffer_complete_callback=*/base::DoNothing()) {}
+
+  void SetOverlayImageSize(const gpu::Mailbox& mailbox, const gfx::Size& size) {
+    overlay_sizes_[mailbox] = size;
+  }
+
+ protected:
+  std::optional<gl::DCLayerOverlayImage> BeginOverlayAccess(
+      const gpu::Mailbox& mailbox) override {
+    auto it = overlay_sizes_.find(mailbox);
+    if (it != overlay_sizes_.end()) {
+      return gl::DCLayerOverlayImage(it->second,
+                                     Microsoft::WRL::ComPtr<IUnknown>());
+    }
+    return std::nullopt;
+  }
+
+ private:
+  base::flat_map<gpu::Mailbox, gfx::Size> overlay_sizes_;
+};
+
+class SkiaOutputDeviceDCompTest : public testing::Test {
+ public:
+  void SetUp() override {
+    auto presenter = base::MakeRefCounted<MockPresenter>();
+    presenter_ = presenter.get();
+
+    auto surface = base::MakeRefCounted<gl::GLSurfaceStub>();
+    auto context = base::MakeRefCounted<gl::GLContextStub>();
+    context->Initialize(surface.get(), gl::GLContextAttribs());
+    context_state_ = base::MakeRefCounted<gpu::SharedContextState>(
+        /*share_group=*/nullptr,
+        surface,
+        context,
+        /*use_virtualized_gl_contexts=*/false,
+        /*context_lost_callback=*/base::DoNothing(),
+        gpu::GrContextType::kGraphiteDawn);
+
+    output_device_ = std::make_unique<TestSkiaOutputDeviceDComp>(
+        std::move(presenter), context_state_.get());
+  }
+
+  void TearDown() override {
+    presenter_ = nullptr;
+    output_device_.reset();
+    context_state_.reset();
+  }
+
+ protected:
+  scoped_refptr<gpu::SharedContextState> context_state_;
+  std::unique_ptr<TestSkiaOutputDeviceDComp> output_device_;
+  raw_ptr<MockPresenter> presenter_ = nullptr;
+};
+
+TEST_F(SkiaOutputDeviceDCompTest, ClampsOutOfBoundsUVRect) {
+  gfx::Size image_size(100, 100);
+  gpu::Mailbox mailbox = gpu::Mailbox::Generate();
+  output_device_->SetOverlayImageSize(mailbox, image_size);
+
+  OverlayCandidate candidate;
+  candidate.mailbox = mailbox;
+  candidate.display_rect = gfx::RectF(0, 0, 100, 100);
+  candidate.uv_rect = gfx::RectF(-0.1f, -0.1f, 1.2f, 1.2f);  // OOB
+  candidate.resource_size_in_pixels = image_size;
+  candidate.transform = gfx::Transform();
+
+  SkiaOutputSurface::OverlayList overlays;
+  overlays.push_back(candidate);
+
+  EXPECT_CALL(*presenter_, ScheduleDCLayers(_))
+      .WillOnce([](std::vector<gl::DCLayerOverlayParams> params) {
+        ASSERT_EQ(params.size(), 1u);
+        // The uv_rect is [-0.1, -0.1, 1.2, 1.2].
+        // Content rect in pixels would be [-10, -10, 120, 120].
+        // Clamped content rect should be [0, 0, 100, 100].
+        EXPECT_EQ(params[0].content_rect, gfx::RectF(0, 0, 100, 100));
+      });
+
+  output_device_->ScheduleOverlays(std::move(overlays));
+}
+
+TEST_F(SkiaOutputDeviceDCompTest, ClampsOutOfBoundsResourceSize) {
+  gfx::Size image_size(100, 100);
+  gpu::Mailbox mailbox = gpu::Mailbox::Generate();
+  output_device_->SetOverlayImageSize(mailbox, image_size);
+
+  OverlayCandidate candidate;
+  candidate.mailbox = mailbox;
+  candidate.display_rect = gfx::RectF(0, 0, 100, 100);
+  candidate.uv_rect = gfx::RectF(0.0f, 0.0f, 1.0f, 1.0f);
+  candidate.resource_size_in_pixels = gfx::Size(150, 150);
+  candidate.transform = gfx::Transform();
+
+  SkiaOutputSurface::OverlayList overlays;
+  overlays.push_back(candidate);
+
+  EXPECT_CALL(*presenter_, ScheduleDCLayers(_))
+      .WillOnce([](std::vector<gl::DCLayerOverlayParams> params) {
+        ASSERT_EQ(params.size(), 1u);
+        // ScaleRect gives [0, 0, 150, 150].
+        // Clamped to image size [100, 100] gives [0, 0, 100, 100].
+        EXPECT_EQ(params[0].content_rect, gfx::RectF(0, 0, 100, 100));
+      });
+
+  output_device_->ScheduleOverlays(std::move(overlays));
+}
+
+}  // namespace
+}  // namespace viz