Cursor support for FileSystemSyncAccessHandle #76

jesup · 2022-11-30T03:27:17Z

Adds cursor to FileSystemSyncAccessHandle, similar to the cursor for WritableFileStream. This means that supplying an 'at:' parameter for every read and write will not be needed.

At least two implementers are interested (and none opposed):
- Mozilla
- Google
Tests are written and can be reviewed and commented upon at:
- https://phabricator.services.mozilla.com/D163399
Implementation bugs are filed:
- Chromium: https://crbug.com/1394790
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1803241
- WebKit: …
MDN issue is filed: …

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

index.bs

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

index.bs

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

jesup · 2022-11-30T20:52:04Z

Thanks for the careful review, @annevk !

index.bs

a-sully · 2022-12-01T00:39:01Z

index.bs

@@ -1048,6 +1048,8 @@ as well as obtaining and changing the size of, a single file.
 A {{FileSystemSyncAccessHandle}} offers synchronous methods. This allows for higher performance on
 contexts where asynchronous operations come with high overhead, e.g., WebAssembly.

+A {{FileSystemWritableFileStream}} has a <dfn for="FileSystemWritableFileStream">file position cursor</dfn> initialized at byte offset 0 from the top of the file.


Both a FileSystemSyncAccessHandle and FileSystemWritableFileStream should have a cursor, right? And does this need to be explicitly initialized to 0 in the "create a new FileSystemSyncAccessHandle" algorithm below?

It's usually fine to state the default value of an internal field upfront like is done here.

index.bs

Co-authored-by: Austin Sullivan <asully@google.com>

jesup · 2022-12-01T18:53:02Z

Note that the existing cursor for WritableFileStream should have normative text for [=FileSystemWritableFileStream/file position cursor=]; right now all references are in non-normative text. I'm considering this a separate issue

a-sully

LGTM apart from one question, but I'd wait for the +1 from Anne before merging. Thanks for putting this together!

index.bs

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

index.bs

a-sully · 2023-01-04T04:50:39Z

Question: with this change, should FileSystemReadWriteOptions.at become optional rather than defaulting to 0? Otherwise the behavior for read and write calls using default options is different than if you call the version of the method that does not specify any options

annevk · 2023-01-04T08:42:55Z

Filed #81.

Specified at whatwg/fs#76 This only makes sense if the "at" option is optional, so that was later specified here: whatwg/fs#82 Fixed: 1394790 Change-Id: I51e7343d221c1b89dfb10e22434d9d213e152b2d

Specified at whatwg/fs#76 This only makes sense if the "at" option is optional, so that was later specified here: whatwg/fs#82 Fixed: 1394790 Change-Id: I51e7343d221c1b89dfb10e22434d9d213e152b2d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4137091 Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: Austin Sullivan <asully@chromium.org> Reviewed-by: Daseul Lee <dslee@chromium.org> Cr-Commit-Position: refs/heads/main@{#1093907}

Specified at whatwg/fs#76 This only makes sense if the "at" option is optional, so that was later specified here: whatwg/fs#82 Fixed: 1394790 Change-Id: I51e7343d221c1b89dfb10e22434d9d213e152b2d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4137091 Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: Austin Sullivan <asully@chromium.org> Reviewed-by: Daseul Lee <dslee@chromium.org> Cr-Commit-Position: refs/heads/main@{#1093907}

…ndles, a=testonly Automatic update from web-platform-tests FSA: Implement a cursor for SyncAccessHandles Specified at whatwg/fs#76 This only makes sense if the "at" option is optional, so that was later specified here: whatwg/fs#82 Fixed: 1394790 Change-Id: I51e7343d221c1b89dfb10e22434d9d213e152b2d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4137091 Auto-Submit: Austin Sullivan <asully@chromium.org> Commit-Queue: Austin Sullivan <asully@chromium.org> Reviewed-by: Daseul Lee <dslee@chromium.org> Cr-Commit-Position: refs/heads/main@{#1093907} -- wpt-commits: ad88d36028e1564673f3d34b99862c1155da81ab wpt-pr: 37764

