Bug 231676 – Implement FileSystemHandle move()

• Component: New Bugs
• Status: RESOLVED FIXED

[Sihui Liu]

...

[Sihui Liu]

Created attachment 441084 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/84199823>

[Sihui Liu]

Created attachment 441087 [details]
Patch

[Sihui Liu]

Created attachment 441092 [details]
Patch

[youenn fablet]

Comment on attachment 441092 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        Provide a way to move an existing file to a new place in OPFS.

APFS?

> Source/WebCore/Modules/filesystemaccess/FileSystemHandle.cpp:66
> +        promise.settle(WTFMove(result));

As a future refactoring, we should probably enqueue a task to resolve the promises. That will make sure we do this in the event loop.

> Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:77
> +    return !(name == "." || name == ".." || name.contains(pathSeparator));

Might be easier to read as: name != "." && name != ".." && !name.containts(pathSeparator).

> Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:108
> +    // Do not remove file if there is open access handle as web process may be performing oeprations.

s/oeprations/operations/

> Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:110
> +        return FileSystemStorageError::InvalidState;

Do we have a test for it?

> Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:273
> +        return FileSystemStorageError::InvalidState;

Do we have a test for it?

> Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:283
> +        return FileSystemStorageError::InvalidName;

Do we have a test for it?

[Sihui Liu]

(In reply to youenn fablet from comment #5)
> Comment on attachment 441092 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=441092&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        Provide a way to move an existing file to a new place in OPFS.
> 
> APFS?

Will use Origin Private File System.

> 
> > Source/WebCore/Modules/filesystemaccess/FileSystemHandle.cpp:66
> > +        promise.settle(WTFMove(result));
> 
> As a future refactoring, we should probably enqueue a task to resolve the
> promises. That will make sure we do this in the event loop.

Sure.

> 
> > Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:77
> > +    return !(name == "." || name == ".." || name.contains(pathSeparator));
> 
> Might be easier to read as: name != "." && name != ".." &&
> !name.containts(pathSeparator).

Will change.

> 
> > Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:108
> > +    // Do not remove file if there is open access handle as web process may be performing oeprations.
> 
> s/oeprations/operations/

Will change.

> 
> > Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:110
> > +        return FileSystemStorageError::InvalidState;
> 
> Do we have a test for it?

No, I will update existing test.

> 
> > Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:273
> > +        return FileSystemStorageError::InvalidState;
> 
> Do we have a test for it?

Yes, it's in the new test.

> 
> > Source/WebKit/NetworkProcess/storage/FileSystemStorageHandle.cpp:283
> > +        return FileSystemStorageError::InvalidName;
> 
> Do we have a test for it?

No, will add it in the new test.

[Sihui Liu]

Created attachment 441099 [details]
Patch for landing

[Sihui Liu]

Created attachment 441100 [details]
Patch for landing

[EWS]

Committed r284124 (242944@main): <https://commits.webkit.org/242944@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441100 [details].