Bug 212650 – ASSERTION FAILED: isCell() under WebCore::JSDOMConstructor seen with webaudio/the-audio-api/the-audiocontext-interface/audiocontextoptions.html

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audiocontext-interface/audiocontextoptions.html, imported with https://trac.webkit.org/changeset/262405/webkit, is consistently crashing on debug bots with the following:

ASSERTION FAILED: isCell()
/Volumes/Data/slave/catalina-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/JSCJSValueInlines.h(557) : JSC::JSCell *JSC::JSValue::asCell() const
1   0x3e5a304d9 WTFCrash
2   0x3c800296b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x3c80eda7b JSC::JSValue::asCell() const
4   0x3c831dd65 JSC::asObject(JSC::JSValue)
5   0x3c861e52b WebCore::JSDOMConstructor<WebCore::JSAudioContext>::construct(JSC::JSGlobalObject*, JSC::CallFrame*)
6   0x3e6e99c45 JSC::NativeFunction::operator()(JSC::JSGlobalObject*, JSC::CallFrame*)
7   0x3e6e99b22 JSC::TaggedNativeFunction::operator()(JSC::JSGlobalObject*, JSC::CallFrame*)
8   0x3e6f10c43 JSC::LLInt::handleHostCall(JSC::CallFrame*, JSC::JSValue, JSC::CodeSpecializationKind)
9   0x3e6f0fdd1 JSC::LLInt::setUpCall(JSC::CallFrame*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*)
10  0x3e6f01763 JSC::SlowPathReturnType JSC::LLInt::genericCall<JSC::OpConstruct>(JSC::CodeBlock*, JSC::CallFrame*, JSC::OpConstruct&&, JSC::CodeSpecializationKind, unsigned int)
11  0x3e6f015ff llint_slow_path_construct
12  0x3e5fb69cc llint_entry
13  0x3e5fb5b32 llint_entry
14  0x3e5fb5b32 llint_entry
15  0x3e5fb5b32 llint_entry
16  0x3e5fb5b32 llint_entry
17  0x3e5fb693b llint_entry
18  0x3e5fb6e4b llint_entry
19  0x3e5fb5a8f llint_entry
20  0x3e5fb5b32 llint_entry
21  0x3e5fb693b llint_entry
22  0x3e5fb5b32 llint_entry
23  0x3e5f95d33 vmEntryToJavaScript
24  0x3e6dc9d8b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
25  0x3e6dca54f JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
26  0x3e7146a7d JSC::call(JSC::JSGlobalObject*, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
27  0x3e7146d53 JSC::profiledCall(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::JSValue, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
28  0x3e7323482 JSC::JSMicrotask::run(JSC::JSGlobalObject*)
29  0x3ca5140ce WebCore::JSExecState::runTask(JSC::JSGlobalObject*, JSC::Microtask&)
30  0x3ca51b27b WebCore::JSMicrotaskCallback::call()
31  0x3ca51b0fd WebCore::JSDOMWindowBase::queueMicrotaskToEventLoop(JSC::JSGlobalObject&, WTF::Ref<JSC::Microtask, WTF::DumbPtrTraits<JSC::Microtask> >&&)::$_35::operator()()
LEAK: 1 WebPageProxy

https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fwebaudio%2Fthe-audio-api%2Fthe-audiocontext-interface%2Faudiocontextoptions.html

[Ryan Haddad]

<rdar://63886202>

[Radar WebKit Bug Importer]

<rdar://problem/63887102>

[Ryan Haddad]

Skipped the test for macOS/iOS debug in r262464 and r262465.

[Yusuke Suzuki]

Created attachment 400895 [details]
Patch

[Yusuke Suzuki]

Created attachment 400896 [details]
Patch

[Yusuke Suzuki]

I'll update binding tests results.

[Mark Lam]

Comment on attachment 400896 [details]
Patch

r=me.  Please rebase the bindings test results.

[Yusuke Suzuki]

imported/w3c/web-platform-tests/service-workers/service-worker/fetch-request-no-freshness-headers.https.html is failing on iOS without this. https://results.webkit.org/?suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fservice-workers%2Fservice-worker%2Ffetch-request-no-freshness-headers.https.html

[Yusuke Suzuki]

Committed r262479: <https://trac.webkit.org/changeset/262479>

[Chris Dumez]

Comment on attachment 400896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=400896&action=review

> Source/WebCore/ChangeLog:8
> +        Some DOM constructor can return jsNull. For example, AudioContext constructor can return jsNull when it exceeds # of hardware audio contexts.

This looks like a bug in the AudioContext implementation. This is not the specified behavior:
https://www.w3.org/TR/webaudio/#AudioContext-constructors

"If the AudioContext is not allowed to start, abort these steps."

So we're supposed to construct an AudioContext but it is not supposed to start playing.

I can understand if you don't want to fix the WebAudio implementation. But then why not simply add a new WebIDL attribute like [ConstructorMayReturnNull], and then the bindings generator would use toJS() instead of toJSNewlyCreated() in this case?

> Source/WebCore/ChangeLog:9
> +        However CodeGeneratorJS assumes that DOM constructor always returns an object, or throws an exception.

Yes, this was the whole idea behind toJSNewlyCreated() vs toJS(). This was an optimization to avoid the null check when the object was just constructed. It seems like this change is reverting the optimization.