Fix issues with visual overflow in inline elements for legacy layout

In r727590 there was introduced a regression regarding visual overflow
of inline elements in legacy layout.
The problem is that inline elements in legacy layout don't recompute
the visual overflow, so we still need to mark the element
for layout in that case.

The fix is basically bringing back the code in LayoutObject::SetStyle()
to mark inline elements for layout, including an extra condition
for doing it only for legacy layout objects.

BUG=1043927
TEST=ParameterizedLayoutInlineTest.VisualOverflowRecalcLegacyLayout

Change-Id: I4c5dff4c75c3a9484a52f51fbd85f2b65672de1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2025389Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Manuel Rego <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#736422}

third_party/blink/renderer/core/layout/layout_inline_test.cc

@@ -4,6 +4,7 @@
 
 #include "third_party/blink/renderer/core/layout/layout_inline.h"
 
+#include "build/build_config.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/layout/layout_block_flow.h"
@@ -676,4 +677,94 @@ TEST_P(ParameterizedLayoutInlineTest, AddAnnotatedRegionsVerticalRL) {
   EXPECT_TRUE(regions3.IsEmpty());
 }
 
+TEST_P(ParameterizedLayoutInlineTest, VisualOverflowRecalcLegacyLayout) {
+  // "contenteditable" forces us to use legacy layout, other options could be
+  // using "display: -webkit-box", ruby, etc.
+  LoadAhem();
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      body {
+        margin: 0;
+        font: 20px/20px Ahem;
+      }
+      target {
+        outline: 50px solid red;
+      }
+    </style>
+    <div contenteditable>
+      <span id="span">SPAN1</span>
+      <span id="span2">SPAN2</span>
+    </div>
+  )HTML");
+
+  auto* span = ToLayoutInline(GetLayoutObjectByElementId("span"));
+  auto* span_element = GetDocument().getElementById("span");
+  auto* span2_element = GetDocument().getElementById("span2");
+
+  span_element->setAttribute(html_names::kStyleAttr, "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(-50, -50, 200, 120),
+            span->PhysicalVisualOverflowRect());
+
+  span_element->setAttribute(html_names::kStyleAttr, "");
+  span2_element->setAttribute(html_names::kStyleAttr,
+                              "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(0, 0, 100, 20), span->PhysicalVisualOverflowRect());
+
+  span2_element->setAttribute(html_names::kStyleAttr, "");
+  span_element->setAttribute(html_names::kStyleAttr, "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(-50, -50, 200, 120),
+            span->PhysicalVisualOverflowRect());
+
+  span_element->setAttribute(html_names::kStyleAttr, "");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(0, 0, 100, 20), span->PhysicalVisualOverflowRect());
+}
+
+TEST_P(ParameterizedLayoutInlineTest, VisualOverflowRecalcLayoutNG) {
+  LoadAhem();
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      body {
+        margin: 0;
+        font: 20px/20px Ahem;
+      }
+      target {
+        outline: 50px solid red;
+      }
+    </style>
+    <div>
+      <span id="span">SPAN1</span>
+      <span id="span2">SPAN2</span>
+    </div>
+  )HTML");
+
+  auto* span = ToLayoutInline(GetLayoutObjectByElementId("span"));
+  auto* span_element = GetDocument().getElementById("span");
+  auto* span2_element = GetDocument().getElementById("span2");
+
+  span_element->setAttribute(html_names::kStyleAttr, "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(-50, -50, 200, 120),
+            span->PhysicalVisualOverflowRect());
+
+  span_element->setAttribute(html_names::kStyleAttr, "");
+  span2_element->setAttribute(html_names::kStyleAttr,
+                              "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(0, 0, 100, 20), span->PhysicalVisualOverflowRect());
+
+  span2_element->setAttribute(html_names::kStyleAttr, "");
+  span_element->setAttribute(html_names::kStyleAttr, "outline: 50px solid red");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(-50, -50, 200, 120),
+            span->PhysicalVisualOverflowRect());
+
+  span_element->setAttribute(html_names::kStyleAttr, "");
+  UpdateAllLifecyclePhasesForTest();
+  EXPECT_EQ(PhysicalRect(0, 0, 100, 20), span->PhysicalVisualOverflowRect());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/layout/layout_object.cc

@@ -2106,8 +2106,19 @@ void LayoutObject::SetStyle(scoped_refptr<const ComputedStyle> style,
   }
 
   if (diff.NeedsRecomputeVisualOverflow()) {
-    PaintingLayer()->SetNeedsVisualOverflowRecalc();
-    SetShouldCheckForPaintInvalidation();
+    if (IsInline() && !IsInLayoutNGInlineFormattingContext() &&
+        !NeedsLayout()) {
+      // TODO(rego): This is still needed because RecalcVisualOverflow() does
+      // not actually compute the visual overflow for inline elements (legacy
+      // layout). However in LayoutNG RecalcInlineChildrenInkOverflow() is
+      // called and visual overflow is recomputed properly so we don't need this
+      // (see crbug.com/1043927).
+      SetNeedsLayoutAndPrefWidthsRecalc(
+          layout_invalidation_reason::kStyleChange);
+    } else {
+      PaintingLayer()->SetNeedsVisualOverflowRecalc();
+      SetShouldCheckForPaintInvalidation();
+    }
   }
 
   if (diff.NeedsPaintInvalidationSubtree() ||

third_party/blink/web_tests/flag-specific/disable-layout-ng/paint/invalidation/outline/outline-containing-image-in-non-standard-mode-expected.txt

@@ -10,6 +10,11 @@
           "object": "LayoutInline SPAN id='target'",
           "rect": [6, 6, 204, 58],
           "reason": "style change"
+        },
+        {
+          "object": "LayoutImage IMG",
+          "rect": [8, 8, 200, 50],
+          "reason": "geometry"
         }
       ]
     }