Correctly resolve queries when container size is affected by a subseq…

…uent sibling

https://bugs.webkit.org/show_bug.cgi?id=241457

Reviewed by Alan Bujtas.

We currently only take style changes before the container into account.

This patch changes the query container resolution so that whenever we encounter a query container
we skip the subtree and restart the resolution from the main resolution loop. This way we
can optimize away unnecessary layouts while also handling backward dependencies.

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/sibling-layout-dependency-expected.txt:
* Source/WebCore/dom/Document.cpp:
(WebCore::Document::resolveStyle):

Loop also when there are no style changes to apply but there are unresolved container. In this case we just resume
the style resolution where we left it.

* Source/WebCore/dom/Element.cpp:
(WebCore::Element::invalidateForQueryContainerChange):

Invalidate all descendants when a query container is resized. In practice we ended up computing them all anyway.

* Source/WebCore/style/StyleChange.cpp:
(WebCore::Style::determineChange):
* Source/WebCore/style/StyleChange.h:

Add new Descendants value for when container type or name changes.

* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::computeDescendantsToResolve):
(WebCore::Style::TreeResolver::pushParent):
(WebCore::Style::TreeResolver::popParent):
(WebCore::Style::TreeResolver::resolveComposedTree):
(WebCore::Style::TreeResolver::determineQueryContainerAction):

Use two buckets for unresolved and resolved containers. Use the resolved bucket to avoid
recomputing containers we have resolved already.

(WebCore::Style::TreeResolver::resolve):

Move the containers from the unresolved to resolved bucket in the beginning.

(WebCore::Style::TreeResolver::updateQueryContainer): Deleted.
* Source/WebCore/style/StyleTreeResolver.h:

Canonical link: https://commits.webkit.org/251424@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295418 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/imported/w3c/web-platform-tests/css/css-contain/container-queries/sibling-layout-dependency-expected.txt

@@ -1,7 +1,7 @@
 XXXX
 
-FAIL Sibling style mutation assert_equals: expected "10" but got "20"
-FAIL Sibling style mutation, parent is affected assert_equals: expected "10" but got "20"
-FAIL Sibling style mutation, ancestor is affected assert_equals: expected "10" but got "20"
-FAIL Sibling text mutation assert_equals: expected "10" but got "20"
+PASS Sibling style mutation
+PASS Sibling style mutation, parent is affected
+PASS Sibling style mutation, ancestor is affected
+PASS Sibling text mutation
 

Source/WebCore/dom/Document.cpp

@@ -2095,13 +2095,15 @@ void Document::resolveStyle(ResolveStyleType type)
         Style::TreeResolver resolver(*this, WTFMove(m_pendingRenderTreeUpdate));
         auto styleUpdate = resolver.resolve();
 
