Invalidate ink overflow on text decoration changes

Found while adding ink overflow for text decorations,
we currently do not invalidate ink overflow on any
text decoration style changes (presumably because
we never previously computed overflow for them).

Here we add the necessary predicate function and
update the json template to invoke the predicate
on the appropriate style changes.

Tested with a unit test to confirm the style difference
computation sets the necessary flag.

Bug: 896295
Change-Id: I3e3e697c70a21ae66da7288c0b0a5d9826593a29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2411300Reviewed-by: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807433}

third_party/blink/renderer/core/css/resolver/style_adjuster.h

@@ -23,6 +23,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RESOLVER_STYLE_ADJUSTER_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_CSS_RESOLVER_STYLE_ADJUSTER_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
 
 namespace blink {
@@ -38,7 +39,7 @@ class StyleAdjuster {
   STATIC_ONLY(StyleAdjuster);
 
  public:
-  static void AdjustComputedStyle(StyleResolverState&, Element*);
+  CORE_EXPORT static void AdjustComputedStyle(StyleResolverState&, Element*);
   static void AdjustStyleForEditing(ComputedStyle&);
 
  private:

third_party/blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc

@@ -599,11 +599,11 @@ struct StyleChangeData {
   unsigned needs_collect_inlines;
   base::Optional<bool> is_line_dirty;
 } style_change_data[] = {
-    // Changing color, text-decoration, etc. should not re-run
+    // Changing color, etc. should not re-run
     // |CollectInlines()|.
     {"#parent.after { color: red; }", StyleChangeData::kNone, false},
-    {"#parent.after { text-decoration-line: underline; }",
-     StyleChangeData::kNone, false},
+    {"#parent.after { text-decoration-color: red; }", StyleChangeData::kNone,
+     false},
     // Changing fonts should re-run |CollectInlines()|.
     {"#parent.after { font-size: 200%; }", StyleChangeData::kAll, true},
     // Changing from/to out-of-flow should re-rerun |CollectInlines()|.

third_party/blink/renderer/core/style/computed_style.cc

@@ -1711,6 +1711,33 @@ void ComputedStyle::UpdateFontOrientation() {
   SetFontDescription(font_description);
 }
 
+bool ComputedStyle::TextDecorationVisualOverflowEqual(
+    const ComputedStyle& o) const {
+  const Vector<AppliedTextDecoration>& applied_with_this =
+      AppliedTextDecorations();
+  const Vector<AppliedTextDecoration>& applied_with_other =
+      o.AppliedTextDecorations();
+  if (applied_with_this.size() != applied_with_other.size())
+    return false;
+  for (auto decoration_index = 0u; decoration_index < applied_with_this.size();
+       ++decoration_index) {
+    const AppliedTextDecoration& decoration_from_this =
+        applied_with_this[decoration_index];
+    const AppliedTextDecoration& decoration_from_other =
+        applied_with_other[decoration_index];
+    if (decoration_from_this.Thickness() != decoration_from_other.Thickness() ||
+        decoration_from_this.UnderlineOffset() !=
+            decoration_from_other.UnderlineOffset() ||
+        decoration_from_this.Style() != decoration_from_other.Style() ||
+        decoration_from_this.Lines() != decoration_from_other.Lines())
+      return false;
+  }
+  if (TextUnderlinePosition() != o.TextUnderlinePosition())
+    return false;
+
+  return true;
+}
+
 TextDecoration ComputedStyle::TextDecorationsInEffect() const {
   if (HasSimpleUnderlineInternal())
     return TextDecoration::kUnderline;

third_party/blink/renderer/core/style/computed_style.h

@@ -2134,13 +2134,14 @@ class ComputedStyle : public ComputedStyleBase,
   }
 
   // Text decoration utility functions.
+  bool TextDecorationVisualOverflowEqual(const ComputedStyle& o) const;
   void ApplyTextDecorations(const Color& parent_text_decoration_color,
                             bool override_existing_colors);
   void ClearAppliedTextDecorations();
   void RestoreParentTextDecorations(const ComputedStyle& parent_style);
   CORE_EXPORT const Vector<AppliedTextDecoration>& AppliedTextDecorations()
       const;
-  TextDecoration TextDecorationsInEffect() const;
+  CORE_EXPORT TextDecoration TextDecorationsInEffect() const;
 
   // Overflow utility functions.
 
@@ -2939,6 +2940,11 @@ class ComputedStyle : public ComputedStyleBase,
   FRIEND_TEST_ALL_PREFIXES(ComputedStyleTest,
                            InitialAndInheritedAndNonInheritedVariableNames);
   FRIEND_TEST_ALL_PREFIXES(StyleCascadeTest, ForcedVisitedBackgroundColor);
+  FRIEND_TEST_ALL_PREFIXES(
+      ComputedStyleTest,
+      TextDecorationEqualDoesNotRequireRecomputeInkOverflow);
+  FRIEND_TEST_ALL_PREFIXES(ComputedStyleTest,
+                           TextDecorationNotEqualRequiresRecomputeInkOverflow);
 };
 
 inline bool ComputedStyle::HasAnyPseudoElementStyles() const {

third_party/blink/renderer/core/style/computed_style_diff_functions.json5

@@ -396,6 +396,12 @@
             predicate: "a.BorderVisualOverflowEqual(b)",
             field_dependencies: ["border-image"]
           },
+          {
+            predicate: "a.TextDecorationVisualOverflowEqual(b)",
+            field_dependencies: ["text-decoration-thickness", "text-decoration-style",
+                                 "text-decoration-line", "text-underline-offset", "text-underline-position",
+                                 "AppliedTextDecorations"]
+          },
         ]
     },
     {
@@ -411,6 +417,7 @@
         name: "UpdatePropertySpecificDifferencesTextDecorationOrColor",
         fields_to_diff: ["color", "-internal-visited-color", "text-decoration-line",
                 "text-decoration-style", "text-decoration-color",
+                "text-decoration-thickness", "text-underline-offset",
                 "-internal-visited-text-decoration-color", "TextEmphasisFill",
                 "text-underline-position", "text-decoration-skip-ink", "AppliedTextDecorations"],
         methods_to_diff: [

third_party/blink/renderer/core/style/computed_style_test.cc

@@ -17,6 +17,7 @@
 #include "third_party/blink/renderer/core/css/parser/css_parser.h"
 #include "third_party/blink/renderer/core/css/properties/css_property_ref.h"
 #include "third_party/blink/renderer/core/css/property_registry.h"
+#include "third_party/blink/renderer/core/css/resolver/style_adjuster.h"
 #include "third_party/blink/renderer/core/css/resolver/style_cascade.h"
 #include "third_party/blink/renderer/core/css/resolver/style_resolver_state.h"
 #include "third_party/blink/renderer/core/css/style_engine.h"
@@ -947,4 +948,114 @@ TEST(ComputedStyleTest, BorderWidthZoom) {
   }
 }
 
+TEST(ComputedStyleTest, TextDecorationEqualDoesNotRequireRecomputeInkOverflow) {
+  using css_test_helpers::ParseDeclarationBlock;
+
+  std::unique_ptr<DummyPageHolder> dummy_page_holder_ =
+      std::make_unique<DummyPageHolder>(IntSize(0, 0), nullptr);
+  const ComputedStyle* initial = &ComputedStyle::InitialStyle();
+
+  StyleResolverState state(dummy_page_holder_->GetDocument(),
+                           *dummy_page_holder_->GetDocument().documentElement(),
+                           initial, initial);
+
+  scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
+
+  // Set up the initial text decoration properties
+  style->SetTextDecorationStyle(ETextDecorationStyle::kSolid);
+  style->SetTextDecorationColor(StyleColor(CSSValueID::kGreen));
+  style->SetTextDecoration(TextDecoration::kUnderline);
+  style->SetTextDecorationThickness(
+      TextDecorationThickness(Length(5, Length::Type::kFixed)));
+  style->SetTextUnderlineOffset(Length(2, Length::Type::kFixed));
+  style->SetTextUnderlinePosition(kTextUnderlinePositionUnder);
+  state.SetStyle(style);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  EXPECT_EQ(TextDecoration::kUnderline, style->TextDecorationsInEffect());
+
+  scoped_refptr<ComputedStyle> other = ComputedStyle::Clone(*style);
+  StyleDifference diff1;
+  style->UpdatePropertySpecificDifferences(*other, diff1);
+  EXPECT_FALSE(diff1.NeedsRecomputeVisualOverflow());
+
+  // Change the color, and it should not invalidate
+  other->SetTextDecorationColor(StyleColor(CSSValueID::kBlue));
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff2;
+  style->UpdatePropertySpecificDifferences(*other, diff2);
+  EXPECT_FALSE(diff2.NeedsRecomputeVisualOverflow());
+}
+
+TEST(ComputedStyleTest, TextDecorationNotEqualRequiresRecomputeInkOverflow) {
+  using css_test_helpers::ParseDeclarationBlock;
+
+  std::unique_ptr<DummyPageHolder> dummy_page_holder_ =
+      std::make_unique<DummyPageHolder>(IntSize(0, 0), nullptr);
+  const ComputedStyle* initial = &ComputedStyle::InitialStyle();
+
+  StyleResolverState state(dummy_page_holder_->GetDocument(),
+                           *dummy_page_holder_->GetDocument().documentElement(),
+                           initial, initial);
+
+  scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
+
+  // Set up the initial text decoration properties
+  style->SetTextDecorationStyle(ETextDecorationStyle::kSolid);
+  style->SetTextDecorationColor(StyleColor(CSSValueID::kGreen));
+  style->SetTextDecoration(TextDecoration::kUnderline);
+  style->SetTextDecorationThickness(
+      TextDecorationThickness(Length(5, Length::Type::kFixed)));
+  style->SetTextUnderlineOffset(Length(2, Length::Type::kFixed));
+  style->SetTextUnderlinePosition(kTextUnderlinePositionUnder);
+  state.SetStyle(style);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+
+  // Change decoration style
+  scoped_refptr<ComputedStyle> other = ComputedStyle::Clone(*style);
+  other->SetTextDecorationStyle(ETextDecorationStyle::kWavy);
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff_decoration_style;
+  style->UpdatePropertySpecificDifferences(*other, diff_decoration_style);
+  EXPECT_TRUE(diff_decoration_style.NeedsRecomputeVisualOverflow());
+
+  // Change decoration line
+  other = ComputedStyle::Clone(*style);
+  other->SetTextDecoration(TextDecoration::kOverline);
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff_decoration_line;
+  style->UpdatePropertySpecificDifferences(*other, diff_decoration_line);
+  EXPECT_TRUE(diff_decoration_line.NeedsRecomputeVisualOverflow());
+
+  // Change decoration thickness
+  other = ComputedStyle::Clone(*style);
+  other->SetTextDecorationThickness(
+      TextDecorationThickness(Length(3, Length::Type::kFixed)));
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff_decoration_thickness;
+  style->UpdatePropertySpecificDifferences(*other, diff_decoration_thickness);
+  EXPECT_TRUE(diff_decoration_thickness.NeedsRecomputeVisualOverflow());
+
+  // Change underline offset
+  other = ComputedStyle::Clone(*style);
+  other->SetTextUnderlineOffset(Length(4, Length::Type::kFixed));
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff_underline_offset;
+  style->UpdatePropertySpecificDifferences(*other, diff_underline_offset);
+  EXPECT_TRUE(diff_underline_offset.NeedsRecomputeVisualOverflow());
+
+  // Change underline position
+  other = ComputedStyle::Clone(*style);
+  other->SetTextUnderlinePosition(kTextUnderlinePositionLeft);
+  state.SetStyle(other);
+  StyleAdjuster::AdjustComputedStyle(state, nullptr /* element */);
+  StyleDifference diff_underline_position;
+  style->UpdatePropertySpecificDifferences(*other, diff_underline_position);
+  EXPECT_TRUE(diff_underline_position.NeedsRecomputeVisualOverflow());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/style/style_difference.h

@@ -27,6 +27,7 @@ class StyleDifference {
     kCSSClipChanged = 1 << 5,
     // The object needs to issue paint invalidations if it is affected by text
     // decorations or properties dependent on color (e.g., border or outline).
+    // TextDecorationis changes must also invalidate ink overflow.
     kTextDecorationOrColorChanged = 1 << 6,
     kBlendModeChanged = 1 << 7,
     kMaskChanged = 1 << 8,

third_party/blink/renderer/core/style/text_decoration_thickness.h

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_CORE_STYLE_TEXT_DECORATION_THICKNESS_H_
 #define THIRD_PARTY_BLINK_RENDERER_CORE_STYLE_TEXT_DECORATION_THICKNESS_H_
 
+#include "third_party/blink/renderer/core/core_export.h"
 #include "third_party/blink/renderer/core/css_value_keywords.h"
 #include "third_party/blink/renderer/platform/geometry/length.h"
 
@@ -16,7 +17,7 @@ class TextDecorationThickness {
  public:
   TextDecorationThickness();
 
-  explicit TextDecorationThickness(const Length& length);
+  CORE_EXPORT explicit TextDecorationThickness(const Length& length);
 
   explicit TextDecorationThickness(CSSValueID from_font_keyword);
 
@@ -27,7 +28,10 @@ class TextDecorationThickness {
   }
   bool IsAuto() const { return !thickness_from_font_ && thickness_.IsAuto(); }
 
-  bool operator==(const TextDecorationThickness&) const;
+  CORE_EXPORT bool operator==(const TextDecorationThickness&) const;
+  bool operator!=(const TextDecorationThickness& other) const {
+    return !(*this == other);
+  }
 
  private:
   Length thickness_;

third_party/blink/web_tests/flag-specific/disable-layout-ng/paint/invalidation/non-text-link-invalidation-optimization-expected.txt

+{
+  "layers": [
+    {
+      "name": "Scrolling Contents Layer",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "backgroundColor": "#FFFFFF",
+      "invalidations": [
+        [164, 8, 145, 19],
+        [75, 8, 61, 19],
+        [8, 8, 57, 19],
+        [353, 8, 52, 19],
+        [136, 8, 28, 19],
+        [319, 8, 24, 19],
+        [343, 13, 10, 10],
+        [309, 13, 10, 10],
+        [65, 13, 10, 10]
+      ]
+    }
+  ]
+}
+