Remove abortReason property from WritableStream spec

[nidhijaju]

Based on #1165, this change removes the abortReason property from the WritableStream spec, as we are no longer implementing it, and will rather use the reason from the AbortSignal when that is added in the DOM spec.

@yutakahirano Does this look okay?

---

Preview | Diff

[yutakahirano]

LGTM.

This affects some WPTs. @domenic do you want to update the WPTs too? Or is it OK to wait for the DOM side change?

[MattiasBuelens]

Could you update the reference implementation too? (Don't forget the Web IDL! 😉)

Some other things to look at:

• We have an explainer about controller.signal in this repo. Maybe we should update it to remove .abortReason, or at least mention what happened to it?
• We've opened a bunch of implementation bugs for Define WritableStreamDefaultController.signal #1132. Once this PR lands, we should probably update those bugs to let implementers know that the spec has changed.

This affects some WPTs. @domenic do you want to update the WPTs too? Or is it OK to wait for the DOM side change?

Yes, I'd say we remove the asserts about controller.abortReason from the WPTs.

[nidhijaju]

Closing and reopening to trigger the CI again as I believe the wpt PR should have fixed the check failure.

[nidhijaju]

I'm running into some test failures related to the WritableStreamDefaultController interface, however I think those can only be fixed by updating https://github.com/web-platform-tests/wpt/blob/master/interfaces/streams.idl. However, IIUC that file will not be generated until the spec is updated, and the spec cannot be updated until the tests pass. What should be done in this case?

[MattiasBuelens]

You're right. This is quite the chicken or the egg situation! 😅

We could temporarily disable the Web IDL tests by adding idlharness.any.html here:

streams/reference-implementation/run-web-platform-tests.js

Lines 35 to 39 in ea03a24

	const excludeGlobs = [
	// These tests use ArrayBuffers backed by WebAssembly.Memory objects, which *should* be non-transferable.
	// However, our TransferArrayBuffer implementation cannot detect these, and will incorrectly "transfer" them anyway.
	'readable-byte-streams/non-transferable-buffers.any.html'
	];

Then we can land the spec change, wait for @webref/idl and WPT to pick up the new Web IDL, and finally roll WPT in this repo and re-enable the test.

@domenic Thoughts?

[MattiasBuelens]

This is quite scary though. Any spec change that affects the Web IDL won't get tested against the reference implementation until WPT has updated their Web IDL. So proposed additions like ReadableStream.from(asyncIterable) or ReadableStreamBYOBReader.fill(view) can't be fully tested before we land them. 😕

[domenic]

Oh, this is frustrating! Strange that we haven't run into it before.

I think the best thing to do is to manually update https://github.com/web-platform-tests/wpt/blob/master/interfaces/streams.idl (even though it says "generated content do not edit") as part of WPT updates. We should submit and merge a second PR there. Then we can roll the WPT here and do everything as normal.

Later, the bot will come along and overwrite our changes, but it should be a no-op since we're just making the changes ahead of the bot.

I will double-check with WPT infra folks that this sounds OK, but let's assume it's the way to go if you don't hear back from me soon.

[nidhijaju]

Thank you @domenic! I'll make the relevant WPT PR/changes on Monday. Please let me know by then if you think we should wait more.

Comments addressed!

[domenic]

I updated the https://bugzilla.mozilla.org/show_bug.cgi?id=1714341 and https://bugs.webkit.org/show_bug.cgi?id=226575 to let them know about the change.

[nidhijaju]

Thank you so much, @domenic! :)