Changeset 293725 in webkit

Timestamp:

May 3, 2022 8:45:26 AM (3 months ago)

Author:

Antti Koivisto

Message:

[CSS Cascade Layers] Endless recursion with revert-layer in other tree context
​https://bugs.webkit.org/show_bug.cgi?id=239967
<rdar://92449950>

Reviewed by Alan Bujtas.

Source/WebCore:

We should only revert within a tree context (scope).

Adding more comprehensive WPTs separately.

Test: fast/css/revert-layer-tree-context-stack-overflow.html

• style/PropertyCascade.cpp:

(WebCore::Style::PropertyCascade::PropertyCascade):

Pass the property tree scope to the rollback cascade.

(WebCore::Style::PropertyCascade::addMatch):

Don't include properties from lower priority tree scopes to rollback cascade.
Reverse the logic for clarity.

• style/PropertyCascade.h:

(WebCore::Style::PropertyCascade::PropertyCascade):

• style/StyleBuilder.cpp:

(WebCore::Style::Builder::ensureRollbackCascadeForRevert):
(WebCore::Style::Builder::ensureRollbackCascadeForRevertLayer):
(WebCore::Style::Builder::makeRollbackCascadeKey):

Include tree scope to the key.

• style/StyleBuilder.h:

LayoutTests:

• fast/css/revert-layer-tree-context-stack-overflow-expected.html: Added.
• fast/css/revert-layer-tree-context-stack-overflow.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/revert-layer-tree-context-stack-overflow-expected.html (added)
• LayoutTests/fast/css/revert-layer-tree-context-stack-overflow.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/style/PropertyCascade.cpp (modified) (2 diffs)
• Source/WebCore/style/PropertyCascade.h (modified) (2 diffs)
• Source/WebCore/style/StyleBuilder.cpp (modified) (2 diffs)
• Source/WebCore/style/StyleBuilder.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-05-03  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] Endless recursion with revert-layer in other tree context
+        https://bugs.webkit.org/show_bug.cgi?id=239967
+        <rdar://92449950>
+
+        Reviewed by Alan Bujtas.
+
+        * fast/css/revert-layer-tree-context-stack-overflow-expected.html: Added.
+        * fast/css/revert-layer-tree-context-stack-overflow.html: Added.
+
 2022-05-03  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-05-03  Antti Koivisto  <antti@apple.com>
+
+        [CSS Cascade Layers] Endless recursion with revert-layer in other tree context
+        https://bugs.webkit.org/show_bug.cgi?id=239967
+        <rdar://92449950>
+
+        Reviewed by Alan Bujtas.
+
+        We should only revert within a tree context (scope).
+
+        Adding more comprehensive WPTs separately.
+
+        Test: fast/css/revert-layer-tree-context-stack-overflow.html
+
+        * style/PropertyCascade.cpp:
+        (WebCore::Style::PropertyCascade::PropertyCascade):
+
+        Pass the property tree scope to the rollback cascade.
+
+        (WebCore::Style::PropertyCascade::addMatch):
+
+        Don't include properties from lower priority tree scopes to rollback cascade.
+        Reverse the logic for clarity.
+
+        * style/PropertyCascade.h:
+        (WebCore::Style::PropertyCascade::PropertyCascade):
+        * style/StyleBuilder.cpp:
+        (WebCore::Style::Builder::ensureRollbackCascadeForRevert):
+        (WebCore::Style::Builder::ensureRollbackCascadeForRevertLayer):
+        (WebCore::Style::Builder::makeRollbackCascadeKey):
+
+        Include tree scope to the key.
+
+        * style/StyleBuilder.h:
+
 2022-05-03  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/style/PropertyCascade.cpp

@@ -47 +47 @@
 }
 
-PropertyCascade::PropertyCascade(const PropertyCascade& parent, CascadeLevel maximumCascadeLevel, std::optional<CascadeLayerPriority> maximumCascadeLayerPriorityForRollback)
+PropertyCascade::PropertyCascade(const PropertyCascade& parent, CascadeLevel maximumCascadeLevel, std::optional<ScopeOrdinal> rollbackScope, std::optional<CascadeLayerPriority> maximumCascadeLayerPriorityForRollback)
     : m_matchResult(parent.m_matchResult)
     , m_includedProperties(parent.m_includedProperties)
     , m_maximumCascadeLevel(maximumCascadeLevel)
