Skip to content

Commit c3229c2

Browse files
anttijkanttijk
committed
[@Property] Handle dependency cycles involving root style
https://bugs.webkit.org/show_bug.cgi?id=250703 rdar://104324136 Reviewed by Alan Baradlay. 'rem' unit and others can create dependency cycles that apply to the document element style only. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/unit-cycles-expected.txt: * Source/WebCore/css/CSSPrimitiveValue.cpp: (WebCore::CSSPrimitiveValue::collectComputedStyleDependencies const): Use Vector instead of a HashMap since there are never very many of these properties. (WebCore::CSSPrimitiveValue::collectDirectComputationalDependencies const): Deleted. (WebCore::CSSPrimitiveValue::collectDirectRootComputationalDependencies const): Deleted. Factor these to a single function that collect both root and normal dependencies. * Source/WebCore/css/CSSPrimitiveValue.h: * Source/WebCore/css/CSSValue.cpp: (WebCore::CSSValue::computedStyleDependencies const): (WebCore::CSSValue::collectComputedStyleDependencies const): (WebCore::CSSValue::collectDirectComputationalDependencies const): Deleted. (WebCore::CSSValue::collectDirectRootComputationalDependencies const): Deleted. * Source/WebCore/css/CSSValue.h: (WebCore::ComputedStyleDependencies::isEmpty const): * Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp: (WebCore::DOMCSSRegisterCustomProperty::registerProperty): * Source/WebCore/css/calc/CSSCalcExpressionNode.h: * Source/WebCore/css/calc/CSSCalcInvertNode.h: * Source/WebCore/css/calc/CSSCalcNegateNode.h: * Source/WebCore/css/calc/CSSCalcOperationNode.cpp: (WebCore::CSSCalcOperationNode::collectComputedStyleDependencies const): (WebCore::CSSCalcOperationNode::collectDirectComputationalDependencies const): Deleted. (WebCore::CSSCalcOperationNode::collectDirectRootComputationalDependencies const): Deleted. * Source/WebCore/css/calc/CSSCalcOperationNode.h: * Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.cpp: (WebCore::CSSCalcPrimitiveValueNode::collectComputedStyleDependencies const): (WebCore::CSSCalcPrimitiveValueNode::collectDirectComputationalDependencies const): Deleted. (WebCore::CSSCalcPrimitiveValueNode::collectDirectRootComputationalDependencies const): Deleted. * Source/WebCore/css/calc/CSSCalcPrimitiveValueNode.h: * Source/WebCore/css/calc/CSSCalcValue.cpp: (WebCore::CSSCalcValue::collectComputedStyleDependencies const): (WebCore::CSSCalcValue::collectDirectComputationalDependencies const): Deleted. (WebCore::CSSCalcValue::collectDirectRootComputationalDependencies const): Deleted. * Source/WebCore/css/calc/CSSCalcValue.h: * Source/WebCore/css/parser/CSSParser.cpp: (WebCore::CSSParser::parseCustomPropertyValueWithVariableReferences): Check for root style dependencies if needed. Always mark all cycles. This could have impact in some sort of multi-property cycle case. * Source/WebCore/css/parser/CSSPropertyParser.cpp: (WebCore::CSSPropertyParser::collectParsedCustomPropertyValueDependencies): * Source/WebCore/css/parser/CSSPropertyParser.h: * Source/WebCore/style/CustomPropertyRegistry.cpp: (WebCore::Style::CustomPropertyRegistry::registerFromStylesheet): * Source/WebCore/style/PropertyCascade.cpp: (WebCore::Style::PropertyCascade::shouldApplyAfterAnimation): Canonical link: https://commits.webkit.org/258985@main
1 parent a5b1dde commit c3229c2

20 files changed

+76
-102
lines changed

