Bug 230101 - Add stub for File System Access API
Summary: Add stub for File System Access API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sihui Liu
URL:
Keywords: InRadar
Depends on:
Blocks: 231706
  Show dependency tree / graph
 
Reported: 2021-09-09 09:00 PDT by Sihui Liu
Modified: 2021-10-13 18:39 PDT (History)
16 users (show)

See Also:

Attachments
Patch (68.77 KB, patch)
2021-09-09 09:03 PDT, Sihui Liu 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (72.39 KB, patch)
2021-09-09 09:18 PDT, Sihui Liu 		no flags Details | Formatted Diff | Diff
Patch (72.90 KB, patch)
2021-09-09 09:36 PDT, Sihui Liu 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (72.93 KB, patch)
2021-09-09 09:58 PDT, Sihui Liu 		no flags Details | Formatted Diff | Diff
Patch (161.87 KB, patch)
2021-09-09 15:50 PDT, Sihui Liu 		no flags Details | Formatted Diff | Diff
Patch for landing (150.69 KB, patch)
2021-09-13 10:50 PDT, Sihui Liu 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sihui Liu 2021-09-09 09:00:38 PDT 
...
Comment 1 Sihui Liu 2021-09-09 09:03:24 PDT 
Created attachment 437747 [details]
Patch
Comment 2 Sihui Liu 2021-09-09 09:18:29 PDT 
Created attachment 437750 [details]
Patch
Comment 3 Sihui Liu 2021-09-09 09:36:34 PDT 
Created attachment 437751 [details]
Patch
Comment 4 Sihui Liu 2021-09-09 09:58:23 PDT 
Created attachment 437755 [details]
Patch
Comment 5 Sihui Liu 2021-09-09 15:50:17 PDT 
Created attachment 437795 [details]
Patch
Comment 6 youenn fablet 2021-09-10 06:13:28 PDT 
Comment on attachment 437795 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437795&action=review

> Source/WebCore/Modules/filesystemaccess/FileSystemDirectoryHandle.h:56
> +    FileSystemDirectoryHandle(String&&);

explicit.

> Source/WebCore/Modules/filesystemaccess/FileSystemFileHandle.h:37
> +    struct CreateWritableOptions {

Let's add it when needed.

> Source/WebCore/Modules/filesystemaccess/FileSystemFileHandle.h:45
> +    FileSystemFileHandle(String&&);

explicit.

> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.h:49
> +    FileSystemHandle(Kind, String&& name);

Since it is refcounted, it is better to move the constructor to protected.

> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.idl:32
> +    EnabledBySetting=FileSystemAccess

I am not sure this has any effect on dictionaries.

> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.idl:47
> +    Promise<PermissionState> requestPermission(optional FileSystemHandlePermissionDescriptor descriptor);

Are we sure we will quickly implement these two?
If not, it might be better to not include them so that feature detection works by just checking that the method is defined.
Same questions for other methods.

> Source/WebCore/Modules/filesystemaccess/FileSystemPermissionDescriptor.idl:28
> +] dictionary FileSystemPermissionDescriptor : PermissionDescriptor {

Let's add this one and related when actually needed, I do not think this is exposed to JS.
Ditto for struct FileSystemPermissionDescriptor

> Source/WebCore/Modules/filesystemaccess/FileSystemPermissionMode.h:30
> +enum class FileSystemPermissionMode : uint8_t {

could be bool I guess
Comment 7 Sihui Liu 2021-09-13 10:50:53 PDT 
Created attachment 438054 [details]
Patch for landing
Comment 8 Sihui Liu 2021-09-13 10:51:37 PDT 
Comment on attachment 437795 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437795&action=review

>> Source/WebCore/Modules/filesystemaccess/FileSystemDirectoryHandle.h:56
>> +    FileSystemDirectoryHandle(String&&);
> 
> explicit.

Will add.

>> Source/WebCore/Modules/filesystemaccess/FileSystemFileHandle.h:37
>> +    struct CreateWritableOptions {
> 
> Let's add it when needed.

Okay.

>> Source/WebCore/Modules/filesystemaccess/FileSystemFileHandle.h:45
>> +    FileSystemFileHandle(String&&);
> 
> explicit.

Will add.

>> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.h:49
>> +    FileSystemHandle(Kind, String&& name);
> 
> Since it is refcounted, it is better to move the constructor to protected.

Sure.

>> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.idl:32
>> +    EnabledBySetting=FileSystemAccess
> 
> I am not sure this has any effect on dictionaries.

I saw other files using it on dictionary and I checked IDLAttributes.json; but it does seem to have no effect on dictionary according to CodeGenerator if I am reading it correctly

>> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.idl:47
>> +    Promise<PermissionState> requestPermission(optional FileSystemHandlePermissionDescriptor descriptor);
> 
> Are we sure we will quickly implement these two?
> If not, it might be better to not include them so that feature detection works by just checking that the method is defined.
> Same questions for other methods.

I think we do; but these two may not be implemented very quickly. I will remove these two first.

>> Source/WebCore/Modules/filesystemaccess/FileSystemPermissionDescriptor.idl:28
>> +] dictionary FileSystemPermissionDescriptor : PermissionDescriptor {
> 
> Let's add this one and related when actually needed, I do not think this is exposed to JS.
> Ditto for struct FileSystemPermissionDescriptor

Sure.

>> Source/WebCore/Modules/filesystemaccess/FileSystemPermissionMode.h:30
>> +enum class FileSystemPermissionMode : uint8_t {
> 
> could be bool I guess

Will remove this file and add when it's needed.
Comment 9 EWS 2021-09-13 14:44:48 PDT 
Committed r282361 (241624@main): <https://commits.webkit.org/241624@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438054 [details].
Comment 10 Radar WebKit Bug Importer 2021-09-22 13:43:16 PDT 
<rdar://problem/83415609>