Modify ComputeStyle of <select>'s UA shadow outside style recalc

We attempted to remove SetModifiedStyleOutsideStyleRecalc in crrev.com/c/3620574, but for now it's still needed for <select>'s UA shadow. (cherry picked from commit 557c06b) Bug: 1329417 Change-Id: Ia7051b66ed314d0660e526f751d27f6bcc96dfa2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3669113 Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1008252} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3678266 Auto-Submit: Anders Hartvoll Ruud <andruud@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/branch-heads/5060@{#403} Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
chromium · May 31, 2022 · fd91880 · fd91880
1 parent ad9bee0
commit fd91880
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 7 deletions.
diff --git a/third_party/blink/renderer/core/html/forms/select_type.cc b/third_party/blink/renderer/core/html/forms/select_type.cc
@@ -498,8 +498,8 @@ String MenuListSelectType::UpdateTextStyleInternal() {
     cloned_style->SetUnicodeBidi(option_style->GetUnicodeBidi());
     cloned_style->SetTextAlign(option_style->GetTextAlign(true));
     if (auto* inner_layout = inner_element.GetLayoutObject()) {
-      inner_layout->SetStyle(std::move(cloned_style),
-                             LayoutObject::ApplyStyleChanges::kYes);
+      inner_layout->SetModifiedStyleOutsideStyleRecalc(
+          std::move(cloned_style), LayoutObject::ApplyStyleChanges::kYes);
     } else {
       inner_element.SetComputedStyle(std::move(cloned_style));
     }

diff --git a/third_party/blink/renderer/core/layout/layout_object.cc b/third_party/blink/renderer/core/layout/layout_object.cc
@@ -4753,7 +4753,7 @@ Vector<PhysicalRect> LayoutObject::OutlineRects(
   return outline_rects;
 }
 
-void LayoutObject::SetTextAutoSizedStyle(
+void LayoutObject::SetModifiedStyleOutsideStyleRecalc(
     scoped_refptr<const ComputedStyle> style,
     ApplyStyleChanges apply_changes) {
   NOT_DESTROYED();

diff --git a/third_party/blink/renderer/core/layout/layout_object.h b/third_party/blink/renderer/core/layout/layout_object.h
@@ -2327,8 +2327,15 @@ class CORE_EXPORT LayoutObject : public GarbageCollected<LayoutObject>,
   void SetPseudoElementStyle(scoped_refptr<const ComputedStyle>,
                              bool match_parent_size = false);
 
-  void SetTextAutoSizedStyle(scoped_refptr<const ComputedStyle>,
-                             ApplyStyleChanges);
+  // In some cases we modify the ComputedStyle after the style recalc, either
+  // for updating anonymous style or doing layout hacks for special elements
+  // where we update the ComputedStyle during layout.
+  // If the LayoutObject has an associated node, we will SetComputedStyle on
+  // that node with the new ComputedStyle. Modifying the ComputedStyle of a node
+  // outside of style recalc can break invariants in the style engine, so this
+  // function must not gain any new call sites.
+  void SetModifiedStyleOutsideStyleRecalc(scoped_refptr<const ComputedStyle>,
+                                          ApplyStyleChanges);
 
   // This function returns an enclosing non-anonymous LayoutBlock for this
   // element. This function is not always returning the containing block as

diff --git a/third_party/blink/renderer/core/layout/text_autosizer.cc b/third_party/blink/renderer/core/layout/text_autosizer.cc
@@ -1258,7 +1258,7 @@ void TextAutosizer::ApplyMultiplier(LayoutObject* layout_object,
       else
         DCHECK(false);
 
-      layout_object->SetTextAutoSizedStyle(
+      layout_object->SetModifiedStyleOutsideStyleRecalc(
           std::move(style), LayoutObject::ApplyStyleChanges::kNo);
       if (layout_object->IsText())
         To<LayoutText>(layout_object)->AutosizingMultiplerChanged();
@@ -1270,7 +1270,7 @@ void TextAutosizer::ApplyMultiplier(LayoutObject* layout_object,
 
     case kLayoutNeeded:
       DCHECK(!layouter);
-      layout_object->SetTextAutoSizedStyle(
+      layout_object->SetModifiedStyleOutsideStyleRecalc(
           std::move(style), LayoutObject::ApplyStyleChanges::kYes);
       break;
   }

diff --git a/...nk/web_tests/external/wpt/css/css-writing-modes/crashtests/unicode-bidi-select-crash.html b/...nk/web_tests/external/wpt/css/css-writing-modes/crashtests/unicode-bidi-select-crash.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<link rel="help" href="https://crbug.com/1329417">
+<style>
+  select {
+    unicode-bidi: bidi-override;
+  }
+</style>
+<body>
+  <select>
+    <option>Test1</option>
+    <option id="opt" hidden="hidden">Test2</option>
+  </select>
+</body>
+<script>
+  document.body.offsetTop;
+  opt.defaultSelected = true;
+  window.getSelection().selectAllChildren(document.body);
+  document.vlinkColor = "red";
+</script>