Don't treat color fixed-attachment background as fixed

Background attachment doesn't apply to color backgrounds. We should
always treat them as default attachment.

The added condition is like how we check if there is effective
fixed-attachment background elsewhere using

The changed code is special because we check background-attachment
of individual fill layer.

FillLayer: :AnyLayerHasFixedAttachmentImage().
Change-Id: I877bf7e3cb1a9e23f463d53b64acb507720d37e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1868839Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707544}

third_party/blink/renderer/core/paint/background_image_geometry.cc

@@ -373,6 +373,13 @@ LayoutSize BackgroundImageGeometry::GetBackgroundObjectDimensions(
   return LayoutSize(width, column_height);
 }
 
+bool BackgroundImageGeometry::ShouldUseFixedAttachment(
+    const FillLayer& fill_layer) {
+  // Solid color background should use default attachment.
+  return fill_layer.GetImage() &&
+         fill_layer.Attachment() == EFillAttachment::kFixed;
+}
+
 namespace {
 
 LayoutRect FixedAttachmentPositioningArea(const LayoutBoxModelObject& obj,
@@ -642,7 +649,7 @@ void BackgroundImageGeometry::ComputePositioningArea(
     LayoutRect& snapped_positioning_area,
     LayoutPoint& unsnapped_box_offset,
     LayoutPoint& snapped_box_offset) {
-  if (fill_layer.Attachment() == EFillAttachment::kFixed) {
+  if (ShouldUseFixedAttachment(fill_layer)) {
     // No snapping for fixed attachment.
     SetHasNonLocalGeometry();
     offset_in_background_ = LayoutPoint();
@@ -1029,7 +1036,7 @@ void BackgroundImageGeometry::Calculate(const LayoutBoxModelObject* container,
       unsnapped_dest_rect_ = snapped_dest_rect_ = LayoutRect();
   }
 
-  if (fill_layer.Attachment() == EFillAttachment::kFixed)
+  if (ShouldUseFixedAttachment(fill_layer))
     UseFixedAttachment(paint_rect.Location());
 
   // Clip the final output rect to the paint rect, maintaining snapping.

third_party/blink/renderer/core/paint/background_image_geometry.h

@@ -95,6 +95,8 @@ class BackgroundImageGeometry {
   const ComputedStyle& ImageStyle() const;
   InterpolationQuality ImageInterpolationQuality() const;
 
+  static bool ShouldUseFixedAttachment(const FillLayer&);
+
  private:
   void SetSpaceSize(const LayoutSize& repeat_spacing) {
     repeat_spacing_ = repeat_spacing;

third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc

@@ -958,6 +958,59 @@ TEST_P(PaintLayerScrollableAreaTest, ViewScrollWithFixedAttachmentBackground) {
   EXPECT_TRUE(GetLayoutView().NeedsPaintPropertyUpdate());
 }
 
+TEST_P(PaintLayerScrollableAreaTest,
+       ViewScrollWithSolidColorFixedAttachmentBackground) {
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      html, #fixed-background {
+        background: green fixed;
+      }
+      #fixed-background {
+        width: 200px;
+        height: 200px;
+        overflow: scroll;
+      }
+    </style>
+    <div id="fixed-background">
+      <div style="height: 3000px"></div>
+    </div>
+    <div style="height: 3000px"></div>
+  )HTML");
+
+  // Fixed-attachment solid-color background should be treated as default
+  // attachment.
+  EXPECT_EQ(kBackgroundPaintInScrollingContents,
+            GetLayoutView().GetBackgroundPaintLocation());
+  auto* fixed_background_div =
+      ToLayoutBox(GetLayoutObjectByElementId("fixed-background"));
+  EXPECT_EQ(kBackgroundPaintInScrollingContents,
+            fixed_background_div->GetBackgroundPaintLocation());
+  auto* div_scrollable_area = fixed_background_div->GetScrollableArea();
+  auto* view_scrollable_area = GetLayoutView().GetScrollableArea();
+
+  // Programmatically changing the view's scroll offset. Should invalidate all
+  // objects with fixed attachment background.
+  view_scrollable_area->SetScrollOffset(ScrollOffset(0, 1),
+                                        kProgrammaticScroll);
+  EXPECT_FALSE(fixed_background_div->ShouldDoFullPaintInvalidation());
+  EXPECT_FALSE(fixed_background_div->BackgroundNeedsFullPaintInvalidation());
+  EXPECT_FALSE(fixed_background_div->NeedsPaintPropertyUpdate());
+  EXPECT_FALSE(GetLayoutView().ShouldDoFullPaintInvalidation());
+  EXPECT_FALSE(GetLayoutView().BackgroundNeedsFullPaintInvalidation());
+  EXPECT_TRUE(GetLayoutView().NeedsPaintPropertyUpdate());
+  UpdateAllLifecyclePhasesForTest();
+
+  // Programmatically changing the div's scroll offset. Should invalidate the
+  // scrolled div with fixed attachment background.
+  div_scrollable_area->SetScrollOffset(ScrollOffset(0, 1), kProgrammaticScroll);
+  EXPECT_FALSE(fixed_background_div->ShouldDoFullPaintInvalidation());
+  EXPECT_FALSE(fixed_background_div->BackgroundNeedsFullPaintInvalidation());
+  EXPECT_TRUE(fixed_background_div->NeedsPaintPropertyUpdate());
+  EXPECT_FALSE(GetLayoutView().ShouldDoFullPaintInvalidation());
+  EXPECT_FALSE(GetLayoutView().BackgroundNeedsFullPaintInvalidation());
+  EXPECT_TRUE(GetLayoutView().NeedsPaintPropertyUpdate());
+}
+
 TEST_P(PaintLayerScrollableAreaTest,
        ViewScrollWithFixedAttachmentBackgroundPreferCompositingToLCDText) {
   GetDocument().GetFrame()->GetSettings()->SetPreferCompositingToLCDTextEnabled(

third_party/blink/renderer/core/paint/view_painter.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/paint/view_painter.h"
 
+#include "base/containers/adapters.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/frame/settings.h"
 #include "third_party/blink/renderer/core/layout/layout_box.h"
@@ -270,14 +271,11 @@ void ViewPainter::PaintBoxDecorationBackgroundInternal(
 
   BackgroundImageGeometry geometry(layout_view_);
   BoxModelObjectPainter box_model_painter(layout_view_);
-  for (auto it = reversed_paint_list.rbegin(); it != reversed_paint_list.rend();
-       ++it) {
-    DCHECK((*it)->Clip() == EFillBox::kBorder);
-
-    bool should_paint_in_viewport_space =
-        (*it)->Attachment() == EFillAttachment::kFixed;
-    if (should_paint_in_viewport_space) {
-      box_model_painter.PaintFillLayer(paint_info, Color(), **it,
+  for (const auto* fill_layer : base::Reversed(reversed_paint_list)) {
+    DCHECK(fill_layer->Clip() == EFillBox::kBorder);
+
+    if (BackgroundImageGeometry::ShouldUseFixedAttachment(*fill_layer)) {
+      box_model_painter.PaintFillLayer(paint_info, Color(), *fill_layer,
                                        PhysicalRect(background_rect),
                                        kBackgroundBleedNone, geometry);
     } else {
@@ -285,7 +283,7 @@ void ViewPainter::PaintBoxDecorationBackgroundInternal(
       // TODO(trchen): We should be able to handle 3D-transformed root
       // background with slimming paint by using transform display items.
       context.ConcatCTM(transform.ToAffineTransform());
-      box_model_painter.PaintFillLayer(paint_info, Color(), **it,
+      box_model_painter.PaintFillLayer(paint_info, Color(), *fill_layer,
                                        PhysicalRect(paint_rect),
                                        kBackgroundBleedNone, geometry);
       context.Restore();

third_party/blink/web_tests/flag-specific/enable-blink-features=CompositeAfterPaint/paint/invalidation/background/no-repaint-for-solid-color-background-attachment-fixed-expected.txt

+{
+  "layers": [
+    {
+      "name": "Scrolling background of LayoutView #document",
+      "bounds": [800, 5016],
+      "contentsOpaque": true,
+      "backgroundColor": "#0000FF",
+      "transform": 1
+    }
+  ],
+  "transforms": [
+    {
+      "id": 1,
+      "transform": [
+        [1, 0, 0, 0],
+        [0, 1, 0, 0],
+        [0, 0, 1, 0],
+        [0, -1000, 0, 1]
+      ]
+    }
+  ]
+}
+

third_party/blink/web_tests/paint/invalidation/background/no-repaint-for-solid-color-background-attachment-fixed-expected.html

+<!DOCTYPE html>
+<body style="background: blue"></body>

third_party/blink/web_tests/paint/invalidation/background/no-repaint-for-solid-color-background-attachment-fixed-expected.txt

+{
+  "layers": [
+    {
+      "name": "Scrolling Contents Layer",
+      "bounds": [800, 5016],
+      "contentsOpaque": true,
+      "backgroundColor": "#0000FF",
+      "transform": 1
+    },
+    {
+      "name": "Vertical Scrollbar Layer",
+      "position": [800, 0],
+      "bounds": [0, 600]
+    }
+  ],
+  "transforms": [
+    {
+      "id": 1,
+      "transform": [
+        [1, 0, 0, 0],
+        [0, 1, 0, 0],
+        [0, 0, 1, 0],
+        [0, -1000, 0, 1]
+      ]
+    }
+  ]
+}
+

third_party/blink/web_tests/paint/invalidation/background/no-repaint-for-solid-color-background-attachment-fixed.html

+<!DOCTYPE html>
+<style>
+::-webkit-scrollbar { display: none; }
+body {
+    background: blue;
+    background-attachment: fixed;
+}
+</style>
+<script src="../resources/text-based-repaint.js" type="text/javascript"></script>
+<script>
+if (window.testRunner)
+    internals.settings.setPreferCompositingToLCDTextEnabled(false);
+
+function repaintTest() {
+    window.scrollTo(0, 1000);
+}
+
+window.onload = function() {
+    runRepaintAndPixelTest();
+};
+</script>
+<div style="height: 5000px">
+Tests that scrolling a frame with background-attachment: fixed and solid color background should not invalidate.
+</div>