‎LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/unit-cycles-expected.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,19 @@ PASS Lengths with ex units may not be referenced from font-size
55
PASS Lengths with ch units may not be referenced from font-size
66
PASS Lengths with lh units may not be referenced from font-size
77
PASS Lengths with rem units may be referenced from font-size on non-root element
8-
FAIL Lengths with rem units may not be referenced from font-size on root element assert_equals: expected "16px" but got "32px"
8+
PASS Lengths with rem units may not be referenced from font-size on root element
99
PASS Lengths with lh units may not be referenced from line-height
1010
PASS Fallback may not use font-relative units
1111
PASS Fallback may not use line-height-relative units
1212
PASS Fallback not triggered while inside em unit cycle
1313
PASS Fallback not triggered while inside ex unit cycle
1414
PASS Fallback not triggered while inside ch unit cycle
15-
FAIL Fallback not triggered while inside rem unit cycle on root element assert_equals: expected "16px" but got "32px"
15+
PASS Fallback not triggered while inside rem unit cycle on root element
1616
PASS Fallback not triggered while inside lh unit cycle
1717
PASS Lengths with em units are detected via var references
1818
PASS Lengths with ex units are detected via var references
1919
PASS Lengths with ch units are detected via var references
20-
FAIL Lengths with rem units are detected via var references assert_equals: expected "16px" but got "160px"
20+
PASS Lengths with rem units are detected via var references
2121
PASS Lengths with lh units are detected via var references
2222
PASS Inherited lengths with em units may be used
2323
PASS Inherited lengths with ex units may be used

‎Source/WebCore/css/CSSPrimitiveValue.cpp‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,40 +1674,29 @@ Ref<DeprecatedCSSOMPrimitiveValue> CSSPrimitiveValue::createDeprecatedCSSOMPrimi
16741674
}
16751675

16761676
// https://drafts.css-houdini.org/css-properties-values-api/#dependency-cycles
1677-
void CSSPrimitiveValue::collectDirectComputationalDependencies(HashSet<CSSPropertyID>& values) const
1677+
void CSSPrimitiveValue::collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const
16781678
{
16791679
switch (primitiveUnitType()) {
1680+
case CSSUnitType::CSS_REMS:
1681+
dependencies.rootProperties.appendIfNotContains(CSSPropertyFontSize);
1682+
break;
1683+
case CSSUnitType::CSS_RLHS:
1684+
dependencies.rootProperties.appendIfNotContains(CSSPropertyFontSize);
1685+
dependencies.rootProperties.appendIfNotContains(CSSPropertyLineHeight);
1686+
break;
16801687
case CSSUnitType::CSS_EMS:
16811688
case CSSUnitType::CSS_QUIRKY_EMS:
16821689
case CSSUnitType::CSS_EXS:
16831690
case CSSUnitType::CSS_CHS:
16841691
case CSSUnitType::CSS_IC:
1685-
values.add(CSSPropertyFontSize);
1692+
dependencies.properties.appendIfNotContains(CSSPropertyFontSize);
16861693
break;
16871694
case CSSUnitType::CSS_LHS:
1688-
values.add(CSSPropertyFontSize);
1689-
values.add(CSSPropertyLineHeight);
1690-
break;
1691-
case CSSUnitType::CSS_CALC:
1692-
m_value.calc->collectDirectComputationalDependencies(values);
1693-
break;
1694-
default:
1695-
break;
1696-
}
1697-
}
1698-
1699-
void CSSPrimitiveValue::collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>& values) const
1700-
{
1701-
switch (primitiveUnitType()) {
1702-
case CSSUnitType::CSS_REMS:
1703-
values.add(CSSPropertyFontSize);
1704-
break;
1705-
case CSSUnitType::CSS_RLHS:
1706-
values.add(CSSPropertyFontSize);
1707-
values.add(CSSPropertyLineHeight);
1695+
dependencies.properties.appendIfNotContains(CSSPropertyFontSize);
1696+
dependencies.properties.appendIfNotContains(CSSPropertyLineHeight);
17081697
break;
17091698
case CSSUnitType::CSS_CALC:
1710-
m_value.calc->collectDirectRootComputationalDependencies(values);
1699+
m_value.calc->collectComputedStyleDependencies(dependencies);
17111700
break;
17121701
default:
17131702
break;

‎Source/WebCore/css/CSSPrimitiveValue.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,7 @@ class CSSPrimitiveValue final : public CSSValue {
202202

203203
Ref<DeprecatedCSSOMPrimitiveValue> createDeprecatedCSSOMPrimitiveWrapper(CSSStyleDeclaration&) const;
204204

205-
void collectDirectComputationalDependencies(HashSet<CSSPropertyID>&) const;
206-
void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>&) const;
205+
void collectComputedStyleDependencies(ComputedStyleDependencies&) const;
207206

208207
private:
209208
friend class CSSValuePool;

‎Source/WebCore/css/CSSValue.cpp‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -222,28 +222,23 @@ bool CSSValue::traverseSubresources(const Function<bool(const CachedResource&)>&
222222
});
223223
}
224224

