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/CAL5BFfW1bPEwOP_Ho-XLzONhs5CN-W0cSeuzuXOvuhrDGwfW3A%40mail.gmail.com.

Reply via email to