[webkit-reviews] review granted: [Bug 232146] Perform FileSystemSyncAccessHandle operations in web process : [Attachment 443050] 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 232146: Perform FileSystemSyncAccessHandle operations in web process
https://bugs.webkit.org/show_bug.cgi?id=232146

Attachment 443050: Patch

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




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

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

> Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:159
> +{

Might be worth adding ASSERT(!isClosingOrClosed()) since we are calling this
from several places.

> Source/WebCore/Modules/filesystemaccess/FileSystemSyncAccessHandle.cpp:163
> +	   });

return here to remove the else.

> Source/WebCore/workers/WorkerGlobalScope.cpp:82
> +    static NeverDestroyed<Ref<WorkQueue>> queue(WorkQueue::create("Shared
File System Storage Queue",  WorkQueue::QOS::Utility));

Not really sure what the best QOS value is. On one hand, we want to be fast but
we do not want too many fast queues.
Maybe default is fine here.

> Source/WebCore/workers/WorkerGlobalScope.cpp:83
> +    return queue.get().copyRef();

Seems wasteful to return a Ref<>. We could return a WorQueue&, or define the
static queue directly in WorkerGlobalScope::postFileSystemStorageTask