Crash under JSC::JSCell::toNumber(JSC::ExecState*): Thread 0 name: Dispatch queue: com.apple.main-thread Thread 0 Crashed ↩: 0 JavaScriptCore 0x000000018a49e37c JSC::JSCell::toNumber(JSC::ExecState*) const + 0 (JSCell.cpp:175) 1 WebCore 0x000000018bc783d0 WebCore::DOMMatrixInit WebCore::convertDictionary<WebCore::DOMMatrixInit>(JSC::ExecState&, JSC::JSValue) + 4344 (JSCJSValueInlines.h:728) 2 WebCore 0x000000018bc99e44 WebCore::jsDOMPointReadOnlyPrototypeFunctionMatrixTransform(JSC::ExecState*) + 232 (JSDOMConvertDictionary.h:42) 3 ??? 0x0000000dded3c240 0 + 59572994624 4 JavaScriptCore 0x000000018a4fe4a0 llint_entry + 28848 5 JavaScriptCore 0x000000018a4fe4a0 llint_entry + 28848 6 JavaScriptCore 0x000000018a4fe4a0 llint_entry + 28848 7 JavaScriptCore 0x000000018a4fe4a0 llint_entry + 28848 8 JavaScriptCore 0x000000018a4fe4a0 llint_entry + 28848 9 JavaScriptCore 0x000000018a4fe89c llint_entry + 29868 10 JavaScriptCore 0x000000018a4f7220 vmEntryToJavaScript + 272 11 JavaScriptCore 0x000000018aaa3ad4 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 184 (JITCode.cpp:81) 12 JavaScriptCore 0x000000018a40111c JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 464 (Interpreter.cpp:1028) 13 WebCore 0x000000018c7472c4 WebCore::HTMLMediaElement::didAddUserAgentShadowRoot(WebCore::ShadowRoot&) + 1132 (HTMLMediaElement.cpp:7177) 14 WebCore 0x000000018c5cc3f4 WebCore::Element::addShadowRoot(WTF::Ref<WebCore::ShadowRoot, WTF::DumbPtrTraits<WebCore::ShadowRoot> >&&) + 380 (Element.cpp:1798) 15 WebCore 0x000000018b9a1ddc WebCore::Element::ensureUserAgentShadowRoot() + 80 (Element.cpp:1893) 16 WebCore 0x000000018c730660 WebCore::HTMLMediaElement::configureMediaControls() + 532 (HTMLMediaElement.cpp:4279) 17 WebCore 0x000000018c5824a4 WebCore::ContainerNode::appendChildWithoutPreInsertionValidityCheck(WebCore::Node&) + 960 (ContainerNode.cpp:200) 18 WebCore 0x000000018c584798 WebCore::ContainerNode::appendChild(WebCore::Node&) + 80 (ContainerNode.cpp:672) 19 WebCore 0x000000018b99311c WebCore::jsNodePrototypeFunctionAppendChild(JSC::ExecState*) + 236 (JSNode.cpp:851)
<rdar://problem/37694346>
Created attachment 334301 [details] Patch
Comment on attachment 334301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334301&action=review > Source/WebCore/ChangeLog:15 > + No new tests, extended new test. Did you mean "extended existing test"? > Source/WebCore/bindings/js/JSDOMConvertNumbers.h:388 > static JSC::JSValue convert(Type value) > { > - return JSC::jsNumber(value); > + return JSC::jsNumber(JSC::purifyNaN(value)); > } If you look in JSCJSValue.h, you'll see that jsNumber() can also take integer values. Is IDLUnrestrictedDouble::ImplementationType guaranteed to be a double? If not, then maybe you should have 2 versions of convert: one for integral types and one for double. > Source/WebCore/testing/TypeConversions.h:147 > + double testImpureNaNUnrestrictedDouble() const { return bitwise_cast<double>(0xffff000000000000ll); } > + double testImpureNaN2UnrestrictedDouble() const { return bitwise_cast<double>(0x7ff8000000000001ll); } > + double testQuietNaNUnrestrictedDouble() const { return std::numeric_limits<double>::quiet_NaN(); } Maybe you should add a case for PureNaN as well just to make sure that it is also working as expected.
Comment on attachment 334301 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334301&action=review >> Source/WebCore/ChangeLog:15 >> + No new tests, extended new test. > > Did you mean "extended existing test"? Yep. Will fix. >> Source/WebCore/bindings/js/JSDOMConvertNumbers.h:388 >> } > > If you look in JSCJSValue.h, you'll see that jsNumber() can also take integer values. Is IDLUnrestrictedDouble::ImplementationType guaranteed to be a double? If not, then maybe you should have 2 versions of convert: one for integral types and one for double. AFAICT IDLUnrestrictedDouble::ImplementationType has to be a double. >> Source/WebCore/testing/TypeConversions.h:147 >> + double testQuietNaNUnrestrictedDouble() const { return std::numeric_limits<double>::quiet_NaN(); } > > Maybe you should add a case for PureNaN as well just to make sure that it is also working as expected. Good point, will add.
Created attachment 334304 [details] Patch
Comment on attachment 334304 [details] Patch r=me if EWS bots are happy.
Comment on attachment 334304 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334304&action=review > Source/WebCore/bindings/js/JSDOMConvertNumbers.h:363 > + ASSERT(!std::isnan(value)); I will add this assertion in a follow-up because it hits for the following test: LayoutTests/media/track/track-add-remove-cue.html Seems to be related to VTTCue.startTime where we do not match the spec and sometimes return NaN.
Created attachment 334313 [details] Patch
Comment on attachment 334313 [details] Patch Clearing flags on attachment: 334313 Committed r228851: <https://trac.webkit.org/changeset/228851>
All reviewed patches have been landed. Closing bug.