Skip to content

Commit d928546

Browse files
andruudChromium LUCI CQ
andruud
authored and
Chromium LUCI CQ
committed
Make custom properties that are IACVT guaranteed-invalid
Per recent spec change [1], custom properties that are invalid at computed-value time (IACVT) shall become guaranteed-invalid [2] if the custom property supports guaranteed-invalid values. [1] w3c/csswg-drafts#6006 [2] https://drafts.csswg.org/css-variables/#guaranteed-invalid-value Fixed: 1110188 I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/0xrbzYe_vxU/m/7bsL76n9CgAJ Change-Id: Ibfbb0c641abaf29f099e082115d5f04f77637840 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2734621 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#860161}
1 parent b8f2c96 commit d928546

File tree

14 files changed

+76
-302
lines changed

14 files changed

+76
-302
lines changed

‎third_party/blink/public/mojom/web_feature/web_feature.mojom

+1-1
Original file line numberDiff line numberDiff line change
@@ -2685,7 +2685,7 @@ enum WebFeature {
26852685
kV8RTCRtpTransceiver_Stopped_AttributeGetter = 3374,
26862686
kV8RTCRtpTransceiver_Stop_Method = 3375,
26872687
kSecurePaymentConfirmation = 3376,
2688-
kCSSInvalidVariableUnset = 3377,
2688+
kOBSOLETE_CSSInvalidVariableUnset = 3377,
26892689
kElementInternalsShadowRoot = 3378,
26902690
kAnyPiiFieldDetected_PredictedTypeMatch = 3379,
26912691
kEmailFieldDetected_PredictedTypeMatch = 3380,

‎third_party/blink/renderer/core/css/properties/longhand.h

+8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,14 @@ class Longhand : public CSSProperty {
3131
}
3232
virtual void ApplyInitial(StyleResolverState&) const { NOTREACHED(); }
3333
virtual void ApplyInherit(StyleResolverState&) const { NOTREACHED(); }
34+
35+
void ApplyUnset(StyleResolverState& state) const {
36+
if (IsInherited())
37+
ApplyInherit(state);
38+
else
39+
ApplyInitial(state);
40+
}
41+
3442
// Properties which take tree-scoped references should override this method to
3543
// handle the TreeScope during application.
3644
virtual void ApplyValue(StyleResolverState& state,

‎third_party/blink/renderer/core/css/properties/longhands/custom_property.cc

+5
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ void CustomProperty::ApplyInherit(StyleResolverState& state) const {
8181
void CustomProperty::ApplyValue(StyleResolverState& state,
8282
const CSSValue& value) const {
8383
if (value.IsInvalidVariableValue()) {
84+
if (!SupportsGuaranteedInvalid()) {
85+
ApplyUnset(state);
86+
return;
87+
}
88+
8489
state.Style()->SetVariableData(name_, nullptr, IsInherited());
8590
if (registration_)
8691
state.Style()->SetVariableValue(name_, nullptr, IsInherited());

‎third_party/blink/renderer/core/css/resolver/style_cascade.cc

+1-20
Original file line numberDiff line numberDiff line change
@@ -639,14 +639,9 @@ const CSSValue* StyleCascade::ResolveCustomProperty(
639639

640640
state_.Style()->SetHasVariableDeclaration();
641641

642-
if (resolver.InCycle())
642+
if (!data || resolver.InCycle())
643643
return CSSInvalidVariableValue::Create();
644644

645-
if (!data) {
646-
MaybeUseCountInvalidVariableUnset(To<CustomProperty>(property));
647-
return cssvalue::CSSUnsetValue::Create();
648-
}
649-
650645
if (data == decl.Value())
651646
return &decl;
652647

@@ -981,18 +976,4 @@ void StyleCascade::MaybeUseCountRevert(const CSSValue& value) {
981976
CountUse(WebFeature::kCSSKeywordRevert);
982977
}
983978

984-
void StyleCascade::MaybeUseCountInvalidVariableUnset(
985-
const CustomProperty& property) {
986-
if (!property.SupportsGuaranteedInvalid())
987-
return;
988-
if (!property.IsInherited() && !property.HasInitialValue())
989-
return;
990-
const AtomicString& name = property.GetPropertyNameAtomicString();
991-
const ComputedStyle* parent_style = state_.ParentStyle();
992-
if (parent_style &&
993-
parent_style->GetVariableData(name, property.IsInherited())) {
994-
CountUse(WebFeature::kCSSInvalidVariableUnset);
995-
}
996-
}
997-
998979
} // namespace blink

‎third_party/blink/renderer/core/css/resolver/style_cascade.h

-1
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,6 @@ class CORE_EXPORT StyleCascade {
340340
void CountUse(WebFeature);
341341
void MaybeUseCountRevert(const CSSValue&);
342342
void MaybeUseCountSummaryDisplayBlock();
343-
void MaybeUseCountInvalidVariableUnset(const CustomProperty&);
344343

345344
StyleResolverState& state_;
346345
MatchResult match_result_;

‎third_party/blink/renderer/core/css/resolver/style_cascade_test.cc

+23-10
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,19 @@ TEST_F(StyleCascadeTest, RegisteredCycle) {
10011001
cascade.Add("--b", "var(--a)");
10021002
cascade.Apply();
10031003

1004+
EXPECT_EQ("0px", cascade.ComputedValue("--a"));
1005+
EXPECT_EQ("0px", cascade.ComputedValue("--b"));
1006+
}
1007+
1008+
TEST_F(StyleCascadeTest, UniversalSyntaxCycle) {
1009+
RegisterProperty(GetDocument(), "--a", "*", "foo", false);
1010+
RegisterProperty(GetDocument(), "--b", "*", "bar", false);
1011+
1012+
TestCascade cascade(GetDocument());
1013+
cascade.Add("--a", "var(--b)");
1014+
cascade.Add("--b", "var(--a)");
1015+
cascade.Apply();
1016+
10041017
EXPECT_FALSE(cascade.ComputedValue("--a"));
10051018
EXPECT_FALSE(cascade.ComputedValue("--b"));
10061019
}
@@ -1013,11 +1026,11 @@ TEST_F(StyleCascadeTest, PartiallyRegisteredCycle) {
10131026
cascade.Add("--b", "var(--a)");
10141027
cascade.Apply();
10151028

1016-
EXPECT_FALSE(cascade.ComputedValue("--a"));
1029+
EXPECT_EQ("0px", cascade.ComputedValue("--a"));
10171030
EXPECT_FALSE(cascade.ComputedValue("--b"));
10181031
}
10191032

1020-
TEST_F(StyleCascadeTest, FallbackTriggeredByRegisteredCycle) {
1033+
TEST_F(StyleCascadeTest, ReferencedRegisteredCycle) {
10211034
RegisterProperty(GetDocument(), "--a", "<length>", "0px", false);
10221035
RegisterProperty(GetDocument(), "--b", "<length>", "0px", false);
10231036

@@ -1030,10 +1043,10 @@ TEST_F(StyleCascadeTest, FallbackTriggeredByRegisteredCycle) {
10301043
cascade.Add("--d", "var(--b,2px)");
10311044
cascade.Apply();
10321045

1033-
EXPECT_FALSE(cascade.ComputedValue("--a"));
1034-
EXPECT_FALSE(cascade.ComputedValue("--b"));
1035-
EXPECT_EQ("1px", cascade.ComputedValue("--c"));
1036-
EXPECT_EQ("2px", cascade.ComputedValue("--d"));
1046+
EXPECT_EQ("0px", cascade.ComputedValue("--a"));
1047+
EXPECT_EQ("0px", cascade.ComputedValue("--b"));
1048+
EXPECT_EQ("0px", cascade.ComputedValue("--c"));
1049+
EXPECT_EQ("0px", cascade.ComputedValue("--d"));
10371050
}
10381051

10391052
TEST_F(StyleCascadeTest, CycleStillInvalidWithFallback) {
@@ -1196,7 +1209,7 @@ TEST_F(StyleCascadeTest, EmUnitCycle) {
11961209
cascade.Add("--x", "10em");
11971210
cascade.Apply();
11981211

1199-
EXPECT_FALSE(cascade.ComputedValue("--x"));
1212+
EXPECT_EQ("0px", cascade.ComputedValue("--x"));
12001213
}
12011214

12021215
TEST_F(StyleCascadeTest, SubstitutingEmCycles) {
@@ -1209,8 +1222,8 @@ TEST_F(StyleCascadeTest, SubstitutingEmCycles) {
12091222
cascade.Add("--z", "var(--x,1px)");
12101223
cascade.Apply();
12111224

1212-
EXPECT_FALSE(cascade.ComputedValue("--y"));
1213-
EXPECT_EQ("1px", cascade.ComputedValue("--z"));
1225+
EXPECT_EQ("0px", cascade.ComputedValue("--y"));
1226+
EXPECT_EQ("0px", cascade.ComputedValue("--z"));
12141227
}
12151228

12161229
TEST_F(StyleCascadeTest, RemUnit) {
@@ -1259,7 +1272,7 @@ TEST_F(StyleCascadeTest, RemUnitInRootFontSizeCycle) {
12591272
cascade.Add("--x", "1rem");
12601273
cascade.Apply();
12611274

1262-
EXPECT_FALSE(cascade.ComputedValue("--x"));
1275+
EXPECT_EQ("0px", cascade.ComputedValue("--x"));
12631276
}
12641277

12651278
TEST_F(StyleCascadeTest, RemUnitInRootFontSizeNonCycle) {

‎third_party/blink/renderer/core/css/style_engine_test.cc

-233
Original file line numberDiff line numberDiff line change
@@ -3182,239 +3182,6 @@ TEST_F(StyleEngineTest, SystemColorComputeToSelfUseCount) {
31823182
GetDocument().IsUseCounted(WebFeature::kCSSSystemColorComputeToSelf));
31833183
}
31843184

3185-
TEST_F(StyleEngineTest, InvalidVariableUnsetUseCount) {
3186-
// Do not count for basic variable usage.
3187-
GetDocument().body()->setInnerHTML(R"HTML(
3188-
<style>
3189-
#outer { --x: foo; }
3190-
#inner { --x: bar; }
3191-
</style>
3192-
<div id=outer>
3193-
<div id=inner></div>
3194-
<div>
3195-
)HTML");
3196-
UpdateAllLifecyclePhases();
3197-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3198-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3199-
3200-
// Do not count when a fallback handles the unknown variable.
3201-
GetDocument().body()->setInnerHTML(R"HTML(
3202-
<style>
3203-
#outer { --x: foo; }
3204-
#inner { --x: var(--unknown,bar); }
3205-
</style>
3206-
<div id=outer>
3207-
<div id=inner></div>
3208-
<div>
3209-
)HTML");
3210-
UpdateAllLifecyclePhases();
3211-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3212-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3213-
3214-
// Do not count for explicit 'unset'.
3215-
GetDocument().body()->setInnerHTML(R"HTML(
3216-
<style>
3217-
#outer { --x: foo; }
3218-
#inner { --x: unset; }
3219-
</style>
3220-
<div id=outer>
3221-
<div id=inner></div>
3222-
<div>
3223-
)HTML");
3224-
UpdateAllLifecyclePhases();
3225-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3226-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3227-
3228-
// Do not count when we anyway end up with the guaranteed-invalid value.
3229-
// (Applies to registered properties as well).
3230-
GetDocument().body()->setInnerHTML(R"HTML(
3231-
<style>
3232-
@property --y {
3233-
syntax: "*";
3234-
inherits: true;
3235-
}
3236-
@property --z {
3237-
syntax: "*";
3238-
inherits: false;
3239-
}
3240-
#inner {
3241-
--x: var(--unknown);
3242-
--y: var(--unknown);
3243-
--z: var(--unknown);
3244-
}
3245-
</style>
3246-
<div id=outer>
3247-
<div id=inner></div>
3248-
<div>
3249-
)HTML");
3250-
UpdateAllLifecyclePhases();
3251-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3252-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3253-
3254-
// Count when 'unset' inherits something that not guaranteed-invalid.
3255-
GetDocument().body()->setInnerHTML(R"HTML(
3256-
<style>
3257-
#outer { --x: foo; }
3258-
#inner { --x: var(--unknown); }
3259-
</style>
3260-
<div id=outer>
3261-
<div id=inner></div>
3262-
<div>
3263-
)HTML");
3264-
UpdateAllLifecyclePhases();
3265-
EXPECT_TRUE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3266-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3267-
3268-
// Do not count for non-universal registered custom properties.
3269-
GetDocument().body()->setInnerHTML(R"HTML(
3270-
<style>
3271-
@property --x {
3272-
syntax: "<length>";
3273-
inherits: true;
3274-
initial-value: 0px;
3275-
}
3276-
#outer { --x: 1px; }
3277-
#inner { --x: var(--unknown); }
3278-
</style>
3279-
<div id=outer>
3280-
<div id=inner></div>
3281-
<div>
3282-
)HTML");
3283-
UpdateAllLifecyclePhases();
3284-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3285-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3286-
3287-
// Count for universal registered custom properties.
3288-
GetDocument().body()->setInnerHTML(R"HTML(
3289-
<style>
3290-
@property --x {
3291-
syntax: "*";
3292-
inherits: true;
3293-
}
3294-
#outer { --x: bar; }
3295-
#inner { --x: var(--unknown); }
3296-
</style>
3297-
<div id=outer>
3298-
<div id=inner></div>
3299-
<div>
3300-
)HTML");
3301-
UpdateAllLifecyclePhases();
3302-
EXPECT_TRUE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3303-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3304-
3305-
// Do not count for non-inherited universal registered custom properties
3306-
// without initial value.
3307-
GetDocument().body()->setInnerHTML(R"HTML(
3308-
<style>
3309-
@property --x {
3310-
syntax: "*";
3311-
inherits: false;
3312-
}
3313-
#outer { --x: bar; }
3314-
#inner { --x: var(--unknown); }
3315-
</style>
3316-
<div id=outer>
3317-
<div id=inner></div>
3318-
<div>
3319-
)HTML");
3320-
UpdateAllLifecyclePhases();
3321-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3322-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3323-
3324-
// Count for universal registered custom properties even with an
3325-
// initial-value defined.
3326-
GetDocument().body()->setInnerHTML(R"HTML(
3327-
<style>
3328-
@property --x {
3329-
syntax: "*";
3330-
inherits: true;
3331-
initial-value: foo;
3332-
}
3333-
#outer { --x: bar; }
3334-
#inner { --x: var(--unknown); }
3335-
</style>
3336-
<div id=outer>
3337-
<div id=inner></div>
3338-
<div>
3339-
)HTML");
3340-
UpdateAllLifecyclePhases();
3341-
EXPECT_TRUE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3342-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3343-
3344-
// Do not count for cycles.
3345-
GetDocument().body()->setInnerHTML(R"HTML(
3346-
<style>
3347-
@property --a {
3348-
syntax: "*";
3349-
inherits: true;
3350-
}
3351-
@property --b {
3352-
syntax: "*";
3353-
inherits: true;
3354-
}
3355-
#outer {
3356-
--a: foo;
3357-
--b: foo;
3358-
--c: foo;
3359-
--d: foo;
3360-
}
3361-
#inner {
3362-
--a: var(--b);
3363-
--b: var(--a);
3364-
--c: var(--d);
3365-
--d: var(--c);
3366-
}
3367-
</style>
3368-
<div id=outer>
3369-
<div id=inner></div>
3370-
<div>
3371-
)HTML");
3372-
UpdateAllLifecyclePhases();
3373-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3374-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3375-
3376-
// Count for @keyframes
3377-
GetDocument().body()->setInnerHTML(R"HTML(
3378-
<style>
3379-
@keyframes anim {
3380-
from { --x: var(--unknown); }
3381-
to { --x: var(--unknown); }
3382-
}
3383-
#outer {
3384-
--x: foo;
3385-
}
3386-
#inner {
3387-
animation: anim 10s;
3388-
}
3389-
</style>
3390-
<div id=outer>
3391-
<div id=inner></div>
3392-
<div>
3393-
)HTML");
3394-
UpdateAllLifecyclePhases();
3395-
EXPECT_TRUE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3396-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3397-
3398-
// Don't count for @keyframes if there's nothing to inherit.
3399-
GetDocument().body()->setInnerHTML(R"HTML(
3400-
<style>
3401-
@keyframes anim {
3402-
from { --x: var(--unknown); }
3403-
to { --x: var(--unknown); }
3404-
}
3405-
#inner {
3406-
animation: anim 10s;
3407-
}
3408-
</style>
3409-
<div id=outer>
3410-
<div id=inner></div>
3411-
<div>
3412-
)HTML");
3413-
UpdateAllLifecyclePhases();
3414-
EXPECT_FALSE(IsUseCounted(WebFeature::kCSSInvalidVariableUnset));
3415-
ClearUseCounter(WebFeature::kCSSInvalidVariableUnset);
3416-
}
3417-
34183185
// https://crbug.com/1050564
34193186
TEST_F(StyleEngineTest, MediaAttributeChangeUpdatesFontCacheVersion) {
34203187
GetDocument().body()->setInnerHTML(R"HTML(

0 commit comments

Comments
 (0)