[webkit-reviews] review granted: [Bug 204119] Take service worker assertions based on client processes assertion states : [Attachment 383413] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 204119: Take service worker assertions based on client processes assertion
states
https://bugs.webkit.org/show_bug.cgi?id=204119

Attachment 383413: Patch

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




--- Comment #8 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 383413
  --> https://bugs.webkit.org/attachment.cgi?id=383413
Patch

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

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:1239
> +   
serviceWorkerProcess->registerWebProcessToServiceWorkerProcess(*webProcess);

Could this be named better? Maybe registerClientWebProcess() /
unregisterClientWebProcess() ?

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in:65
> +    RegisterWebProcessToServiceWorkerProcess(WebCore::ProcessIdentifier
webProcessIdentifier, WebCore::ProcessIdentifier
serviceWorkerProcessIdentifier)

I think the naming could be better. Maybe RegisterServiceWorkerProcessClient()
or RegisterServiceWorkerProcessClientWebProcess()

> Source/WebKit/UIProcess/WebProcessPool.cpp:2051
> +    callOnMainRunLoop([this, weakThis = makeWeakPtr(this)] {

What's the benefit about doing this in the next run loop iteration?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1667
> +

waitUntilServiceWorkerProcessForegroundActivityState(webView.get(), false);  to
be safe?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1669
> +    waitUntilServiceWorkerProcessForegroundActivityState(webView.get(),
true);

waitUntilServiceWorkerProcessBackgroundActivityState(webView.get(), false); to
be safe?