1875441 - [css-properties-values-api] DOMWindowUtils.computeAnimationDistance causes tab crash when called with a registered property

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

Calling DOMWindowUtils.computeAnimationDistance with a registered property name, e.g. DOMWindowUtils.computeAnimationDistance(el, "--my-registered-css-property", "red", "blue") causes a tab crash.

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Looks like this is caused by https://searchfox.org/mozilla-central/rev/47b65cbe249613b9af936cd4660789bb642a8e30/layout/style/StyleAnimationValue.cpp#217-220

RefPtr<StyleLockedDeclarationBlock> declarations =
    ServoCSSParser::ParseProperty(property, aValue,
                                  ServoCSSParser::GetParsingEnvironment(doc),
                                  StyleParsingMode::DEFAULT);

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

and this might be because of https://searchfox.org/mozilla-central/rev/47b65cbe249613b9af936cd4660789bb642a8e30/servo/components/style/properties/mod.rs#437

Self::Custom(unsafe { crate::Atom::from_raw(property.mCustomName.mRawPtr) })

Comment 3

[Zach Hoffman [:zrhoffman]]

Attachment: crashtest

Adding a crashtest using Nicolas's reproduction instructions in comment 0.

Stack trace:

Mozilla crash reason: MOZ_ASSERT(aProperty != eCSSPropertyExtra_variable) (Cannot create an AnimatedPropertyID from only a eCSSPropertyExtra_variable.)
#01: mozilla::AnimatedPropertyID::AnimatedPropertyID(nsCSSPropertyID) (obj-x86_64-pc-linux-gnu/dist/include/mozilla/AnimatedPropertyID.h:20)
#02: mozilla::AnimationValue::FromString(nsCSSPropertyID, nsTSubstring<char> const&, mozilla::dom::Element*) (layout/style/StyleAnimationValue.cpp:217)
#03: nsDOMWindowUtils::ComputeAnimationDistance(mozilla::dom::Element*, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, double*) (dom/base/nsDOMWindowUtils.cpp:3155)
#04: ??? (obj-x86_64-pc-linux-gnu/dist/bin/libxul.so + 0x8dd2f12)
#05: CallMethodHelper::Invoke() (js/xpconnect/src/XPCWrappedNative.cpp:1627)
#06: CallMethodHelper::Call() (js/xpconnect/src/XPCWrappedNative.cpp:1180)
#07: XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (js/xpconnect/src/XPCWrappedNative.cpp:1126)
#08: XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (js/xpconnect/src/XPCWrappedNativeJSOps.cpp:966)
#09: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) (js/src/vm/Interpreter.cpp:479)
#10: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) (js/src/vm/Interpreter.cpp:573)

Comment 4

[Zach Hoffman [:zrhoffman]]

Attachment: Bug 1875441 - Pass entire AnimatedPropertyID to AnimationValue::FromString r=nchevobbe,#animation,#layout

When only an nsCSSPropertyID was passed, AnimationValue::FromString
would create an AnimatedPropertyID anyway, sometimes from
eCSSPropertyExtra_variable without a custom name.

Also removed an unused argument from AnimationValue::ComputeDistance.

Drive-by: Remove an unnecessary include and forward declaration.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb4394e287e3
Pass entire AnimatedPropertyID to AnimationValue::FromString r=layout-reviewers,emilio

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/bb4394e287e3

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1846516