[webkit-reviews] review granted: [Bug 234271] REGRESSION (r286601): storage/filesystemaccess/sync-access-handle-read-write-worker.html and file-system-access/sandboxed_FileSystemSyncAccessHandle-truncate.https.tentative.worker.html are consistently failing : [Attachment 447174] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 234271: REGRESSION (r286601):
storage/filesystemaccess/sync-access-handle-read-write-worker.html and
file-system-access/sandboxed_FileSystemSyncAccessHandle-truncate.https.tentativ
e.worker.html are consistently failing
https://bugs.webkit.org/show_bug.cgi?id=234271

Attachment 447174: Patch

https://bugs.webkit.org/attachment.cgi?id=447174&action=review




--- Comment #7 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 447174
  --> https://bugs.webkit.org/attachment.cgi?id=447174
Patch

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

>
Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:3
7
> +class FileSystemHandleCloseScope : public
ThreadSafeRefCounted<FileSystemHandleCloseScope> {

Not sure it needs to be refcounted, maybe we can just have a unique_ptr>
Also, maybe we should expose this object in WebCore and callbacks with a needed
WebProcess specialisation to do the clean-up.
That way, we would not need to modify the code to allow FileSystemFileHandle
creation with a null context in WebCore.

>
Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:4
7
> +	       m_connection->closeHandle(m_identifier);

I am not sure closeHandle is expected to be called from a non main thread.
You might fix this by explicitly call callOnMainThread, or make
ThreadSafeRefCounted destruction main run loop.

>
Source/WebCore/Modules/filesystemaccess/WorkerFileSystemStorageConnection.cpp:1
68
> +		       return;

We are doing twice the same "is scope closed" here and in the WebCore callback.