225-
void CSSValue::collectDirectComputationalDependencies(HashSet<CSSPropertyID>& values) const
225+
ComputedStyleDependencies CSSValue::computedStyleDependencies() const
226226
{
227-
if (auto* asList = dynamicDowncast<CSSValueList>(*this)) {
228-
for (auto& listValue : *asList)
229-
listValue->collectDirectComputationalDependencies(values);
230-
return;
231-
}
232-
233-
if (is<CSSPrimitiveValue>(*this))
234-
downcast<CSSPrimitiveValue>(*this).collectDirectComputationalDependencies(values);
227+
ComputedStyleDependencies dependencies;
228+
collectComputedStyleDependencies(dependencies);
229+
return dependencies;
235230
}
236231

237-
void CSSValue::collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>& values) const
232+
void CSSValue::collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const
238233
{
239234
if (auto* asList = dynamicDowncast<CSSValueList>(*this)) {
240235
for (auto& listValue : *asList)
241-
listValue->collectDirectRootComputationalDependencies(values);
236+
listValue->collectComputedStyleDependencies(dependencies);
242237
return;
243238
}
244239

245240
if (is<CSSPrimitiveValue>(*this))
246-
downcast<CSSPrimitiveValue>(*this).collectDirectRootComputationalDependencies(values);
241+
downcast<CSSPrimitiveValue>(*this).collectComputedStyleDependencies(dependencies);
247242
}
248243

249244
bool CSSValue::equals(const CSSValue& other) const

‎Source/WebCore/css/CSSValue.h‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <wtf/Noncopyable.h>
2424
#include <wtf/RefPtr.h>
2525
#include <wtf/TypeCasts.h>
26+
#include <wtf/Vector.h>
2627
#include <wtf/text/ASCIILiteral.h>
2728

