Bug 235456

Summary: jsc_fuz/wktr: crash with new XRReferenceSpaceEvent('', {referenceSpace})
Product: WebKit Reporter: Gabriel Nava Marino <gnavamarino>
Component: WebXRAssignee: Gabriel Nava Marino <gnavamarino>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, esprehn+autocc, ews-watchlist, kondapallykalyan, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=235756
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Gabriel Nava Marino 2022-01-21 15:08:14 PST
FastMalloc.h specifies you need to annotate each derived class with WTF_MAKE_ISO_ALLOCATED if your base class is annotated with WTF_MAKE_ISO_ALLOCATED. After doing this, the crash in WebCore::Event::operator new(unsigned long) is no longer happening.
Comment 1 Gabriel Nava Marino 2022-01-21 15:09:16 PST
<rdar://problem/71708005>
Comment 2 Gabriel Nava Marino 2022-01-21 15:21:31 PST
Created attachment 449701 [details]
Patch
Comment 3 Gabriel Nava Marino 2022-01-26 10:26:02 PST
The proposed patch exposed an assertion being hit in mac-debug-wk1

Application Specific Information:
CRASHING TEST: webxr/xr-reference-space-event-crash.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000004a2540f4e WTFCrash + 14
1   com.apple.WebCore             	0x000000047cc6a2bb WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x000000047f8197ea WebCore::XRReferenceSpaceEvent::XRReferenceSpaceEvent(WTF::AtomString const&, WebCore::XRReferenceSpaceEvent::Init const&, WebCore::EventIsTrusted) + 362
3   com.apple.WebCore             	0x000000047f819673 WebCore::XRReferenceSpaceEvent::XRReferenceSpaceEvent(WTF::AtomString const&, WebCore::XRReferenceSpaceEvent::Init const&, WebCore::EventIsTrusted) + 51
4   com.apple.WebCore             	0x000000047f81961b WebCore::XRReferenceSpaceEvent::create(WTF::AtomString const&, WebCore::XRReferenceSpaceEvent::Init const&, WebCore::EventIsTrusted) + 75
5   com.apple.WebCore             	0x000000047ebaaa90 WebCore::JSDOMConstructor<WebCore::JSXRReferenceSpaceEvent>::construct(JSC::JSGlobalObject*, JSC::CallFrame*) + 1184

I am taking a look.
Comment 4 Gabriel Nava Marino 2022-01-26 13:42:01 PST
Created attachment 450065 [details]
Patch
Comment 5 Chris Dumez 2022-01-26 13:48:25 PST
Comment on attachment 450065 [details]
Patch

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

The changes look good but I think the test should be improved.

> LayoutTests/webxr/xr-reference-space-event-crash.html:3
> +  navigator.xr.requestSession('inline')

Looks like this test is doing work asynchronously, therefore, I would:

> LayoutTests/webxr/xr-reference-space-event-crash.html:6
> +      new XRReferenceSpaceEvent('', {referenceSpace})

call this right after:
```
if (window.testRunner)
  testRunner.notifyDone();
```

Also, I recommend adding some validation of the event. In particular, make sure that event.referenceSpace is referenceSpace and that event.transform is null.

> LayoutTests/webxr/xr-reference-space-event-crash.html:10
> +      testRunner.dumpAsText();

call testRunner.waitUntilDone() too
Comment 6 Gabriel Nava Marino 2022-01-26 15:10:35 PST
Created attachment 450074 [details]
Patch
Comment 7 Chris Dumez 2022-01-26 15:11:23 PST
Comment on attachment 450074 [details]
Patch

r=me
Comment 8 EWS 2022-01-27 07:37:35 PST
Committed r288672 (246478@main): <https://commits.webkit.org/246478@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450074 [details].