Bug 235456 - jsc_fuz/wktr: crash with new XRReferenceSpaceEvent('', {referenceSpace})
Summary: jsc_fuz/wktr: crash with new XRReferenceSpaceEvent('', {referenceSpace})
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebXR (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gabriel Nava Marino
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-21 15:08 PST by Gabriel Nava Marino
Modified: 2022-01-27 23:30 PST (History)
7 users (show)

See Also:


Attachments
Patch (3.92 KB, patch)
2022-01-21 15:21 PST, Gabriel Nava Marino
no flags Details | Formatted Diff | Diff
Patch (6.37 KB, patch)
2022-01-26 13:42 PST, Gabriel Nava Marino
no flags Details | Formatted Diff | Diff
Patch (6.94 KB, patch)
2022-01-26 15:10 PST, Gabriel Nava Marino
no flags Details | Formatted Diff | Diff

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