[webkit-reviews] review granted: [Bug 180669] Merge ServiceWorkerClientIdentifier into ServiceWorkerClientData : [Attachment 329031] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 11 14:56:22 PST 2017

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 180669: Merge ServiceWorkerClientIdentifier into ServiceWorkerClientData

Attachment 329031: Patch


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

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

> Source/WebCore/dom/Document.cpp:7630
> +	   m_serviceWorkerConnection->unregisterServiceWorkerClient({
m_serviceWorkerConnection->serverConnectionIdentifier(), identifier() });

I wonder whether we should not have made SWClientConnection only take
DocumentIdentifier and do the translation to ServiceWorkerClientIdentifier on
its own.
Maybe this is too late for that. If not we could stick with passing a
DocumentIdentifier to unregisterServieWorkerClient.

> Source/WebCore/workers/service/ServiceWorkerClientData.cpp:65
> +    RELEASE_ASSERT(isDocument); // We do not support dedicated workers as
clients yet.

Isn't the downcast<Document> doing this release assert already?

> Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:121
> +	       auto sourceClient =

Maybe use RefPtr<ServiceWorkerClient> here instead of auto so that there is not
the need for it below for source =...?

> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h:93
> +    void unregisterServiceWorkerClient(const

Sometimes we are using const ServiceWorkerClientIdentifier&, sometimes
ServiceWorkerClientIdentifier directly.

More information about the webkit-reviews mailing list