-        while (resolver.hasUnresolvedQueryContainers() && styleUpdate) {
-            SetForScope resolvingContainerQueriesScope(m_isResolvingContainerQueries, true);
-
-            updateRenderTree(WTFMove(styleUpdate));
-
-            if (frameView.layoutContext().needsLayout())
-                frameView.layoutContext().layout();
+        while (resolver.hasUnresolvedQueryContainers()) {
+            if (styleUpdate) {
+                SetForScope resolvingContainerQueriesScope(m_isResolvingContainerQueries, true);
+
+                updateRenderTree(WTFMove(styleUpdate));
+
+                if (frameView.layoutContext().needsLayout())
+                    frameView.layoutContext().layout();
+            }
 
             styleUpdate = resolver.resolve();
         }

Source/WebCore/dom/Element.cpp

@@ -2267,8 +2267,8 @@ void Element::invalidateStyleForSubtreeInternal()
 
 void Element::invalidateForQueryContainerChange()
 {
-    // FIXME: This doesn't really need to recompute the element style.
-    Node::invalidateStyle(Style::Validity::ElementInvalid);
+    // FIXME: Ideally we would just recompute things that are actually affected by containers queries within the subtree.
+    Node::invalidateStyle(Style::Validity::SubtreeInvalid);
 }
 
 void Element::invalidateEventListenerRegions()

Source/WebCore/style/StyleChange.cpp

@@ -57,11 +57,11 @@ Change determineChange(const RenderStyle& s1, const RenderStyle& s2)
     if (s1.hasTextCombine() != s2.hasTextCombine())
         return Change::Renderer;
 
-    if (!s1.descendantAffectingNonInheritedPropertiesEqual(s2))
-        return Change::Inherited;
-
     // Query container changes affect descendant style.
     if (s1.containerType() != s2.containerType() || s1.containerNames() != s2.containerNames())
+        return Change::Descendants;
+
+    if (!s1.descendantAffectingNonInheritedPropertiesEqual(s2))
         return Change::Inherited;
 
     if (!s1.nonFastPathInheritedEqual(s2))

Source/WebCore/style/StyleChange.h

@@ -36,6 +36,7 @@ enum class Change : uint8_t {
     NonInherited,
     FastPathInherited,
     Inherited,
+    Descendants,
     Renderer
 };
 

Source/WebCore/style/StyleTreeResolver.cpp

@@ -199,6 +199,7 @@ auto TreeResolver::computeDescendantsToResolve(Change change, Validity validity,
     case Change::FastPathInherited:
     case Change::Inherited:
         return DescendantsToResolve::Children;
+    case Change::Descendants:
     case Change::Renderer:
         return DescendantsToResolve::All;
     };
@@ -625,6 +626,8 @@ ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderSt
 void TreeResolver::pushParent(Element& element, const RenderStyle& style, Change change, DescendantsToResolve descendantsToResolve)
 {
     scope().selectorMatchingState.selectorFilter.pushParent(&element);
+    if (style.containerType() != ContainerType::None)
+        scope().selectorMatchingState.queryContainers.append(element);
 
     Parent parent(element, style, change, descendantsToResolve);
 
@@ -645,7 +648,9 @@ void TreeResolver::popParent()
     auto& parentElement = *parent().element;
 
     parentElement.setHasValidStyle();
-    parentElement.clearChildNeedsStyleRecalc();
+    // Don't clear the child flags if there are unresolved containers because we are going to resume the style resolution.
+    if (!hasUnresolvedQueryContainers())
+        parentElement.clearChildNeedsStyleRecalc();
 
     if (parent().didPushScope)
         popScope();
@@ -834,10 +839,21 @@ void TreeResolver::resolveComposedTree()
         if (!style)
             resetStyleForNonRenderedDescendants(element);
 
-        bool shouldIterateChildren = style && (element.childNeedsStyleRecalc() || descendantsToResolve != DescendantsToResolve::None);
+        auto queryContainerAction = determineQueryContainerAction(element, style, previousContainerType);
+
+        bool shouldIterateChildren = [&] {
+            // display::none, no need to resolve descendants.
+            if (!style)
+                return false;
+            // Style resolution will be resumed after the container has been resolved.
+            if (queryContainerAction == QueryContainerAction::Resolve)
+                return false;
+            return element.childNeedsStyleRecalc() || descendantsToResolve != DescendantsToResolve::None;
+        }();
 
-        if (style && updateQueryContainer(element, *style, previousContainerType) == QueryContainerAction::Layout)
-            shouldIterateChildren = false;
+        // Ensure we respect DescendantsToResolve::All after resuming the style resolution.
+        if (queryContainerAction == QueryContainerAction::Resolve && descendantsToResolve == DescendantsToResolve::All)
+            element.invalidateStyleForSubtreeInternal();
 
         if (!m_didSeePendingStylesheet)
             m_didSeePendingStylesheet = hasLoadingStylesheet(m_document.styleScope(), element, !shouldIterateChildren);
@@ -860,39 +876,34 @@ void TreeResolver::resolveComposedTree()
     popParentsToDepth(1);
 }
 
-auto TreeResolver::updateQueryContainer(Element& element, const RenderStyle& style, ContainerType previousContainerType) -> QueryContainerAction
+auto TreeResolver::determineQueryContainerAction(const Element& element, const RenderStyle* style, ContainerType previousContainerType) -> QueryContainerAction
 {
-    if (style.containerType() != ContainerType::None)
-        scope().selectorMatchingState.queryContainers.append(element);
-
-    if (m_unresolvedQueryContainers.remove(&element))
-        return QueryContainerAction::Continue;
+    if (!style)
+        return QueryContainerAction::None;
 
-    // Render tree needs to be updated before proceeding to children also if we have a former query container
-    // because container query resolution for descendants relies on it being up-to-date.
-    if (style.containerType() == ContainerType::None && previousContainerType == ContainerType::None)
+    // FIXME: Render tree needs to be updated before proceeding to children also if we have a former query container
+    // because container unit resolution for descendants relies on it being up-to-date.
+    if (style->containerType() == ContainerType::None && previousContainerType == ContainerType::None)
         return QueryContainerAction::None;
 
-    if (m_update->isEmpty())
-        return QueryContainerAction::Continue;
+    if (m_resolvedQueryContainers.contains(element))
+        return QueryContainerAction::None;
 
-    // Bail out from TreeResolver to build a render tree and do a layout. Resolution continues after.
-    m_unresolvedQueryContainers.add(&element);
-    return QueryContainerAction::Layout;
+    m_unresolvedQueryContainers.append(element);
+    return QueryContainerAction::Resolve;
 }
 
 std::unique_ptr<Update> TreeResolver::resolve()
 {
+    m_resolvedQueryContainers.add(m_unresolvedQueryContainers.begin(), m_unresolvedQueryContainers.end());
+    m_unresolvedQueryContainers.clear();
+
     Element* documentElement = m_document.documentElement();
     if (!documentElement) {
         m_document.styleScope().resolver();
         return nullptr;
     }
 
-    // FIXME: Just need to restore the ancestor marking.
-    for (auto& queryContainer : m_unresolvedQueryContainers)
-        queryContainer->invalidateStyleForSubtreeInternal();
-
     if (!documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
         return WTFMove(m_update);
 

Source/WebCore/style/StyleTreeResolver.h

@@ -63,8 +63,8 @@ class TreeResolver {
 
     void resolveComposedTree();
 
-    enum class QueryContainerAction : uint8_t { None, Continue, Layout };
-    QueryContainerAction updateQueryContainer(Element&, const RenderStyle&, ContainerType previousContainerType);
+    enum class QueryContainerAction : uint8_t { None, Resolve };
+    QueryContainerAction determineQueryContainerAction(const Element&, const RenderStyle*, ContainerType previousContainerType);
 
     enum class DescendantsToResolve : uint8_t { None, ChildrenWithExplicitInherit, Children, All };
     std::pair<ElementUpdate, DescendantsToResolve> resolveElement(Element&, ResolutionType);
@@ -129,7 +129,8 @@ class TreeResolver {
     Vector<Parent, 32> m_parentStack;
     bool m_didSeePendingStylesheet { false };
 
-    HashSet<RefPtr<Element>> m_unresolvedQueryContainers;
+    Vector<Ref<const Element>> m_unresolvedQueryContainers;
+    HashSet<Ref<const Element>> m_resolvedQueryContainers;
 
     std::unique_ptr<Update> m_update;
 };