Bug 222228

Summary: Regression(r268700) postMessage changes prototype of basic types
Product: WebKit Reporter: Stefan Sechelmann <stefan>
Component: WebCore JavaScriptAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Critical CC: cdumez, darin, esprehn+autocc, ews-watchlist, fpizlo, ggaren, hi, joepeck, kangil.han, keith_miller, webkit-bug-importer, ysuzuki
Priority: P4 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Mac (Intel)   
OS: macOS 11   
Bug Depends on:    
Bug Blocks: 217912    
Attachments:
Description Flags
Patch none

Stefan Sechelmann
Reported 2021-02-20 02:32:42 PST
Execute this in the current Safari Tech Preview (120): >window.addEventListener('message', event => { > console.log('is Object after postMessage?: ', event.data instanceof Object) > console.log('is Array after postMessage?: ', event.data.array instanceof Array) >}) >const testObject = { > array: [ 1, 2, 3, 4 ] >}; >console.log('is Object before postMessage?: ', testObject instanceof Object) >console.log('is Array before postMessage?: ', testObject.array instanceof Array) >window.postMessage(testObject); This probably extends to other build-in types like Map, Set, ArrayBuffer and the like... This applies to Safari Tech Preview 120 and is not present in Safari 14.0.3.
Attachments
Patch (8.72 KB, patch)
2021-02-24 13:01 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Stefan Sechelmann
Comment 1 2021-02-20 02:38:37 PST
I experienced this previously when dealing with data read from IndexedDB in an iframe: bug 185906 and bug 182097.
Stefan Sechelmann
Comment 2 2021-02-20 02:40:16 PST
Here is a demo: https://codepen.io/sechel/pen/xxRLPKp Check the dev console.
Radar WebKit Bug Importer
Comment 3 2021-02-22 14:02:12 PST
<rdar://problem/74612853>
Yusuke Suzuki
Comment 4 2021-02-22 15:04:23 PST
This is Structured Cloning related, so WebCore.
Chris Dumez
Comment 5 2021-02-24 09:53:27 PST
Indeed, I see this in STP 120: [Log] is Object before postMessage?: – true (iframeConsoleRunner-7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) [Log] is Array before postMessage?: – true (iframeConsoleRunner-7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) [Log] is Object after postMessage?: – false (iframeConsoleRunner-7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) [Log] is Array after postMessage?: – false (iframeConsoleRunner-7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) and this in my System Safari: [Log] is Object before postMessage?: – true (xxRLPKp, line 52) [Log] is Array before postMessage?: – true (xxRLPKp, line 53) [Log] is Object after postMessage?: – true (xxRLPKp, line 45) [Log] is Array after postMessage?: – true (xxRLPKp, line 46) I will bisect.
Chris Dumez
Comment 6 2021-02-24 10:14:55 PST
(In reply to Chris Dumez from comment #5) > Indeed, I see this in STP 120: > [Log] is Object before postMessage?: – true > (iframeConsoleRunner- > 7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) > [Log] is Array before postMessage?: – true > (iframeConsoleRunner- > 7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) > [Log] is Object after postMessage?: – false > (iframeConsoleRunner- > 7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) > [Log] is Array after postMessage?: – false > (iframeConsoleRunner- > 7f4d47902dc785f30dedcac9c996b9f31d4dfcc33567cc48f0431bc918c2bf05.js, line 1) > > and this in my System Safari: > [Log] is Object before postMessage?: – true (xxRLPKp, line 52) > [Log] is Array before postMessage?: – true (xxRLPKp, line 53) > [Log] is Object after postMessage?: – true (xxRLPKp, line 45) > [Log] is Array after postMessage?: – true (xxRLPKp, line 46) > > I will bisect. Never mind, I am not actually seeing a recent regression. The difference I saw was before and after a reload. If I load https://codepen.io/sechel/pen/xxRLPKp, I see: [Log] is Object before postMessage?: – true [Log] is Array before postMessage?: – true [Log] is Object after postMessage?: – true [Log] is Array after postMessage?: – true If I press the reload button, I see: [Log] is Object before postMessage?: – true [Log] is Array before postMessage?: – true [Log] is Object after postMessage?: – false [Log] is Array after postMessage?: – false I see this behavior in STP 120 as well as in older WebKit builds...
Chris Dumez
Comment 7 2021-02-24 10:29:03 PST
Ok, looks like I had not gone far back enough. r267039: GOOD r269041: BAD In the good build, results are good even after the reload. It smells like a JSC bug to me but I guess we'll see after bisection.
Chris Dumez
Comment 8 2021-02-24 10:59:15 PST
Regression range: https://trac.webkit.org/log/webkit/trunk?mode=follow_copy&rev=268700&stop_rev=268698 Almost certainly from http://trac.webkit.org/r268700, which I wrote :S Investigating..
Chris Dumez
Comment 9 2021-02-24 11:47:49 PST
Seems it was caused by this part of the change: JSC::JSGlobalObject* ScriptExecutionContext::globalObject() { - if (is<Document>(*this)) { - Document& document = downcast<Document>(*this); - auto* frame = document.frame(); - return frame ? frame->script().globalObject(mainThreadNormalWorld()) : nullptr; - } + if (is<Document>(*this)) + return WebCore::globalObject(mainThreadNormalWorld(), downcast<Document>(*this).page());
Chris Dumez
Comment 10 2021-02-24 11:50:24 PST
(In reply to Chris Dumez from comment #9) > Seems it was caused by this part of the change: > > JSC::JSGlobalObject* ScriptExecutionContext::globalObject() > { > - if (is<Document>(*this)) { > - Document& document = downcast<Document>(*this); > - auto* frame = document.frame(); > - return frame ? > frame->script().globalObject(mainThreadNormalWorld()) : nullptr; > - } > + if (is<Document>(*this)) > + return WebCore::globalObject(mainThreadNormalWorld(), > downcast<Document>(*this).page()); Oh, I think the issue is that WebCore::globalObject() ends up using the main frame's scriptController instead of the current frame's scriptController. Will fix shortly.
Chris Dumez
Comment 11 2021-02-24 12:34:15 PST
I am struggling a bit to write a layout test that reproduces the issue. What seems to trigger the bug in Safari is to have Web Inspector open when the test is running.
Chris Dumez
Comment 12 2021-02-24 13:01:50 PST
Created attachment 421445 [details] Patch
Geoffrey Garen
Comment 13 2021-02-24 13:35:01 PST
Comment on attachment 421445 [details] Patch r=me
EWS
Comment 14 2021-02-24 14:03:45 PST
Committed r273438: <https://commits.webkit.org/r273438> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421445 [details].
Stefan Sechelmann
Comment 15 2021-02-24 22:54:37 PST
Thank you.
Note You need to log in before you can comment on or make changes to this bug.