Intent to Implement and Ship: Remove FileSystemWritableFileStream::close() implementation

[Austin Sullivan]

Contact emails

asu...@chromium.org

m...@chromium.org

Explainer

None

Specification

https://wicg.github.io/file-system-access/#filesystemwritablefilestream

Summary

This just overrides the (identical behaving) implementation from the base WritableStream class. Remove it from FileSystemWritableFileStream. WritableStream didn't used to have a close() method, as such FileSystemWritableFileStream implemented its own. Later WritableStream added one, but we forgot to remove the implementation of the override before shipping the API. This fixes that inconsistency between the spec and implementation.

Blink component

Blink>Storage>FileSystem

TAG review

https://github.com/w3ctag/design-reviews/issues/390

TAG review status

This change makes the implementation align with the spec which was TAG reviewed for the File System Access API.

Risks

Interoperability and Compatibility

This change should be harmless. The current implementation just overrides the (identical behaving) implementation from the base WritableStream class. We'll now be using the base WritableStream class's close() method directly.

Gecko: No signal

Edge: No signal

WebKit: No signal

Web developers: No signals

Is this feature fully tested by web-platform-tests?

Yes

Tracking bug

https://crbug.com/1163351

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/5747310233387008

This intent message was generated by Chrome Platform Status.

[Domenic Denicola]

In case it helps the API owners, this change is technically web-exposed, but very subtle and hard to notice. Code would have to be doing something like:

FileSystemWritableFileStream.prototype.hasOwnProperty('close')
// true before this change; false after

FileSystemWritableFileStream.prototype.close === WritableStream.prototype.close
// false before this change; true after

FileSystemWritableFileStream.prototype.close.call(nonFSWritableStream)
// throws before this change; works afterward

(Maybe this could have been a bugfix PSA instead of an Intent to Ship, but erring on the side of getting approval never hurts IMO.)

[Mike West]

I agree with Domenic that this is minor enough to have warranted a PSA (or, really, just landing a bugfix CL). But, since you asked, LGTM1! :)

-mike

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/MN2PR13MB32327C2C0772B6915AA3A901DFA39%40MN2PR13MB3232.namprd13.prod.outlook.com.

[Yoav Weiss]

I was the one asking for the intent here, as it wasn't immediately clear to me from the CL that this is safe to remove.

Given the explanations, LGTM2 :)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKXHy%3Dfdk4hcoYXRhEAR7HA_zmJ8vFBwj_nbUAC-6bFBACtL6g%40mail.gmail.com.

[Chris Harrelson]

LGTM3

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CACj%3DBEjEb97HqvnG%2B0ENud4zCq-9FHO0s-VV_XYDh%2B_C4sWayw%40mail.gmail.com.