| Summary: | Implement FileSystemHandle move() | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sihui Liu <sihui_liu> | ||||||||||||
| Component: | New Bugs | Assignee: | Sihui Liu <sihui_liu> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | cdumez, esprehn+autocc, ews-watchlist, kondapallykalyan, webkit-bug-importer, youennf | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Bug Depends on: | |||||||||||||||
| Bug Blocks: | 231706 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Sihui Liu
2021-10-13 08:25:27 PDT
Created attachment 441084 [details]
Patch
Created attachment 441087 [details]
Patch
Created attachment 441092 [details]
Patch
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? (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. Created attachment 441099 [details]
Patch for landing
Created attachment 441100 [details]
Patch for landing
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]. |