Fix paint issue with transparent scrollbars

LayoutBox::BackgroundClipBorderBoxIsEquivalentToPaddingBox now correctly accounts for non-auto scrollbar gutters, along with non-opaque scrollbars. ScrollbarDisplayItem::IsOpaque also now handles non-opaque non-overlay scrollbars. Bug: 1490798, 1459802 Change-Id: I7c3d5c3c1a498b43082dddfb7cab9850951e4ec6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4952560 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: Luke <lukewarlow156@gmail.com> Cr-Commit-Position: refs/heads/main@{#1212641}
chromium · Oct 20, 2023 · 7176250 · 7176250
1 parent 034ff93
commit 7176250
Show file tree

Hide file tree

Showing 14 changed files with 85 additions and 14 deletions.
diff --git a/cc/input/scrollbar.h b/cc/input/scrollbar.h
@@ -53,6 +53,7 @@ class Scrollbar : public base::RefCounted<Scrollbar> {
   virtual bool HasThumb() const = 0;
   virtual bool SupportsDragSnapBack() const = 0;
   virtual bool JumpOnTrackClick() const = 0;
+  virtual bool IsOpaque() const = 0;
 
   // The following rects are all relative to the scrollbar's origin.
   // The location of ThumbRect reflects scroll offset, but cc will ignore it

diff --git a/cc/test/fake_scrollbar.cc b/cc/test/fake_scrollbar.cc
@@ -116,4 +116,8 @@ gfx::Rect FakeScrollbar::NinePatchThumbAperture() const {
   return uses_nine_patch_thumb_resource_ ? gfx::Rect(0, 0, 5, 5) : gfx::Rect();
 }
 
+bool FakeScrollbar::IsOpaque() const {
+  return !is_overlay_ && is_opaque_;
+}
+
 }  // namespace cc
diff --git a/cc/test/fake_scrollbar.h b/cc/test/fake_scrollbar.h
@@ -45,6 +45,7 @@ class FakeScrollbar : public Scrollbar {
   gfx::Rect NinePatchThumbAperture() const override;
   gfx::Rect ShrinkMainThreadedMinimalModeThumbRect(
       gfx::Rect& rect) const override;
+  bool IsOpaque() const override;
 
   void set_should_paint(bool b) { should_paint_ = b; }
   void set_has_thumb(bool b) { has_thumb_ = b; }
@@ -69,6 +70,7 @@ class FakeScrollbar : public Scrollbar {
   void set_needs_repaint_track(bool needs_repaint) {
     needs_repaint_track_ = needs_repaint;
   }
+  void set_is_opaque(bool b) { is_opaque_ = b; }
 
  protected:
   ~FakeScrollbar() override;
@@ -91,6 +93,7 @@ class FakeScrollbar : public Scrollbar {
   gfx::Rect back_button_rect_;
   gfx::Rect forward_button_rect_;
   SkColor fill_color_ = SK_ColorGREEN;
+  bool is_opaque_ = true;
 };
 
 }  // namespace cc

diff --git a/third_party/blink/renderer/core/layout/custom_scrollbar.h b/third_party/blink/renderer/core/layout/custom_scrollbar.h
@@ -68,6 +68,9 @@ class CORE_EXPORT CustomScrollbar final : public Scrollbar {
 
   void PositionScrollbarParts();
 
+  // Custom scrollbars may be translucent.
+  bool IsOpaque() const override { return false; }
+
   LayoutCustomScrollbarPart* GetPart(ScrollbarPart part_type) {
     auto it = parts_.find(part_type);
     return it != parts_.end() ? it->value.Get() : nullptr;

diff --git a/third_party/blink/renderer/core/layout/layout_box.cc b/third_party/blink/renderer/core/layout/layout_box.cc
@@ -4126,16 +4126,18 @@ static bool HasInsetBoxShadow(const ComputedStyle& style) {
 // If all borders and scrollbars are opaque, then background-clip: border-box
 // is equivalent to background-clip: padding-box.
 bool LayoutBox::BackgroundClipBorderBoxIsEquivalentToPaddingBox() const {
-  // Custom scrollbars may be translucent.
-  // TODO(flackr): Detect opaque custom scrollbars which would cover up a
-  // border-box background.
   const auto* scrollable_area = GetScrollableArea();
-  if (scrollable_area &&
-      ((scrollable_area->HorizontalScrollbar() &&
-        scrollable_area->HorizontalScrollbar()->IsCustomScrollbar()) ||
-       (scrollable_area->VerticalScrollbar() &&
-        scrollable_area->VerticalScrollbar()->IsCustomScrollbar()))) {
-    return false;
+  if (scrollable_area) {
+    if (auto* scrollbar = scrollable_area->HorizontalScrollbar()) {
+      if (!scrollbar->IsOverlayScrollbar() && !scrollbar->IsOpaque()) {
+        return false;
+      }
+    }
+    if (auto* scrollbar = scrollable_area->VerticalScrollbar()) {
+      if (!scrollbar->IsOverlayScrollbar() && !scrollbar->IsOpaque()) {
+        return false;
+      }
+    }
   }
 
   if (StyleRef().BorderTopWidth() &&
@@ -4159,6 +4161,10 @@ bool LayoutBox::BackgroundClipBorderBoxIsEquivalentToPaddingBox() const {
     return false;
   }
 
+  if (!StyleRef().IsScrollbarGutterAuto()) {
+    return false;
+  }
+
   return true;
 }
 

diff --git a/third_party/blink/renderer/core/scroll/scrollbar.cc b/third_party/blink/renderer/core/scroll/scrollbar.cc
@@ -935,6 +935,19 @@ absl::optional<blink::Color> Scrollbar::ScrollbarTrackColor() const {
   return absl::nullopt;
 }
 
+bool Scrollbar::IsOpaque() const {
+  if (IsOverlayScrollbar()) {
+    return false;
+  }
+
+  absl::optional<blink::Color> track_color = ScrollbarTrackColor();
+  if (!track_color) {
+    // The native themes should ensure opaqueness of non-overlay scrollbars.
+    return true;
+  }
+  return track_color->IsOpaque();
+}
+
 mojom::blink::ColorScheme Scrollbar::UsedColorScheme() const {
   return scrollable_area_->UsedColorSchemeScrollbars();
 }

diff --git a/third_party/blink/renderer/core/scroll/scrollbar.h b/third_party/blink/renderer/core/scroll/scrollbar.h
@@ -215,6 +215,8 @@ class CORE_EXPORT Scrollbar : public GarbageCollected<Scrollbar>,
   absl::optional<blink::Color> ScrollbarThumbColor() const;
   absl::optional<blink::Color> ScrollbarTrackColor() const;
 
+  virtual bool IsOpaque() const;
+
   // The LayoutObject that supplies our style information. If the scrollbar is
   // for a document, this is:
   // 1. the LayoutView (with some scrollbar related styles propagated from the

diff --git a/third_party/blink/renderer/core/scroll/scrollbar_layer_delegate.cc b/third_party/blink/renderer/core/scroll/scrollbar_layer_delegate.cc
@@ -100,6 +100,10 @@ bool ScrollbarLayerDelegate::JumpOnTrackClick() const {
   return scrollbar_->GetTheme().JumpOnTrackClick();
 }
 
+bool ScrollbarLayerDelegate::IsOpaque() const {
+  return scrollbar_->IsOpaque();
+}
+
 gfx::Rect ScrollbarLayerDelegate::BackButtonRect() const {
   gfx::Rect back_button_rect =
       scrollbar_->GetTheme().BackButtonRect(*scrollbar_);

diff --git a/third_party/blink/renderer/core/scroll/scrollbar_layer_delegate.h b/third_party/blink/renderer/core/scroll/scrollbar_layer_delegate.h
@@ -31,6 +31,7 @@ class CORE_EXPORT ScrollbarLayerDelegate : public cc::Scrollbar {
   bool IsFluentOverlayScrollbarMinimalMode() const override;
   bool SupportsDragSnapBack() const override;
   bool JumpOnTrackClick() const override;
+  bool IsOpaque() const override;
 
   // The following rects are all relative to the scrollbar's origin.
   gfx::Rect ThumbRect() const override;

diff --git a/third_party/blink/renderer/platform/graphics/paint/scrollbar_display_item.cc b/third_party/blink/renderer/platform/graphics/paint/scrollbar_display_item.cc
@@ -110,8 +110,8 @@ scoped_refptr<cc::ScrollbarLayerBase> ScrollbarDisplayItem::CreateOrReuseLayer(
 
 bool ScrollbarDisplayItem::IsOpaque() const {
   DCHECK(!IsTombstone());
-  // The native themes should ensure opaqueness of non-overlay scrollbars.
-  return !data_->scrollbar_->IsOverlay();
+
+  return data_->scrollbar_->IsOpaque();
 }
 
 bool ScrollbarDisplayItem::EqualsForUnderInvalidationImpl(

diff --git a/third_party/blink/web_tests/TestExpectations b/third_party/blink/web_tests/TestExpectations
@@ -2995,7 +2995,6 @@ crbug.com/626703 [ Mac ] external/wpt/html/canvas/offscreen/text/2d.text.fontVar
 crbug.com/626703 [ Linux ] external/wpt/html/canvas/offscreen/text/2d.text.fontVariantCaps3.html [ Failure ]
 crbug.com/626703 [ Mac ] external/wpt/html/canvas/offscreen/text/2d.text.fontVariantCaps4.html [ Failure ]
 crbug.com/626703 [ Linux ] external/wpt/html/canvas/offscreen/text/2d.text.fontVariantCaps4.html [ Failure ]
-crbug.com/626703 external/wpt/css/css-overflow/scrollbar-gutter-dynamic-003.html [ Failure ]
 crbug.com/626703 [ Mac12 ] virtual/threaded-prefer-compositing/external/wpt/scroll-animations/view-timelines/view-timeline-inset.html [ Timeout ]
 crbug.com/626703 [ Mac13 ] virtual/threaded-prefer-compositing/external/wpt/scroll-animations/view-timelines/view-timeline-inset.html [ Timeout ]
 crbug.com/626703 [ Mac12 ] virtual/threaded-prefer-compositing/external/wpt/scroll-animations/scroll-timelines/effect-updateTiming.html [ Timeout ]

diff --git a/...party/blink/web_tests/external/wpt/css/css-overflow/scrollbar-gutter-dynamic-003.html.ini b/...party/blink/web_tests/external/wpt/css/css-overflow/scrollbar-gutter-dynamic-003.html.ini
diff --git a/third_party/blink/web_tests/external/wpt/css/css-scrollbars/scrollbar-color-012-ref.html b/third_party/blink/web_tests/external/wpt/css/css-scrollbars/scrollbar-color-012-ref.html
@@ -0,0 +1,13 @@
+<!doctype html>
+<html>
+<style>
+  .container {
+    overflow: auto;
+    height: 200px;
+    width: 200px;
+    background-color: red;
+  }
+</style>
+<div class="container">
+  <div class="content"></div>
+</div>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-scrollbars/scrollbar-color-012.html b/third_party/blink/web_tests/external/wpt/css/css-scrollbars/scrollbar-color-012.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html>
+<title>scrollbar-color with transparent colors works</title>
+<link rel="author" title="Luke Warlow" href="mailto:luke@warlow.dev" />
+<link rel="match" href="scrollbar-color-012-ref.html" />
+<link rel="help" href="https://drafts.csswg.org/css-scrollbars" />
+<style>
+  html {scrollbar-color: transparent transparent;}
+  body {height: 200vh}
+  .container {
+    overflow: auto;
+    height: 200px;
+    width: 200px;
+    background-color: red;
+  }
+
+  .content {
+    height: 300px;
+    width: 300px;
+  }
+</style>
+<div class="container">
+  <div class="content"></div>
+</div>