| Summary: | Release FileSystemStorageHandle when it is not in use | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sihui Liu <sihui_liu> | ||||||||||||
| Component: | New Bugs | Assignee: | Sihui Liu <sihui_liu> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | ayumi_kojima, sihui_liu, 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-26 20:10:10 PDT
Created attachment 442564 [details]
Patch
Created attachment 442570 [details]
Patch
Created attachment 442596 [details]
Patch
Comment on attachment 442596 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=442596&action=review > Source/WebCore/Modules/filesystemaccess/FileSystemFileHandle.cpp:69 > + FileSystem::closeFile(file); If we need to do it there, we might have an issue as this task might be scheduled but never executed. Maybe we should have a result that, if destroyed and its file is valid, would call closeFile whatever. > Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:81 > + return; If we do not have a scope, shouldn't we still call closeHandle? > Source/WebCore/Modules/storage/StorageManager.cpp:125 > + connection->closeHandle(identifier); Ditto here, should we call closeHandle if the task is not executed? Created attachment 442757 [details]
Patch for landing
Committed r285007 (243653@main): <https://commits.webkit.org/243653@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442757 [details]. Reverted r285007 for reason: Reverting r285007 because it caused storage/filesystemaccess/sync-access-handle-basics-worker.html to constantly fail. Committed r285030 (243674@main): <https://commits.webkit.org/243674@main> Created attachment 442834 [details]
Patch for landing
Comment on attachment 442834 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=442834&action=review Re-landing patch after making a small change to the test file. > LayoutTests/storage/filesystemaccess/resources/sync-access-handle-basics.js:23 > + }, () => { > + createFileHandle(rootHandle); This old patch exits early here. The removeEntry() means to clean up existing test file if it exists. If the file does not exist, removeEntry may throw error, and it's fine to continue test. Committed r285041 (243685@main): <https://commits.webkit.org/243685@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 442834 [details]. |