Bug 212650

Summary: ASSERTION FAILED: isCell() under WebCore::JSDOMConstructor seen with webaudio/the-audio-api/the-audiocontext-interface/audiocontextoptions.html
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, mark.lam, rniwa, webkit-bot-watchers-bugzilla, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+, mark.lam: commit-queue-

Description Ryan Haddad 2020-06-02 12:30:47 PDT 
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
Comment 1 Ryan Haddad 2020-06-02 12:30:59 PDT 
<rdar://63886202>
Comment 2 Radar WebKit Bug Importer 2020-06-02 12:31:23 PDT 
<rdar://problem/63887102>
Comment 3 Ryan Haddad 2020-06-02 17:47:26 PDT 
Skipped the test for macOS/iOS debug in r262464 and r262465.
Comment 4 Yusuke Suzuki 2020-06-02 22:10:28 PDT 
Created attachment 400895 [details]
Patch
Comment 5 Yusuke Suzuki 2020-06-02 22:11:33 PDT 
Created attachment 400896 [details]
Patch
Comment 6 Yusuke Suzuki 2020-06-02 22:16:33 PDT 
I'll update binding tests results.
Comment 7 Mark Lam 2020-06-02 22:17:49 PDT 
Comment on attachment 400896 [details]
Patch

r=me.  Please rebase the bindings test results.
Comment 8 Yusuke Suzuki 2020-06-02 23:49:56 PDT 
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
Comment 9 Yusuke Suzuki 2020-06-02 23:53:23 PDT 
Committed r262479: <https://trac.webkit.org/changeset/262479>
Comment 10 Chris Dumez 2020-06-03 08:19:31 PDT 
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.