+    , m_rollbackScope(rollbackScope)
     , m_maximumCascadeLayerPriorityForRollback(maximumCascadeLayerPriorityForRollback)
 {
@@ -173 +174 @@
 bool PropertyCascade::addMatch(const MatchedProperties& matchedProperties, CascadeLevel cascadeLevel, bool important)
 {
-    auto skipForRollback = [&] {
-        if (!m_maximumCascadeLayerPriorityForRollback)
+    auto includePropertiesForRollback = [&] {
+        if (m_rollbackScope && matchedProperties.styleScopeOrdinal > *m_rollbackScope)
+            return true;
+        if (cascadeLevel < m_maximumCascadeLevel)
+            return true;
+        if (matchedProperties.fromStyleAttribute == FromStyleAttribute::Yes)
             return false;
-        if (matchedProperties.styleScopeOrdinal != ScopeOrdinal::Element)
-            return false;
-        if (cascadeLevel < m_maximumCascadeLevel)
-            return false;
-        if (matchedProperties.fromStyleAttribute == FromStyleAttribute::Yes)
-            return true;
-        return matchedProperties.cascadeLayerPriority > *m_maximumCascadeLayerPriorityForRollback;
+        return matchedProperties.cascadeLayerPriority <= *m_maximumCascadeLayerPriorityForRollback;
     };
-    if (skipForRollback())
+    if (m_maximumCascadeLayerPriorityForRollback && !includePropertiesForRollback())
         return false;
 

trunk/Source/WebCore/style/PropertyCascade.h

@@ -42 +42 @@
 
     PropertyCascade(const MatchResult&, CascadeLevel, IncludedProperties);
-    PropertyCascade(const PropertyCascade&, CascadeLevel, std::optional<CascadeLayerPriority> maximumCascadeLayerPriorityForRollback = { });
+    PropertyCascade(const PropertyCascade&, CascadeLevel, std::optional<ScopeOrdinal> rollbackScope = { }, std::optional<CascadeLayerPriority> maximumCascadeLayerPriorityForRollback = { });
 
     ~PropertyCascade();
@@ -86 +86 @@
     const IncludedProperties m_includedProperties;
     const CascadeLevel m_maximumCascadeLevel;
+    const std::optional<ScopeOrdinal> m_rollbackScope;
     const std::optional<CascadeLayerPriority> m_maximumCascadeLayerPriorityForRollback;
 

trunk/Source/WebCore/style/StyleBuilder.cpp

@@ -389 +389 @@
     --rollbackCascadeLevel;
 
-    auto key = makeRollbackCascadeKey(rollbackCascadeLevel, 0);
+    auto key = makeRollbackCascadeKey(rollbackCascadeLevel);
     return m_rollbackCascades.ensure(key, [&] {
         return makeUnique<const PropertyCascade>(m_cascade, rollbackCascadeLevel);
@@ -408 +408 @@
         --rollbackLayerPriority;
 
-    auto key = makeRollbackCascadeKey(property.cascadeLevel, rollbackLayerPriority);
+    auto key = makeRollbackCascadeKey(property.cascadeLevel, property.styleScopeOrdinal, rollbackLayerPriority);
     return m_rollbackCascades.ensure(key, [&] {
-        return makeUnique<const PropertyCascade>(m_cascade, property.cascadeLevel, rollbackLayerPriority);
+        return makeUnique<const PropertyCascade>(m_cascade, property.cascadeLevel, property.styleScopeOrdinal, rollbackLayerPriority);
     }).iterator->value.get();
 }
 
-auto Builder::makeRollbackCascadeKey(CascadeLevel cascadeLevel, CascadeLayerPriority cascadeLayerPriority) -> RollbackCascadeKey
-{
-    return { static_cast<unsigned>(cascadeLevel), static_cast<unsigned>(cascadeLayerPriority) };
-}
-
-}
-}
+auto Builder::makeRollbackCascadeKey(CascadeLevel cascadeLevel, ScopeOrdinal scopeOrdinal, CascadeLayerPriority cascadeLayerPriority) -> RollbackCascadeKey
+{
+    return { static_cast<unsigned>(cascadeLevel), static_cast<unsigned>(scopeOrdinal), static_cast<unsigned>(cascadeLayerPriority) };
+}
+
+}
+}

trunk/Source/WebCore/style/StyleBuilder.h

@@ -66 +66 @@
     const PropertyCascade* ensureRollbackCascadeForRevertLayer();
 
-    using RollbackCascadeKey = std::pair<unsigned, unsigned>;
-    RollbackCascadeKey makeRollbackCascadeKey(CascadeLevel, CascadeLayerPriority);
+    using RollbackCascadeKey = std::tuple<unsigned, unsigned, unsigned>;
+    RollbackCascadeKey makeRollbackCascadeKey(CascadeLevel, ScopeOrdinal = ScopeOrdinal::Element, CascadeLayerPriority = 0);
 
     const PropertyCascade m_cascade;