[webkit-reviews] review granted: [Bug 230675] Make StorageManager available in Worker : [Attachment 439432] 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 230675: Make StorageManager available in Worker
https://bugs.webkit.org/show_bug.cgi?id=230675

Attachment 439432: Patch

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




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

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

> Source/WebCore/Modules/storage/WorkerStorageConnection.cpp:46
> +	   m_mainThreadConnection =
scope.thread().workerLoaderProxy().storageConnection();

storageConnection() is currently a pointer so m_mainThreadConnection can be
nullptr for instance in getPersisted.
Maybe we can guarantee to have a reference here which will make sure
m_mainThreadConnection is not null.

> Source/WebCore/Modules/storage/WorkerStorageConnection.cpp:71
> +    callOnMainThread([callbackIdentifier, workerThread = Ref {
m_scope->thread() }, mainThreadConnection = m_mainThreadConnection, origin =
origin.isolatedCopy()]() mutable {

Since we are taking a ref to the worker thread, another strategy is to not have
m_mainThreadConnection and retrieve it from worker thread loader proxy each
time we need it from main thread.
I would tend to do this plus a null check on the connection.

> Source/WebCore/Modules/storage/WorkerStorageConnection.cpp:72
> +	   auto mainThreadCallback = [callbackIdentifier, workerThread =
WTFMove(workerThread)](auto result) mutable {

Let's use bool here.
If in the future, we want to have ExceptionOr<bool>, we will need to change the
code and maybe isolate copy it.