LGTM2. -mike
On Tue, Apr 12, 2022 at 6:08 AM Yoav Weiss <yoavwe...@chromium.org> wrote: > LGTM1 > > Thanks for pushing this through. Please make sure to follow-up on any > feedback we'd get on the PR itself. > > On Mon, Apr 11, 2022 at 7:44 PM Emanuel Krivoy <fived...@chromium.org> > wrote: > >> Hello! >> >> I've removed the options object from our implementation, and filed a new >> PR against the WHATWG repo (https://github.com/whatwg/fs/pull/21) that >> incorporates the previous feedback. >> >> Yoav: >> Yes, we'd like to keep the current OT running and then ship on 102 >> without removing availability of the surface in between. >> >> On Thu, Apr 7, 2022 at 12:38 PM Yoav Weiss <yoavwe...@chromium.org> >> wrote: >> >>> >>> >>> On Mon, Apr 4, 2022 at 5:11 PM Emanuel Krivoy <fived...@chromium.org> >>> wrote: >>> >>>> Hello, >>>> >>>> Replying to Mike inline: >>>> >>>> https://github.com/WICG/file-system-access/pull/344 doesn't seem to >>>>> have moved in the last ~2 weeks, and I don't see a new PR against the >>>>> WHATWG spec. What's y'all's timeline for finishing the specification of >>>>> this feature? >>>> >>>> >>>> The plan is to create the PR against the spec in WHATWG this week. It >>>> should include the changes from the current feedback in the old PR. >>>> >>>> >>>> >>>>> Thanks for doing this investigation! It does sound like something we'd >>>>> want to resolve before shipping, as it would be unfortunate for this to >>>>> present a barrier to interop. >>>>> >>>>> I didn't see a bug filed against webkit in a quick search, can you >>>>> follow up on that (or point it out if I missed it)? >>>> >>>> >>>> >>>> I directly followed up with WebKit and the Storage team. The result of >>>> the discussions was that, to avoid compatibility issues with Safari and >>>> leave the design of the options object fully open, we should temporarily >>>> remove the options parameter from createSyncAccessHandle(). >>>> >>>> >>>> >>>> Once there is consensus on how options should be handled, it should be >>>> easy to add them back. We would end up in the desired final state, but with >>>> an inverted default: the OPFS Access Handle behavior is the default one, >>>> and specific options would be needed to use them in other file systems. >>>> Since the OPFS use case is the one that has been proven with trials, and >>>> the one that other browsers intend to implement for now, I think it makes >>>> sense to leave it as the default. >>>> >>>> >>>> >>>> To all: >>>> >>>> >>>> >>>> Since we have to do code changes to remove the options object, and >>>> since the spec still has to be rebased, I wanted to change this request >>>> from shipment on 101 to a gapless shipment on 102. I’ll keep working on >>>> those two pending items and ping this thread when they are done. >>>> >>> >>> Just to clarify, you're planning to run the OT till the end of M101 and >>> then gaplessly ship in M102? >>> >>> >>>> >>>> Thanks! >>>> Emanuel >>>> >>>> On Wed, Mar 30, 2022 at 3:01 PM Mike West <mk...@chromium.org> wrote: >>>> >>>>> On Wed, Mar 23, 2022 at 5:48 PM Emanuel Krivoy <fived...@chromium.org> >>>>> wrote: >>>>> >>>>>> Hey Yoav, >>>>>> >>>>>> >>>>>>> So the plan is to land the PR in WICG, and then (immediately) move >>>>>>> it over to https://fs.spec.whatwg.org/? >>>>>>> What are the current blockers for the WICG PR from landing? >>>>>>> >>>>>> >>>>>> My plan would be to act on the current round of feedback in the WICG >>>>>> PR and then move the spec to its final home in WHATWG to finish the >>>>>> review/merge there. >>>>>> The situation is an artifact of me wanting to do a quick round of >>>>>> feedback before investing time in the rebase, just to make sure the spec >>>>>> was going in the right direction. Now I think it might have made things >>>>>> more confusing than they should have been, sorry! >>>>>> >>>>> >>>>> https://github.com/WICG/file-system-access/pull/344 doesn't seem to >>>>> have moved in the last ~2 weeks, and I don't see a new PR against the >>>>> WHATWG spec. What's y'all's timeline for finishing the specification of >>>>> this feature? >>>>> >>>>> Have you tried running STP against the WPT test suite? That could be >>>>>>> reassuring interop-wise >>>>>>> >>>>>> >>>>>> Thanks for the suggestion. After running the WPTs, there seems to be >>>>>> some divergence with the proposed spec. The most substantial one (beyond >>>>>> some issues around the type of error thrown) is that the implementation >>>>>> of >>>>>> createSyncAccessHandle in Safari TP does not take an options parameter. >>>>>> >>>>>> The options parameter is there to (eventually) allow using access >>>>>> handles on other filesystems (i.e., from outside OPFS, in particular on >>>>>> files hosted in the local file system). This feature has been requested >>>>>> by >>>>>> developers on various occasions, and would make the File System Access >>>>>> API >>>>>> more flexible. In our implementation, the options parameter is required >>>>>> (as >>>>>> in, has to be provided when calling createSyncAccessHandle) to avoid >>>>>> setting the default behavior of access handles to the particular one >>>>>> needed >>>>>> within OPFS. Further context can be found in >>>>>> https://github.com/whatwg/fs/issues/19. >>>>>> >>>>>> I will go ahead and file the appropriate bugs/contact the feature >>>>>> owner! >>>>>> >>>>> >>>>> Thanks for doing this investigation! It does sound like something we'd >>>>> want to resolve before shipping, as it would be unfortunate for this to >>>>> present a barrier to interop. >>>>> >>>>> I didn't see a bug filed against webkit in a quick search, can you >>>>> follow up on that (or point it out if I missed it)? >>>>> >>>>> Thanks! >>>>> >>>>> -mike >>>>> >>>>> >>>>>> >>>>>> On Wed, Mar 23, 2022 at 5:39 AM Yoav Weiss <yoavwe...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> Thanks for working on this important capability!! >>>>>>> >>>>>>> On Tuesday, March 22, 2022 at 5:24:08 PM UTC+1 Emanuel Krivoy wrote: >>>>>>> >>>>>>>> Hello blink-dev, We'd like to request a review on our intent to >>>>>>>> ship Access Handles with Chrome 101. Since we don't envision changes >>>>>>>> to the >>>>>>>> surface and it is currently in use by Photoshop web, this request >>>>>>>> comes one >>>>>>>> release before our OT expires. Please find the details below: >>>>>>>> Contact emails >>>>>>>> >>>>>>>> fived...@chromium.org, r...@chromium.org >>>>>>>> >>>>>>>> Explainer >>>>>>>> >>>>>>>> https://github.com/WICG/file-system-access/blob/main/AccessHandle.md >>>>>>>> >>>>>>>> Specification >>>>>>>> >>>>>>>> Out for review. >>>>>>>> <https://github.com/WICG/file-system-access/pull/344/files> Will >>>>>>>> be moved <https://github.com/WICG/file-system-access/issues/342> >>>>>>>> to WHATWG after replying to pending comments. >>>>>>>> >>>>>>> >>>>>>> So the plan is to land the PR in WICG, and then (immediately) move >>>>>>> it over to https://fs.spec.whatwg.org/? >>>>>>> What are the current blockers for the WICG PR from landing? >>>>>>> >>>>>>> >>>>>>>> Summary >>>>>>>> >>>>>>>> The Origin Private File System (OPFS, part of the File System >>>>>>>> Access API) is augmented with a new surface that brings very performant >>>>>>>> access to data. This new surface differs from existing ones by offering >>>>>>>> in-place and exclusive write access to a file’s content. This change, >>>>>>>> along >>>>>>>> with the ability to consistently read unflushed modifications and the >>>>>>>> availability of a synchronous variant on dedicated workers, >>>>>>>> significantly >>>>>>>> improves performance and unblocks new use cases (especially for porting >>>>>>>> existing IO-heavy applications to WebAssembly). >>>>>>>> >>>>>>>> This Intent-to-Ship is only in reference to the sync variant of the >>>>>>>> API i.e., the createSyncAccessHandle() method and the >>>>>>>> SyncAccessHandle object (only exposed in worker contexts): >>>>>>>> >>>>>>>> const handle = await file.createSyncAccessHandle(); >>>>>>>> >>>>>>>> var writtenBytes = handle.write(buffer); >>>>>>>> >>>>>>>> var readBytes = handle.read(buffer {at: 1}); >>>>>>>> >>>>>>>> The sync variant is meant to be consumed by low-level entities like >>>>>>>> toolchains. We expect application developers to prefer the async API >>>>>>>> with >>>>>>>> its streaming interface which will be shipped later. >>>>>>>> >>>>>>>> AccessHandles is the new API shape for what was previously called >>>>>>>> Storage Foundation API (Intent-to-Experiment: >>>>>>>> http://groups.google.com/a/chromium.org/g/blink-dev/c/Jhirhnq3WbY). >>>>>>>> >>>>>>>> Blink component >>>>>>>> >>>>>>>> Blink>Storage>FileSystem >>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EStorage%3EFileSystem> >>>>>>>> >>>>>>>> TAG review >>>>>>>> >>>>>>>> https://github.com/w3ctag/design-reviews/issues/664 >>>>>>>> >>>>>>>> TAG review status >>>>>>>> >>>>>>>> Issues addressed >>>>>>>> >>>>>>>> Risks >>>>>>>> Interoperability and Compatibility >>>>>>>> >>>>>>>> The feature has to be compatible with existing ways to access data >>>>>>>> on OPFS i.e., createWritable() and getFile(). The use of write >>>>>>>> locks and care for backwards compatibility should mean that the risk >>>>>>>> here >>>>>>>> is low. In order to ease compatibility concerns in the future, we've >>>>>>>> added >>>>>>>> an optional 'mode' parameter to createAccessHandle()/ >>>>>>>> createSyncAccessHandle(). This allows us to eventually extend >>>>>>>> AccessHandle functionality to non-OPFS file systems without >>>>>>>> necessarily taking the OPFS behaviour as default (more details here: >>>>>>>> https://github.com/WICG/file-system-access/blob/main/AccessHandle.md#exposing-accesshandles-on-all-filesystems >>>>>>>> ). >>>>>>>> >>>>>>>> There is a risk of interoperability between vendors, pending the >>>>>>>> position on implementing this surface. This design is the result of >>>>>>>> feedback from Gecko and WebKit, who reviewed previous iterations of >>>>>>>> this >>>>>>>> functionality and gave feedback that it should integrate more strongly >>>>>>>> with >>>>>>>> OPFS. We directly shared documents outlining alternatives >>>>>>>> considered >>>>>>>> <https://docs.google.com/document/d/121OZpRk7bKSF7qU3kQLqAEUVSNxqREnE98malHYwWec>, >>>>>>>> and later our recommendation >>>>>>>> <https://docs.google.com/document/d/1g7ZCqZ5NdiU7oqyCpsc2iZ7rRAY1ZXO-9VoG4LfP7fM> >>>>>>>> towards this particular API shape. >>>>>>>> >>>>>>>> We believe that the new design, when paired with a separate >>>>>>>> streams-based extension to OPFS, meets the goal of more strongly >>>>>>>> integrating with the existing surface. However, we have not yet >>>>>>>> received >>>>>>>> replies to the position requests below. >>>>>>>> >>>>>>>> Gecko: Worth Prototyping ( >>>>>>>> https://github.com/mozilla/standards-positions/issues/562) >>>>>>>> >>>>>>>> WebKit: In development ( >>>>>>>> https://lists.webkit.org/pipermail/webkit-dev/2021-August/031934.html) >>>>>>>> Request for position was not answered, but the feature is being >>>>>>>> implemented >>>>>>>> and is available in TP. See reference bug: >>>>>>>> https://bugs.webkit.org/show_bug.cgi?id=231185 >>>>>>>> >>>>>>> >>>>>>> Have you tried running STP against the WPT test suite? That could be >>>>>>> reassuring interop-wise >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> Web developers: Positive >>>>>>>> >>>>>>>> From our Storage Foundation OT, we received very positive feedback >>>>>>>> on the need for high performance storage, as well as on the general >>>>>>>> shape >>>>>>>> of the API: >>>>>>>> >>>>>>>> >>>>>>>> - >>>>>>>> >>>>>>>> Adobe’s support statement (about the need for the capability) >>>>>>>> <https://github.com/WICG/proposals/issues/10#issuecomment-804145429> >>>>>>>> - >>>>>>>> >>>>>>>> absurd-sql’s mention >>>>>>>> >>>>>>>> <https://github.com/mozilla/standards-positions/issues/481#issuecomment-898061119> >>>>>>>> - >>>>>>>> >>>>>>>> Reception on Twitter after DevRel announcement >>>>>>>> <https://twitter.com/ChromiumDev/status/1405101909757902851> >>>>>>>> >>>>>>>> >>>>>>> Thanks for a very strong developer signal! >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> SyncAccessHandles have a very similar shape to the surface that >>>>>>>> was exposed in Storage Foundation’s Origin Trial. It is currently a >>>>>>>> critical dependency >>>>>>>> <https://web.dev/ps-on-the-web/#high-performance-storage> of >>>>>>>> Photoshop Web. The Photoshop team has confirmed that the current >>>>>>>> surface >>>>>>>> covers their needs and that they have no pending feedback/requests. >>>>>>>> >>>>>>>> Ergonomics >>>>>>>> >>>>>>>> As mentioned above, SyncAccessHandles offer a very similar surface >>>>>>>> to the one positively received during Storage Foundation’s OT. The main >>>>>>>> differences are the migration of file system operations into OPFS and >>>>>>>> the >>>>>>>> asynchronicity of auxiliary methods (i.e. methods other than read and >>>>>>>> write). >>>>>>>> >>>>>>>> Since many of our use cases require good interoperability between >>>>>>>> this API and Wasm, we’ve developed an Emscripten file system >>>>>>>> <https://github.com/rstz/emscripten-pthreadfs/tree/main/pthreadfs> >>>>>>>> that allows ported applications to use SyncAccessHandles. This >>>>>>>> simplifies both activation and use, since the API can be accessed >>>>>>>> through >>>>>>>> standard C/C++ file system libraries. >>>>>>>> >>>>>>>> Security and Privacy >>>>>>>> >>>>>>>> SyncAccessHandles have received approval for Security and Privacy >>>>>>>> in our launch bug >>>>>>>> <https://bugs.chromium.org/p/chromium/issues/detail?id=1232436>. >>>>>>>> >>>>>>>> Debuggability >>>>>>>> >>>>>>>> Basic tooling: Autocomplete works as described in "New WebIDL/DOM >>>>>>>> interfaces and attributes". >>>>>>>> >>>>>>>> Extended tooling: we'll eventually want to be able to explore files >>>>>>>> stored in OPFS. There are two tracking bugs related to this: >>>>>>>> crbug.com/256067 and crbug.com/735618. This API doesn't really add >>>>>>>> new storage backends, just new ways to interact with files, so we'd be >>>>>>>> covered by those as well. >>>>>>>> >>>>>>>> >>>>>>>> File System Access API usage is also reflected in user settings >>>>>>>> pages such as chrome://settings/siteData. >>>>>>>> >>>>>>>> Is this feature fully tested by web-platform-tests >>>>>>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md> >>>>>>>> ? >>>>>>>> >>>>>>>> Yes, we’ve added tests for all new functionality, as well as for >>>>>>>> the intersection between this surface and existing parts of OPFS, e.g., >>>>>>>> we’ve made sure that locking between writables and access handles is >>>>>>>> correct. >>>>>>>> >>>>>>>> Our test suite is also run against our Incognito mode >>>>>>>> implementation, since it is significantly different from the regular >>>>>>>> mode >>>>>>>> one. >>>>>>>> >>>>>>>> wpt.fyi results: >>>>>>>> wpt.fyi/results/file-system-access?label=master&label=experimental&aligned >>>>>>>> >>>>>>>> Is this feature supported on all six Blink platforms? >>>>>>>> >>>>>>>> Not yet. File System Access API is not yet available on Android or >>>>>>>> Android WebView, but the Storage team has expressed interest >>>>>>>> <https://bugs.chromium.org/p/chromium/issues/detail?id=1011535#c9> >>>>>>>> in at least enabling OPFS once there is more usage/cross-browser >>>>>>>> support. >>>>>>>> >>>>>>>> Demo >>>>>>>> >>>>>>>> The API has no UI component. An example code snippet can be found >>>>>>>> here >>>>>>>> <https://github.com/WICG/file-system-access/blob/access-handle-spec/AccessHandle.md#new-data-access-surface> >>>>>>>> . >>>>>>>> >>>>>>>> DevTrial instructions >>>>>>>> >>>>>>>> >>>>>>>> https://github.com/WICG/file-system-access/blob/main/AccessHandle.md#trying-it-out >>>>>>>> >>>>>>>> Flag name >>>>>>>> >>>>>>>> FileSystemAccessAccessHandle >>>>>>>> >>>>>>>> Requires code in //chrome? >>>>>>>> >>>>>>>> False >>>>>>>> >>>>>>>> Tracking bug >>>>>>>> >>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1218431 >>>>>>>> >>>>>>>> Launch bug >>>>>>>> >>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1232436 >>>>>>>> >>>>>>>> Estimated milestones >>>>>>>> >>>>>>>> OriginTrial desktop last >>>>>>>> >>>>>>>> 102 >>>>>>>> >>>>>>>> OriginTrial desktop first >>>>>>>> >>>>>>>> 95 >>>>>>>> >>>>>>>> DevTrial on desktop >>>>>>>> >>>>>>>> 94 >>>>>>>> >>>>>>>> We aim to ship with 101. >>>>>>>> >>>>>>>> Link to entry on the Chrome Platform Status >>>>>>>> >>>>>>>> https://chromestatus.com/feature/5702777582911488 >>>>>>>> >>>>>>>> Links to previous Intent discussions >>>>>>>> >>>>>>>> Intent to prototype: >>>>>>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/33T36N6VBKI >>>>>>>> >>>>>>>> Ready for Trial: >>>>>>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/_nB5VfgXW_I >>>>>>>> >>>>>>>> Intent to Experiment: >>>>>>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/-FVIvFovd3g/m/vUNm4X8UBAAJ >>>>>>>> >>>>>>>> Intent to Extend Experiment: >>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAHExSGL4tBM-mH%2B-Cm7YtBiVMLLGrPMVxtCHYwG6PM_oG67hjw%40mail.gmail.com >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/cahexsgl4tbm-mh+-cm7ytbivmllgrpmvxtchywg6pm_og67...@mail.gmail.com> >>>>>>>> >>>>>>>> This intent message was generated by Chrome Platform Status >>>>>>>> <https://www.chromestatus.com/>. >>>>>>>> >>>>>>> -- 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+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAKXHy%3DdHh_Pnt_MzHv94AxPauLcYeh2sYWsbfq9vMiZ0sjhfdA%40mail.gmail.com.
