Bug 232363 – Release FileSystemStorageHandle when it is not in use

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sihui Liu]

...

[Sihui Liu]

Created attachment 442564 [details]
Patch

[Sihui Liu]

Created attachment 442570 [details]
Patch

[Sihui Liu]

Created attachment 442596 [details]
Patch

[youenn fablet]

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?

[Sihui Liu]

Created attachment 442757 [details]
Patch for landing

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/84784824>

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>

[Sihui Liu]

Created attachment 442834 [details]
Patch for landing

[Sihui Liu]

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.

[EWS]

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].