Write test for Streams global selection
Categories
(Core :: DOM: Streams, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox98 | --- | fixed |
People
(Reporter: mgaudet, Assigned: mgaudet)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
This test had a fair amount of work put into it, it would be bad to lose it if it still provides value.
|
Assignee | |
Comment 1•2 years ago
|
||
Hmm.
Some assumptions from this test may no longer be correct.
The test assumes that the following holds;
otherGlobal = newGlobal({ wantGlobalProperties: ["ReadableStream"] });
let OtherReadableStream = otherGlobal.ReadableStream;
let ReadableStreamReader = new ReadableStream().getReader().constructor;
let stream = new ReadableStream();
let otherReader = OtherReadableStream.prototype.getReader.call(stream);
Assert.equal(otherReader instanceof ReadableStreamReader, false, "because the global is different");
However, this isn't true in our current DOM streams implementation.
Pasting the following into Safari & Chrome, and we get varying results:
// I'm 99% sure creating a same origin iframe creates a new global, and so same as newGlobal above.
var iframe = document.createElement("iframe")
iframe.src = window.location.href
document.body.append(iframe)
let ReadableStreamReader = new ReadableStream().getReader().constructor;
let OtherReadableStream = iframe.contentWindow.ReadableStream
let stream = new ReadableStream();
let otherReader = OtherReadableStream.prototype.getReader.call(stream);
// False in Safari Firefox (with JS Streams) and
// True in Chrome and Firefox with DOM streams
console.log(otherReader instanceof ReadableStreamReader)
The reason this returns true in DOM Streams in firefox right now is that we borrow the parent of the given stream object when acquiring a reader. Since stream in this test case is from our current global, otherReader therefore is also from our current global.
I'm not sure how OtherReader.prototype.getReader.call(stream) switches globals in our current JS streams implementation; something I'm not understanding about .call?
|
|
Assignee | |
Comment 2•2 years ago
|
||
Hmm. Ok. It looks like we're supposed to switch to the called function's realm during calling according to the JS Spec:
- Function.prototype.call, Step 4, ?Call(fun, thisArg, args)
- Call abstract operation
- [[Call]] internal method of a Function Object, step 2, PrepareForOrdinaryCall.
- PrepareForOrdinaryCall step 5. says "Set the Realm of calleeContext to calleeRealm"
So I guess the question here is: Are WebIDL methods like ReadableStream.prototype.getReader ECMAScript Function Objects, or do they obey some other standard?
Olli: Is there a normal pattern to handle this I'm missing?
|
|
Assignee | |
Comment 3•2 years ago
|
||
Side note: Experimentally, I tried changing AcquireReadableStreamDefaultReader to instead create the reader with the incumbent global returned by GetIncumbentGlobal: no success.
|
|
Assignee | |
Comment 4•2 years ago
|
||
(Given that there's disagreement on the web about what the correct behaviour is, I don't currently consider this a blocker to shipping)
|
|
Assignee | |
Comment 5•2 years ago
|
||
It seems that the general expectation is that what we're currently doing for DOM streams is correct.
There will potentially be some followup work to clarify in the specification, but we can follow in testing the Chrome behaviour.
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 6•2 years ago
|
||
(This will also hopefully be clarified and made explicit by https://github.com/whatwg/streams/issues/1213)
|
|
Assignee | |
Comment 7•2 years ago
|
||
This test is based on the general structure of
js/src/tests/non262/ReadableStream/readable-stream-globals.js, however the
expectations have been commented and updated to match what we believe is
the correct WebIDL behaviour.
Not all the features of that test case have been ported over, as there's
some stuff around the handling of Symbol.species that we can test elsewhere,
and it attempts to test BYOB streams, which we have yet to implement.
Note: I tried to initially write this as an xpcshell test case using
Cu.sandbox; but I found that I wasn't geting global behaviour that matched
my expectations from testing on the web, hence why this was recreated as
a mochitest instead.
|
||
Updated•2 years ago
|
|
|
||
Updated•2 years ago
|
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00d4eefcf59b Add WPT Test for Stream Object globals r=smaug,saschanaz
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/32649 for changes under testing/web-platform/tests
|
||
Comment 10•2 years ago
|
||
Backed out for causing wpt failures at global.html.
Backout link: https://hg.mozilla.org/integration/autoland/rev/b2748326b111b8ea2791159ca264732d7310ea84
Failure log: https://treeherder.mozilla.org/logviewer?job_id=366384143&repo=autoland&lineNumber=4533
Upstream PR was closed without merging
|
||
Comment 12•2 years ago
|
||
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7b75650e5e0b Add WPT Test for Stream Object globals r=smaug,saschanaz
|
||
Comment 13•2 years ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
|
||
Comment 15•2 years ago
|
||
Unfortunately it seems like the test never made it upstream. https://github.com/web-platform-tests/wpt/commit/da15929d2adb3cd7889264411c0df5a7fb0cc845
|
||
Comment 16•2 years ago
|
||
I think what happened here is that the bot got confused by the backout being for multiple bugs, and so applied both that and this to upstream in the wrong order. https://github.com/web-platform-tests/wpt/pull/33274 reverts the backout.
Description
•