Changeset 285801 in webkit

Timestamp:

Nov 15, 2021, 3:12:32 AM (5 years ago)

Author:

Antti Koivisto

Message:

Stack overflow with revert-layer
​https://bugs.webkit.org/show_bug.cgi?id=233119
rdar://85342210

Reviewed by Antoine Quint.

Source/WebCore:

We would decrement the cascade layer priority by one after finding 'revert-layer' value and then try
to apply again. If both the default layer and a cascade layer contained 'revert-layer' we would
enter a very deep recursion as the default layer priority is 64k and the cascade layer priorities
start from zero.

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

• style/StyleBuilder.cpp:

(WebCore::Style::Builder::applyRollbackCascadeProperty):

Fix by getting the new cascade layer priority from the actual property rather than the cascade
minimum value.

Factor into a function.

(WebCore::Style::Builder::applyProperty):

• style/StyleBuilder.h:

LayoutTests:

• fast/css/revert-layer-stack-overflow-2-expected.txt: Added.
• fast/css/revert-layer-stack-overflow-2.html: Added.

Location:

trunk

Files:

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

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-15  Antti Koivisto  <antti@apple.com>
+
+        Stack overflow with revert-layer
+        https://bugs.webkit.org/show_bug.cgi?id=233119
+        rdar://85342210
+
+        Reviewed by Antoine Quint.
+
+        * fast/css/revert-layer-stack-overflow-2-expected.txt: Added.
+        * fast/css/revert-layer-stack-overflow-2.html: Added.
+
 2021-11-14  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-15  Antti Koivisto  <antti@apple.com>
+
+        Stack overflow with revert-layer
+        https://bugs.webkit.org/show_bug.cgi?id=233119
+        rdar://85342210
+
+        Reviewed by Antoine Quint.
+
+        We would decrement the cascade layer priority by one after finding 'revert-layer' value and then try
+        to apply again. If both the default layer and a cascade layer contained 'revert-layer' we would
+        enter a very deep recursion as the default layer priority is 64k and the cascade layer priorities
+        start from zero.
+
+        Test: fast/css/revert-layer-stack-overflow-2.html
+
+        * style/StyleBuilder.cpp:
+        (WebCore::Style::Builder::applyRollbackCascadeProperty):
+
+        Fix by getting the new cascade layer priority from the actual property rather than the cascade
+        minimum value.
+
+        Factor into a function.
+
+        (WebCore::Style::Builder::applyProperty):
+        * style/StyleBuilder.h:
+
 2021-11-15  Patrick Griffis  <pgriffis@igalia.com>
 

trunk/Source/WebCore/style/StyleBuilder.cpp

@@ -256 +256 @@
 }
 
+void Builder::applyRollbackCascadeProperty(const PropertyCascade::Property& property, SelectorChecker::LinkMatchMask linkMatchMask)
+{
+    auto* value = property.cssValue[linkMatchMask];
+    if (!value)
+        return;
+
+    SetForScope levelScope(m_state.m_cascadeLevel, property.level);
+    SetForScope scopeScope(m_state.m_styleScopeOrdinal, property.styleScopeOrdinal);
+    SetForScope layerScope(m_state.m_cascadeLayerPriority, property.cascadeLayerPriority);
+
+    applyProperty(property.id, *value, linkMatchMask);
+}
+
 void Builder::applyProperty(CSSPropertyID id, CSSValue& value, SelectorChecker::LinkMatchMask linkMatchMask)
 {
@@ -297 +310 @@
             // With the rollback cascade built, we need to obtain the property and apply it. If the property is
             // not present, then we behave like "unset." Otherwise we apply the property instead of our own.
-            SetForScope cascadeLevelScope(m_state.m_cascadeLevel, rollbackCascade->maximumCascadeLevel());
-            SetForScope cascadeLayerPriorityScope(m_state.m_cascadeLayerPriority, rollbackCascade->maximumCascadeLayerPriority());
             if (customPropertyValue) {
                 if (customPropertyRegistered && customPropertyRegistered->inherits && rollbackCascade->hasCustomProperty(customPropertyValue->name())) {
                     auto property = rollbackCascade->customProperty(customPropertyValue->name());
-                    if (property.cssValue[linkMatchMask])
-                        applyProperty(property.id, *property.cssValue[linkMatchMask], linkMatchMask);
+                    applyRollbackCascadeProperty(property, linkMatchMask);
                     return;
                 }
             } else if (rollbackCascade->hasProperty(id)) {
                 auto& property = rollbackCascade->property(id);
-                if (property.cssValue[linkMatchMask])
-                    applyProperty(property.id, *property.cssValue[linkMatchMask], linkMatchMask);
+                applyRollbackCascadeProperty(property, linkMatchMask);
                 return;
             }

trunk/Source/WebCore/style/StyleBuilder.h

@@ -57 +57 @@
     void applyPropertiesImpl(int firstProperty, int lastProperty);
     void applyCascadeProperty(const PropertyCascade::Property&);
+    void applyRollbackCascadeProperty(const PropertyCascade::Property&, SelectorChecker::LinkMatchMask);
     void applyProperty(CSSPropertyID, CSSValue&, SelectorChecker::LinkMatchMask);