stylo: dom/animation/test/chrome/test_simulate_compute_values_failure.html crash with Assertion failure: mUnit == eCSSUnit_TokenStream (not a token stream value)

NEW
Assigned to

Status

()

Core
CSS Parsing and Computation
P2
normal
11 days ago
4 days ago

People

(Reporter: ferjm, Assigned: ferjm)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

11 days ago
Assertion failure: mUnit == eCSSUnit_TokenStream (not a token stream value), at /Users/ferjm/dev/mozilla/mozilla-central/layout/style/nsCSSValue.h:795
LLVM ERROR: IO failure on output stream.
GECKO(20772) | #01: mozilla::dom::KeyframeEffectReadOnly::SetKeyframes(JSContext*, JS::Handle<JSObject*>, mozilla::ErrorResult&) (ErrorResult.h:380, in XUL)
GECKO(20772) | #02: already_AddRefed<mozilla::dom::KeyframeEffect> mozilla::dom::KeyframeEffectReadOnly::ConstructKeyframeEffect<mozilla::dom::KeyframeEffect, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions>(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) (ErrorResult.h:380, in XUL)
GECKO(20772) | #03: mozilla::dom::KeyframeEffect::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) (KeyframeEffect.cpp:65, in XUL)
GECKO(20772) | #04: mozilla::dom::Element::Animate(mozilla::dom::Nullable<mozilla::dom::ElementOrCSSPseudoElement> const&, JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) (AlreadyAddRefed.h:124, in XUL)
GECKO(20772) | #05: mozilla::dom::Element::Animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::UnrestrictedDoubleOrKeyframeAnimationOptions const&, mozilla::ErrorResult&) (Maybe.h:224, in XUL)
GECKO(20772) | #06: mozilla::dom::ElementBinding::animate(JSContext*, JS::Handle<JSObject*>, mozilla::dom::Element*, JSJitMethodCallArgs const&) (AlreadyAddRefed.h:146, in XUL)
GECKO(20772) | #07: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (BindingUtils.cpp:2956, in XUL)
GECKO(20772) | #08: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(20772) | #09: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:470, in XUL)
GECKO(20772) | #10: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(20772) | #11: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(20772) | #12: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(20772) | #13: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(20772) | #14: js::fun_apply(JSContext*, unsigned int, JS::Value*) (jsfun.cpp:1288, in XUL)
GECKO(20772) | #15: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(20772) | #16: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:470, in XUL)
GECKO(20772) | #17: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(20772) | #18: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(20772) | #19: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(20772) | #20: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(20772) | #21: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(20772) | #22: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(20772) | #23: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(20772) | #24: js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const (Wrapper.cpp:166, in XUL)
GECKO(20772) | #25: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const (CrossCompartmentWrapper.cpp:353, in XUL)
GECKO(20772) | #26: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (Proxy.cpp:479, in XUL)
GECKO(20772) | #27: js::proxy_Call(JSContext*, unsigned int, JS::Value*) (RootingAPI.h:818, in XUL)
GECKO(20772) | #28: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(20772) | #29: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:452, in XUL)
GECKO(20772) | #30: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(20772) | #31: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(20772) | #32: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(20772) | #33: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:521, in XUL)
GECKO(20772) | #34: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:410, in XUL)
GECKO(20772) | #35: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:488, in XUL)
GECKO(20772) | #36: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(20772) | #37: js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const (Wrapper.cpp:166, in XUL)
GECKO(20772) | #38: js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const (CrossCompartmentWrapper.cpp:353, in XUL)
GECKO(20772) | #39: js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) (Proxy.cpp:479, in XUL)
GECKO(20772) | #40: js::proxy_Call(JSContext*, unsigned int, JS::Value*) (RootingAPI.h:818, in XUL)
GECKO(20772) | #41: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:294, in XUL)
GECKO(20772) | #42: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:452, in XUL)
GECKO(20772) | #43: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (Interpreter.cpp:534, in XUL)
GECKO(20772) | #44: JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (jsapi.cpp:2891, in XUL)
GECKO(20772) | #45: mozilla::dom::Function::Call(JSContext*, JS::Handle<JS::Value>, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) (FunctionBinding.cpp:36, in XUL)
GECKO(20772) | #46: void mozilla::dom::Function::Call<nsCOMPtr<nsISupports> >(nsCOMPtr<nsISupports> const&, nsTArray<JS::Value> const&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) (FunctionBinding.h:72, in XUL)
GECKO(20772) | #47: nsGlobalWindow::RunTimeoutHandler(mozilla::dom::Timeout*, nsIScriptContext*) (nsGlobalWindow.cpp:13110, in XUL)
GECKO(20772) | #48: mozilla::dom::TimeoutManager::RunTimeout(mozilla::dom::Timeout*) (TimeoutManager.cpp:720, in XUL)
GECKO(20772) | #49: mozilla::dom::(anonymous namespace)::TimerCallback(nsITimer*, void*) (RefPtr.h:40, in XUL)
GECKO(20772) | #50: nsTimerImpl::Fire(int) (nsTimerImpl.cpp:489, in XUL)
GECKO(20772) | #51: nsTimerEvent::Run() (TimerThread.cpp:285, in XUL)
GECKO(20772) | #52: mozilla::ThrottledEventQueue::Inner::ExecuteRunnable() (atomic:987, in XUL)
GECKO(20772) | #53: mozilla::ThrottledEventQueue::Inner::Executor::Run() (ThrottledEventQueue.cpp:75, in XUL)
GECKO(20772) | #54: nsThread::ProcessNextEvent(bool, bool*) (Maybe.h:224, in XUL)
GECKO(20772) | #55: NS_ProcessPendingEvents(nsIThread*, unsigned int) (nsThreadUtils.cpp:335, in XUL)
GECKO(20772) | #56: nsBaseAppShell::NativeEventCallback() (nsBaseAppShell.cpp:98, in XUL)
GECKO(20772) | #57: nsAppShell::ProcessGeckoEvents(void*) (nsAppShell.mm:400, in XUL)
GECKO(20772) | #58: CFStringFold (in CoreFoundation) + 4209
GECKO(20772) | #59: -[_CFXNotificationRegistrar match:object:observer:enumerator:] (in CoreFoundation) + 1373
GECKO(20772) | #60: -[NSTaggedPointerString uppercaseStringWithLocale:] (in CoreFoundation) + 166
GECKO(20772) | #61: -[__NSDictionaryI countByEnumeratingWithState:objects:count:] (in CoreFoundation) + 100
GECKO(20772) | #62: RunCurrentEventLoopInMode (in HIToolbox) + 240
GECKO(20772) | #63: ReceiveNextEventCommon (in HIToolbox) + 432
GECKO(20772) | #64: _BlockUntilNextEventMatchingListInModeWithFilter (in HIToolbox) + 71
GECKO(20772) | #65: _DPSNextEvent (in AppKit) + 1120
GECKO(20772) | #66: -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] (in AppKit) + 2796
GECKO(20772) | #67: -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (nsAppShell.mm:130, in XUL)
GECKO(20772) | #68: -[NSApplication run] (in AppKit) + 926
GECKO(20772) | #69: nsAppShell::Run() (nsCOMPtr.h:551, in XUL)
GECKO(20772) | #70: nsAppStartup::Run() (nsAppStartup.cpp:283, in XUL)
GECKO(20772) | #71: XREMain::XRE_mainRun() (nsAppRunner.cpp:4550, in XUL)
GECKO(20772) | #72: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4730, in XUL)
GECKO(20772) | #73: XRE_main(int, char**, mozilla::BootstrapConfig const&) (nsAppRunner.cpp:4823, in XUL)
GECKO(20772) | #74: main (nsBrowserApp.cpp:236, in firefox)
TEST-INFO | Main app process: exit 1
(Assignee)

