Skip to content

Commit

Permalink
chore: [20-x-y] cherry-pick 06c87f9f42ff from chromium
Browse files Browse the repository at this point in the history
  • Loading branch information
ppontes committed Nov 2, 2022
1 parent 98ab127 commit 2282b17
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -134,3 +134,4 @@ cherry-pick-65f0ef609c00.patch
cherry-pick-cb9dff93f3d4.patch
build_fix_building_with_enable_plugins_false.patch
build_allow_electron_to_use_exec_script.patch
cherry-pick-06c87f9f42ff.patch
153 changes: 153 additions & 0 deletions patches/chromium/cherry-pick-06c87f9f42ff.patch
@@ -0,0 +1,153 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Rune Lillesveen <futhark@chromium.org>
Date: Fri, 14 Oct 2022 09:52:34 +0000
Subject: Avoid layout roots in subtrees skipped for style recalc

Layout roots are laid out from inner to outer in LocalFrameView. DOM
mutations may have added layout roots inside size container subtrees
before style recalc. If we decide to postpone style recalc until layout
of the size container, it means we may try to layout a root inside a
subtree skipped for style recalc. That causes a DCHECK and possibly
other issues.

This also fixes the use-after-poison issue 1365330.

(cherry picked from commit 0f0f1e99201fadb3c68518350e1cd6af1b665346)

Bug: 1371820, 1365330
Change-Id: Ia48890c08aacfe7b9a3e660817702abce0570564
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3934847
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1055853}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3953455
Auto-Submit: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/branch-heads/5249@{#836}
Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826}

diff --git a/third_party/blink/renderer/core/css/style_engine.cc b/third_party/blink/renderer/core/css/style_engine.cc
index f51b8878eb0abe23889a07277efeaf7cdf961cc8..ddcef20b47357e58acae08b4183db6187b046f80 100644
--- a/third_party/blink/renderer/core/css/style_engine.cc
+++ b/third_party/blink/renderer/core/css/style_engine.cc
@@ -2808,6 +2808,7 @@ void StyleEngine::RecalcStyle(StyleRecalcChange change,
const StyleRecalcContext& style_recalc_context) {
DCHECK(GetDocument().documentElement());
ScriptForbiddenScope forbid_script;
+ SkipStyleRecalcScope skip_scope(*this);
CheckPseudoHasCacheScope check_pseudo_has_cache_scope(&GetDocument());
Element& root_element = style_recalc_root_.RootElement();
Element* parent = FlatTreeTraversal::ParentElement(root_element);
@@ -3400,4 +3401,17 @@ void StyleEngine::MarkForLayoutTreeChangesAfterDetach() {
parent_for_detached_subtree_ = nullptr;
}

+bool StyleEngine::AllowSkipStyleRecalcForScope() const {
+ if (InContainerQueryStyleRecalc())
+ return true;
+ if (LocalFrameView* view = GetDocument().View()) {
+ // Existing layout roots before starting style recalc may end up being
+ // inside skipped subtrees if we allowed skipping. If we start out with an
+ // empty list, any added ones will be a result of an element style recalc,
+ // which means the will not be inside a skipped subtree.
+ return !view->IsSubtreeLayout();
+ }
+ return true;
+}
+
} // namespace blink
diff --git a/third_party/blink/renderer/core/css/style_engine.h b/third_party/blink/renderer/core/css/style_engine.h
index a6c325b69e5bc43c657519f4c0730f6e2efd4f71..f7ccc6c2b7b306d76bba60cf46da91a5b291c29e 100644
--- a/third_party/blink/renderer/core/css/style_engine.h
+++ b/third_party/blink/renderer/core/css/style_engine.h
@@ -178,6 +178,20 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
base::AutoReset<bool> allow_marking_;
};

+ // Set up the condition for allowing to skip style recalc before starting
+ // RecalcStyle().
+ class SkipStyleRecalcScope {
+ STACK_ALLOCATED();
+
+ public:
+ explicit SkipStyleRecalcScope(StyleEngine& engine)
+ : allow_skip_(&engine.allow_skip_style_recalc_,
+ engine.AllowSkipStyleRecalcForScope()) {}
+
+ private:
+ base::AutoReset<bool> allow_skip_;
+ };
+
explicit StyleEngine(Document&);
~StyleEngine() override;

@@ -342,6 +356,10 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,

bool MarkReattachAllowed() const;

+ // Returns true if we can skip style recalc for a size container subtree and
+ // resume it during layout.
+ bool SkipStyleRecalcAllowed() const { return allow_skip_style_recalc_; }
+
CSSFontSelector* GetFontSelector() { return font_selector_; }

void RemoveFontFaceRules(const HeapVector<Member<const StyleRuleFontFace>>&);
@@ -743,6 +761,9 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
Element& changed_element,
bool for_pseudo_change);

+ // Initialization value for SkipStyleRecalcScope.
+ bool AllowSkipStyleRecalcForScope() const;
+
Member<Document> document_;

// Tracks the number of currently loading top-level stylesheets. Sheets loaded
@@ -812,6 +833,9 @@ class CORE_EXPORT StyleEngine final : public GarbageCollected<StyleEngine>,
// AllowMarkStyleDirtyFromRecalcScope.
bool allow_mark_for_reattach_from_rebuild_layout_tree_{false};

+ // Set to true if we are allowed to skip recalc for a size container subtree.
+ bool allow_skip_style_recalc_{false};
+
// See enum ViewportUnitFlag.
unsigned viewport_unit_dirty_flags_{0};

diff --git a/third_party/blink/renderer/core/dom/element.cc b/third_party/blink/renderer/core/dom/element.cc
index 8e5c4aab2e37ed0acda0ef56273d007fbaa58780..2df360c33c54e12af341b77771fcda95e562fa92 100644
--- a/third_party/blink/renderer/core/dom/element.cc
+++ b/third_party/blink/renderer/core/dom/element.cc
@@ -3532,6 +3532,10 @@ bool Element::SkipStyleRecalcForContainer(
const ComputedStyle& style,
const StyleRecalcChange& child_change) {
DCHECK(RuntimeEnabledFeatures::CSSContainerSkipStyleRecalcEnabled());
+
+ if (!GetDocument().GetStyleEngine().SkipStyleRecalcAllowed())
+ return false;
+
if (!child_change.TraversePseudoElements(*this)) {
// If none of the children or pseudo elements need to be traversed for style
// recalc, there is no point in marking the subtree as skipped.
diff --git a/third_party/blink/web_tests/external/wpt/css/css-contain/container-queries/crashtests/chrome-layout-root-crash.html b/third_party/blink/web_tests/external/wpt/css/css-contain/container-queries/crashtests/chrome-layout-root-crash.html
new file mode 100644
index 0000000000000000000000000000000000000000..e3e709a240bd870250b2747c94fe96880bdf52e3
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-contain/container-queries/crashtests/chrome-layout-root-crash.html
@@ -0,0 +1,17 @@
+<!doctype html>
+<html class="reftest-wait">
+<link rel="help" href="https://crbug.com/1371820">
+<style>
+ body, div, img { container-type: size; }
+</style>
+<p>Pass if no crash.</p>
+<div id="div"><img id="img" alt="a"></div>
+<script>
+ requestAnimationFrame(() => requestAnimationFrame(() => {
+ // Adds a layout root inside the div size container.
+ img.alt = img.src = "b";
+ // Marks div size container for layout which skips style recalc for the sub-tree.
+ div.style.width = "500px";
+ document.documentElement.classList.remove("reftest-wait");
+ }));
+</script>

0 comments on commit 2282b17

Please sign in to comment.