Bug 212650 - ASSERTION FAILED: isCell() under WebCore::JSDOMConstructor seen with webaudio/the-audio-api/the-audiocontext-interface/audiocontextoptions.html
Summary: ASSERTION FAILED: isCell() under WebCore::JSDOMConstructor seen with webaudio...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-02 12:30 PDT by Ryan Haddad
Modified: 2020-06-03 08:19 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.23 KB, patch)
2020-06-02 22:10 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (5.22 KB, patch)
2020-06-02 22:11 PDT, Yusuke Suzuki
mark.lam: review+
mark.lam: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.