Updated

11 days ago
Assignee: nobody → ferjmoreno
Priority: -- → P2
I should note about this failure.

In this test case, we are using simulateComputeValuesFailure flag (the flag is only for testing purpose), and when the flag is set, we are storing a given string for shorthand property in nsCSSValue as a token stream if the given string can't be parsed.  Unfortunately in stylo, we don't use nsCSSValue anymore, so we should store the given string somewhere else in the case where simulateComputeValuesFailure is set.
Comment hidden (mozreview-request)

Comment 3

9 days ago
mozreview-review
Comment on attachment 8869395 [details]
Bug 1365674 - stylo: Simulate compute values failure for tests.

https://reviewboard.mozilla.org/r/141052/#review144778

Thanks for doing this!
I am very impressed that the patch came so quickly.

A couple of concerns:
1) The additional flag in PropertyDeclarationBlock
   PropertyDeclarationBlock is used everywhere, I don't think the additional flas is acceptable. The flag is only for testing purpose, if we have no other choise, we should enable it only for debug build (using #[cfg(debug_assertions)]?, I am not sure) and run this test only on debug build
2) The shorthand property value is not stored?
   It seems to me that this patch does not store the shorthand property value at all. It looks that it relies on missing keyframe values. I am inclined to drop simulateComputeValuesFailure flag....

Brian, what do you think?
Flags: needinfo?(bbirtles)
What if we just add a mSimulateComputeValueFailure flag to PropertyValuePair instead and use that for both Servo and Gecko?

That would mean reworking the Gecko side too but would be less invasive that modifying PropertyValueDeclarationBlock (and probably simpler than what we currently do with token stream values). We could potentially make the member debug-only and perhaps mark the corresponding tests as debug-only too if we're worried about swelling the size of PropertyValuePair.
Flags: needinfo?(bbirtles)
Thank you, Brian. PropertyValuePair looks a reasonable place to me.

Fernando, would you mind updating 8869395 to add mSimulateComputeValueFailure to PropertyValuePair and add an nsString to store the shorthand value string?  I'd say we should use the additional members only for debug build, but if it will be messy, I am OK with them for release build too.

Thank you!

Comment 6

7 days ago
mozreview-review
Comment on attachment 8869395 [details]
Bug 1365674 - stylo: Simulate compute values failure for tests.

https://reviewboard.mozilla.org/r/141052/#review144944

Clearing review request for now.
Attachment #8869395 - Flags: review?(hikezoe)
Fernando told me that the test_simulate_compute_values_failure.html does not call KeyframeEffectReadOnly::GetKeyframes() at all.  I thought the test uses GetKeyframes() but actually it uses GetProperties() instead.  Using GetProperties() means that we don't directly use the tokenStream in the test case (actually indirectly used?).  Do we really need to store the shorthand value string?  Where do we use the token stream value in case the case where mSimulateComputeValueFailure is true?  I might be something, but couldn't find any clues.
Flags: needinfo?(bbirtles)
Yeah, I don't think there's any need to preserve the original shorthand value for (simulated) failed values.
Flags: needinfo?(bbirtles)
You need to log in before you can comment on or make changes to this bug.