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

69 views
Skip to first unread message

Austin Sullivan

unread,
Jan 19, 2021, 2:19:42 PMJan 19
to blin...@chromium.org

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

unread,
Jan 19, 2021, 2:24:19 PMJan 19
to Austin Sullivan, blin...@chromium.org
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

unread,
Jan 21, 2021, 8:31:22 AMJan 21
to Domenic Denicola, Austin Sullivan, blin...@chromium.org
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

unread,
Jan 21, 2021, 8:56:47 AMJan 21
to Mike West, Domenic Denicola, Austin Sullivan, blin...@chromium.org
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 :)

Chris Harrelson

unread,
Jan 21, 2021, 11:24:25 AMJan 21
to Yoav Weiss, Mike West, Domenic Denicola, Austin Sullivan, blin...@chromium.org
Reply all
Reply to author
Forward
0 new messages