2829
namespace WebCore {
@@ -33,6 +34,13 @@ class DeprecatedCSSOMValue;
3334

3435
enum CSSPropertyID : uint16_t;
3536

37+
struct ComputedStyleDependencies {
38+
Vector<CSSPropertyID> properties;
39+
Vector<CSSPropertyID> rootProperties;
40+
41+
bool isEmpty() const { return properties.isEmpty() && rootProperties.isEmpty(); }
42+
};
43+
3644
DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(CSSValue);
3745
class CSSValue {
3846
WTF_MAKE_NONCOPYABLE(CSSValue);
@@ -122,9 +130,8 @@ class CSSValue {
122130
bool traverseSubresources(const Function<bool(const CachedResource&)>&) const;
123131

124132
// What properties does this value rely on (eg, font-size for em units)
125-
void collectDirectComputationalDependencies(HashSet<CSSPropertyID>&) const;
126-
// What properties in the root element does this value rely on (eg. font-size for rem units)
127-
void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>&) const;
133+
ComputedStyleDependencies computedStyleDependencies() const;
134+
void collectComputedStyleDependencies(ComputedStyleDependencies&) const;
128135

129136
bool equals(const CSSValue&) const;
130137
bool operator==(const CSSValue& other) const { return equals(other); }

‎Source/WebCore/css/DOMCSSRegisterCustomProperty.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ ExceptionOr<void> DOMCSSRegisterCustomProperty::registerProperty(Document& docum
5858
if (!descriptor.initialValue.isNull()) {
5959
CSSTokenizer tokenizer(descriptor.initialValue);
6060

61-
HashSet<CSSPropertyID> dependencies;
62-
CSSPropertyParser::collectParsedCustomPropertyValueDependencies(*syntax, true /* isInitial */, dependencies, tokenizer.tokenRange(), strictCSSParserContext());
61+
auto dependencies = CSSPropertyParser::collectParsedCustomPropertyValueDependencies(*syntax, tokenizer.tokenRange(), strictCSSParserContext());
6362

6463
if (!dependencies.isEmpty())
6564
return Exception { SyntaxError, "The given initial value must be computationally independent."_s };

‎Source/WebCore/css/calc/CSSCalcExpressionNode.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
namespace WebCore {
3535

3636
class CSSToLengthConversionData;
37+
struct ComputedStyleDependencies;
3738

3839
enum CSSPropertyID : uint16_t;
3940

@@ -57,8 +58,7 @@ class CSSCalcExpressionNode : public RefCounted<CSSCalcExpressionNode> {
5758
virtual Type type() const = 0;
5859
virtual CSSUnitType primitiveType() const = 0;
5960

60-
virtual void collectDirectComputationalDependencies(HashSet<CSSPropertyID>&) const = 0;
61-
virtual void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>&) const = 0;
61+
virtual void collectComputedStyleDependencies(ComputedStyleDependencies&) const = 0;
6262
virtual bool convertingToLengthRequiresNonNullStyle(int lengthConversion) const = 0;
6363

6464
CalculationCategory category() const { return m_category; }

‎Source/WebCore/css/calc/CSSCalcInvertNode.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ class CSSCalcInvertNode final : public CSSCalcExpressionNode {
5656
Type type() const final { return Type::CssCalcInvert; }
5757
CSSUnitType primitiveType() const final { return m_child->primitiveType(); }
5858

59-
void collectDirectComputationalDependencies(HashSet<CSSPropertyID>& properties) const final { m_child->collectDirectComputationalDependencies(properties); }
60-
void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>& properties) const final { m_child->collectDirectRootComputationalDependencies(properties); }
59+
void collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const final { m_child->collectComputedStyleDependencies(dependencies); }
6160
bool convertingToLengthRequiresNonNullStyle(int lengthConversion) const final { return m_child->convertingToLengthRequiresNonNullStyle(lengthConversion); }
6261

6362
void dump(TextStream&) const final;

‎Source/WebCore/css/calc/CSSCalcNegateNode.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,7 @@ class CSSCalcNegateNode final : public CSSCalcExpressionNode {
5656
Type type() const final { return Type::CssCalcNegate; }
5757
CSSUnitType primitiveType() const final { return m_child->primitiveType(); }
5858

59-
void collectDirectComputationalDependencies(HashSet<CSSPropertyID>& properties) const final { m_child->collectDirectComputationalDependencies(properties); }
60-
void collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>& properties) const final { m_child->collectDirectRootComputationalDependencies(properties); }
59+
void collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const final { m_child->collectComputedStyleDependencies(dependencies); }
6160
bool convertingToLengthRequiresNonNullStyle(int lengthConversion) const final { return m_child->convertingToLengthRequiresNonNullStyle(lengthConversion); }
6261

6362
void dump(TextStream&) const final;

‎Source/WebCore/css/calc/CSSCalcOperationNode.cpp‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,16 +1072,10 @@ double CSSCalcOperationNode::computeLengthPx(const CSSToLengthConversionData& co
10721072
}));
10731073
}
10741074

1075-
void CSSCalcOperationNode::collectDirectComputationalDependencies(HashSet<CSSPropertyID>& values) const
1075+
void CSSCalcOperationNode::collectComputedStyleDependencies(ComputedStyleDependencies& dependencies) const
10761076
{
10771077
for (auto& child : m_children)
1078-
child->collectDirectComputationalDependencies(values);
1079-
}
1080-
1081-
void CSSCalcOperationNode::collectDirectRootComputationalDependencies(HashSet<CSSPropertyID>& values) const
1082-
{
1083-
for (auto& child : m_children)
1084-
child->collectDirectRootComputationalDependencies(values);
1078+
child->collectComputedStyleDependencies(dependencies);
10851079
}
10861080

10871081
bool CSSCalcOperationNode::convertingToLengthRequiresNonNullStyle(int lengthConversion) const

0 commit comments

Comments
 (0)