jesup added 3 commits November 29, 2022 13:47

Add cursor for FileSystemSyncAccessHandle

3556117

Add to acknowledgements

7d09bdb

Update file cursor in truncate()

907d424

jesup changed the title ~~Cursor for syncaccesshandle~~ Cursor support for FileSystemSyncAccessHandle Nov 30, 2022

jesup mentioned this pull request Nov 30, 2022

SyncAccessHandle read() & write() and the use of "at:" #49

Closed

annevk reviewed Nov 30, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

jesup and others added 3 commits November 30, 2022 07:58

Update index.bs

24a3101

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

2b91ed4

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

2767243

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

annevk reviewed Nov 30, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Complete converting the references to cursor

b592e1a

annevk reviewed Nov 30, 2022

View reviewed changes

jesup and others added 5 commits November 30, 2022 15:50

Update index.bs

73235c7

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

58d1386

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

1521557

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

827f67e

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Update index.bs

31a9fa8

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

jesup added 3 commits November 30, 2022 16:02

Clean up how the cursor is updated in write()

3a54aa7

Update->Set for cursor update

297d13d

Move Jesup to Acknowledgements

3fba048

a-sully reviewed Dec 1, 2022

View reviewed changes

jesup and others added 5 commits December 1, 2022 13:33

Update index.bs

ad51528

Co-authored-by: Austin Sullivan <asully@google.com>

Update acknowledgements

56f504a

Reference correct cursor for FileSystemSyncAccessHandle

7bc3f62

Missed one WritableFileStream->SyncAccessHandle

e21c354

spurious spaces

8663b9e

jesup mentioned this pull request Dec 1, 2022

FileSystemWritableFileStream should have normative text for the cursor #77

Open

a-sully approved these changes Dec 1, 2022

View reviewed changes

index.bs Show resolved Hide resolved

annevk reviewed Dec 2, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

jesup and others added 4 commits December 2, 2022 22:34

Update index.bs

37b96b7

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

remove ack

e1b1e05

Update index.bs

380e06e

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>

Add non-normative text about cursor for read() and write()

f077a52

annevk reviewed Dec 3, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Update index.bs

f10d791

annevk approved these changes Dec 3, 2022

View reviewed changes

annevk reviewed Dec 3, 2022

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Update index.bs

e5918b1

annevk merged commit 730e2ad into whatwg:main Dec 3, 2022

annevk mentioned this pull request Jan 4, 2023

Prose for dealing with dictionary members is wrong #81

Closed

chromium-wpt-export-bot mentioned this pull request Jan 17, 2023

FSA: Implement a cursor for SyncAccessHandles web-platform-tests/wpt#37764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cursor support for FileSystemSyncAccessHandle #76

Cursor support for FileSystemSyncAccessHandle #76

jesup commented Nov 30, 2022 •

edited by pr-preview bot

Loading

jesup commented Nov 30, 2022

a-sully Dec 1, 2022

annevk Dec 1, 2022

jesup commented Dec 1, 2022

a-sully left a comment

a-sully commented Jan 4, 2023 •

edited

Loading

annevk commented Jan 4, 2023

Cursor support for FileSystemSyncAccessHandle #76

Cursor support for FileSystemSyncAccessHandle #76

Conversation

jesup commented Nov 30, 2022 • edited by pr-preview bot Loading

jesup commented Nov 30, 2022

a-sully Dec 1, 2022

Choose a reason for hiding this comment

annevk Dec 1, 2022

Choose a reason for hiding this comment

jesup commented Dec 1, 2022

a-sully left a comment

Choose a reason for hiding this comment

a-sully commented Jan 4, 2023 • edited Loading

annevk commented Jan 4, 2023

jesup commented Nov 30, 2022 •

edited by pr-preview bot

Loading

a-sully commented Jan 4, 2023 •

edited

Loading