1507952 - stream.tee() shouldn't touch Object.prototype

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Jason Orendorff [:jorendorff]]

Object.prototype.start = function () { throw "FAIL"; };
let source = Object.create(null);
new ReadableStream(source).tee()

The spec has changed since we implemented it. ReadableStreamTee() longer involves creating an underlying source object and passing it to the ReadableStream constructor, so there are no property accesses and Object.prototype.start should not be accessed.

This causes a few test failures in testing/web-platform/tests/streams/readable-streams/general*.html .

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 1: Rename ReadableStream::createStream() to create() and make it our implementation of 3.3.5. r?arai

The general approach of this stack is to update our code to the current
standard and let the bug fixes fall out of that, as a side effect.

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 2: Rewrite ReadableStream constructor to match the standard. r?arai

We were very close to compliance, but all the step numbers have changed and
some user-visible behavior around default arguments was a bit off.

Also, update step numbers in ValidateAndNormalizeHighWaterMark, implement
MakeSizeAlgorithmFromSizeFunction, and generally validate size/highWaterMark
arguments earlier.

Depends on D12454

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 3: Move and rename CreateReadableStreamDefaultController. r?arai

The body of the function is unchanged.

Depends on D12456

Comment 4

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 4: Update SetUpReadableStreamDefaultController to match the standard. r?arai

Depends on D12457

Comment 5

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 5: stream.tee() should not try to call a .start() method. r?arai

This fix is not particularly clean, but at least it's possible to see why the
if statement is there, now that the comments explain how we (imperfectly)
implement the {start,pull,cancel}Algorithm arguments.

Depends on D12458

Comment 6

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1507952 - Part 6: Implement Streams spec 3.3.3. CreateReadableStream. r?arai

This was previously approximated by ReadableStream::createDefaultStream.

Depends on D12459

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc904523f5a3
Part 1: Rename ReadableStream::createStream() to create() and make it our implementation of 3.3.5. r=arai
https://hg.mozilla.org/integration/autoland/rev/5a0df7634eaf
Part 2: Rewrite ReadableStream constructor to match the standard. r=arai
https://hg.mozilla.org/integration/autoland/rev/7687d5a601cb
Part 3: Move and rename CreateReadableStreamDefaultController. r=arai
https://hg.mozilla.org/integration/autoland/rev/51f31e8ac73a
Part 4: Update SetUpReadableStreamDefaultController to match the standard. r=arai
https://hg.mozilla.org/integration/autoland/rev/f02a0ed71bca
Part 5: stream.tee() should not try to call a .start() method. r=arai
https://hg.mozilla.org/integration/autoland/rev/1df56440bdb4
Part 6: Implement Streams spec 3.3.3. CreateReadableStream. r=arai

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/dc904523f5a3
https://hg.mozilla.org/mozilla-central/rev/5a0df7634eaf
https://hg.mozilla.org/mozilla-central/rev/7687d5a601cb
https://hg.mozilla.org/mozilla-central/rev/51f31e8ac73a
https://hg.mozilla.org/mozilla-central/rev/f02a0ed71bca
https://hg.mozilla.org/mozilla-central/rev/1df56440bdb4