Attachment 452966 Details for Bug 237079 – Patch

[patch] Patch

bug-237079-20220223151855.patch (text/plain), 11.48 KB, created by Antti Koivisto on 2022-02-23 05:18:57 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2022-02-23 05:18:57 PST

Size: 11.48 KB

patch

obsolete

>Subversion Revision: 290366
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ed1206a13d7a54e075a747c4c9e602640ede9e17..a2fcd90a5c090e9bf6f14ff7f01934853da9b52d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2022-02-23  Antti Koivisto  <antti@apple.com>
>+
>+        [CSS Container Queries] offsetWidth/Height and similar should update layout for container queries
>+        https://bugs.webkit.org/show_bug.cgi?id=237079
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix
>+
>+        css/css-contain/container-queries/inline-size-containment.html
>+        css/css-contain/container-queries/inline-size-containment-vertical-rl.html
>+
>+        * dom/Document.cpp:
>+        (WebCore::Document::updateLayout):
>+
>+        Make iterative instead of recursive.
>+
>+        (WebCore::Document::updateLayoutIfDimensionsOutOfDate):
>+
>+        Check for query containers.
>+        Call local updateLayout instead of the FrameViewLayoutContext one.
>+
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive):
>+        * style/StyleScope.cpp:
>+        (WebCore::Style::Scope::updateQueryContainerState):
>+
>+        Add protection against infinite layout/invalidation cycle with unstable layouts.
>+
>+        * style/StyleScope.h:
>+
> 2022-02-23  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [GStreamer] Add WebKitDMABufVideoSink
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 2f0bcd8636134ff54fcb9c45d78909e3c6aff3da..8a96ff2ef079cef19eb72b03023eb0ba3b7dac11 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -2208,8 +2208,15 @@ void Document::updateLayout()
> 
>     frameView->layoutContext().layout();
> 
>-    if (styleScope().updateQueryContainerState())
>-        updateLayout();
>+    Style::Scope::QueryContainerUpdateContext queryContainerUpdateContext;
>+    while (styleScope().updateQueryContainerState(queryContainerUpdateContext)) {
>+        updateStyleIfNeeded();
>+
>+        if (!frameView->layoutContext().needsLayout())
>+            break;
>+
>+        frameView->layoutContext().layout();
>+    }
> }
> 
> void Document::updateLayoutIgnorePendingStylesheets(Document::RunPostLayoutTasks runPostLayoutTasks)
>@@ -2313,7 +2320,12 @@ bool Document::updateLayoutIfDimensionsOutOfDate(Element& element, DimensionsChe
>             
>             previousBox = currentBox;
>             currentBox = downcast<RenderBox>(currRenderer);
>-            
>+
>+            if (currentBox->style().containerType() != ContainerType::None) {
>+                requireFullLayout = true;
>+                break;
>+            }
>+
>             // If a box needs layout for itself or if a box has changed children and sizes its width to
>             // its content, then require a full layout.
>             if (currentBox->selfNeedsLayout() ||
>@@ -2348,9 +2360,9 @@ bool Document::updateLayoutIfDimensionsOutOfDate(Element& element, DimensionsChe
>     
>     StackStats::LayoutCheckPoint layoutCheckPoint;
> 
>-    // Only do a layout if changes have occurred that make it necessary.      
>-    if (requireFullLayout && frameView && renderView() && (frameView->layoutContext().isLayoutPending() || renderView()->needsLayout()))
>-        frameView->layoutContext().layout();
>+    // Only do a layout if changes have occurred that make it necessary.
>+    if (requireFullLayout)
>+        updateLayout();
>     
>     return requireFullLayout;
> }
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 46d2c0fb5e3b6c47112436b665dcf246c3549427..1e986ab2d231a3bdee2438bca3ca35cdc772ebec 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -4541,6 +4541,8 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
>         return descendantsDeque.first().ptr();
>     };
> 
>+    Style::Scope::QueryContainerUpdateContext queryContainerUpdateContext;
>+
>     for (unsigned i = 0; i < maxUpdatePasses; ++i) {
>         bool didWork = false;
>         DescendantsDeque deque;
>@@ -4549,7 +4551,7 @@ void FrameView::updateLayoutAndStyleIfNeededRecursive()
>                 didWork = true;
>             if (view->needsLayout()) {
>                 view->layoutContext().layout();
>-                view->frame().document()->styleScope().updateQueryContainerState();
>+                view->frame().document()->styleScope().updateQueryContainerState(queryContainerUpdateContext);
>                 didWork = true;
>             }
>         }
>diff --git a/Source/WebCore/style/StyleScope.cpp b/Source/WebCore/style/StyleScope.cpp
>index 9330606f4ab7571972f0da2cd34b171c5070be15..73a57f95588528db601a1bf698185f5ea41489e9 100644
>--- a/Source/WebCore/style/StyleScope.cpp
>+++ b/Source/WebCore/style/StyleScope.cpp
>@@ -783,7 +783,7 @@ bool Scope::isForUserAgentShadowTree() const
>     return m_shadowRoot && m_shadowRoot->mode() == ShadowRootMode::UserAgent;
> }
> 
>-bool Scope::updateQueryContainerState()
>+bool Scope::updateQueryContainerState(QueryContainerUpdateContext& context)
> {
>     ASSERT(!m_shadowRoot);
>     ASSERT(m_document.renderView());
>@@ -791,7 +791,7 @@ bool Scope::updateQueryContainerState()
>     auto previousStates = WTFMove(m_queryContainerStates);
>     m_queryContainerStates.clear();
> 
>-    Vector<Element*> changedContainers;
>+    Vector<Element*> containersToInvalidate;
> 
>     for (auto& containerRenderer : m_document.renderView()->containerQueryBoxes()) {
>         auto* containerElement = containerRenderer.element();
>@@ -814,15 +814,16 @@ bool Scope::updateQueryContainerState()
> 
>         auto it = previousStates.find(*containerElement);
>         bool changed = it == previousStates.end() || sizeChanged(it->value);
>-        if (changed)
>-            changedContainers.append(containerElement);
>+        // Protect against unstable layout by invalidating only once per container.
>+        if (changed && context.invalidatedContainers.add(containerElement).isNewEntry)
>+            containersToInvalidate.append(containerElement);
>         m_queryContainerStates.add(*containerElement, size);
>     }
> 
>-    for (auto* toInvalidate : changedContainers)
>+    for (auto* toInvalidate : containersToInvalidate)
>         toInvalidate->invalidateForQueryContainerChange();
> 
>-    return !changedContainers.isEmpty();
>+    return !containersToInvalidate.isEmpty();
> }
> 
> HTMLSlotElement* assignedSlotForScopeOrdinal(const Element& element, ScopeOrdinal scopeOrdinal)
>diff --git a/Source/WebCore/style/StyleScope.h b/Source/WebCore/style/StyleScope.h
>index 349760b6fecd4606d3174642f51358f201a29ed6..f139a28a331ad37284524f4bd2058cc201b9fded 100644
>--- a/Source/WebCore/style/StyleScope.h
>+++ b/Source/WebCore/style/StyleScope.h
>@@ -129,7 +129,10 @@ public:
>     static Scope& forNode(Node&);
>     static Scope* forOrdinal(Element&, ScopeOrdinal);
> 
>-    bool updateQueryContainerState();
>+    struct QueryContainerUpdateContext {
>+        HashSet<Element*> invalidatedContainers;
>+    };
>+    bool updateQueryContainerState(QueryContainerUpdateContext&);
> 
> private:
>     Scope& documentScope();
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 992d117ec5ffcc5d4ab28e032e095a163c4e0f11..e16c9c140478b3e0d93d1acf8ee4c723f91d626e 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2022-02-23  Antti Koivisto  <antti@apple.com>
>+
>+        [CSS Container Queries] offsetWidth/Height and similar should update layout for container queries
>+        https://bugs.webkit.org/show_bug.cgi?id=237079
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * web-platform-tests/css/css-contain/container-queries/auto-scrollbars-expected.txt:
>+        * web-platform-tests/css/css-contain/container-queries/inline-size-containment-expected.txt:
>+        * web-platform-tests/css/css-contain/container-queries/inline-size-containment-vertical-rl-expected.txt:
>+        * web-platform-tests/css/css-contain/container-queries/transition-scrollbars-expected.txt:
>+
> 2022-02-22  Antti Koivisto  <antti@apple.com>
> 
>         [CSS container Queries] getComputedStyle should update style for invalid query containers
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/auto-scrollbars-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/auto-scrollbars-expected.txt
>index 6a2d1a2586f7e714ab99aeea204529427ee795fd..ff64b2fdf6a5b895ad03e474356d5cc3030f6079 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/auto-scrollbars-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/auto-scrollbars-expected.txt
>@@ -1,4 +1,4 @@
> 
>-FAIL Initial layout - expecting a scrollbar without overflowing content instead of overflowing content without a scrollbar assert_equals: Layout with a scrollbar means the container query applies expected "50px" but got "100px"
>+FAIL Initial layout - expecting a scrollbar without overflowing content instead of overflowing content without a scrollbar assert_less_than: Expects a vertical scrollbar expected a number less than 100 but got 100
> FAIL Same result after a reflow assert_less_than: Expects a vertical scrollbar expected a number less than 100 but got 100
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-expected.txt
>index 4e475a691fd06bdcd5c8be9c57d8bb61c4873f1d..eb58bd3d480e26470fd700c0dbbd4bf024f9f7b2 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-expected.txt
>@@ -1,3 +1,3 @@
> 
>-FAIL inline-size containment only assert_equals: expected 400 but got 20
>+PASS inline-size containment only
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-vertical-rl-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-vertical-rl-expected.txt
>index b6bd9331c5dfdef61a5ab86e1ad68864cbb7ddd8..eb58bd3d480e26470fd700c0dbbd4bf024f9f7b2 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-vertical-rl-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/inline-size-containment-vertical-rl-expected.txt
>@@ -1,3 +1,3 @@
> 
>-FAIL inline-size containment only assert_equals: expected 50 but got 400
>+PASS inline-size containment only
> 
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/transition-scrollbars-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/transition-scrollbars-expected.txt
>index 24e32f8bd94f6dd8231933ece5fa9b41651ea9dc..bf8ffb2e4ea95f35047747937de50187964ca27e 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/transition-scrollbars-expected.txt
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/transition-scrollbars-expected.txt
>@@ -1,4 +1,4 @@
> Foo bar foo bar foo Foo bar foo bar foo Foo bar foo bar foo Foo bar foo bar foo Foo bar foo bar foo
> 
>-FAIL Scrollbars do not cause a transition of background-color assert_equals: expected "rgb(0, 128, 0)" but got "rgb(0, 0, 255)"
>+FAIL Scrollbars do not cause a transition of background-color assert_equals: expected "rgb(0, 0, 255)" but got "rgb(0, 64, 128)"
>

Actions: View | Formatted Diff | Diff

Attachments on bug 237079: 452966