Bug 204119 – Take service worker assertions based on client processes assertion states

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[youenn fablet]

Take service worker assertions based on client processes assertion states

[youenn fablet]

Created attachment 383362 [details]
Patch

[youenn fablet]

<rdar://problem/57048834>

[youenn fablet]

Created attachment 383367 [details]
Patch

[youenn fablet]

Created attachment 383392 [details]
Patch

[youenn fablet]

Created attachment 383413 [details]
Patch

[EWS Watchlist]

Comment on attachment 383413 [details]
Patch

Attachment 383413 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13246496

New failing tests:
imported/blink/fast/events/panScroll-crash.html

[EWS Watchlist]

Created attachment 383449 [details]
Archive of layout-test-results from ews214 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Chris Dumez]

Comment on attachment 383413 [details]
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?

[youenn fablet]

Comment on attachment 383413 [details]
Patch

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

>> Source/WebKit/UIProcess/WebProcessPool.cpp:2051
>> +    callOnMainRunLoop([this, weakThis = makeWeakPtr(this)] {
> 
> What's the benefit about doing this in the next run loop iteration?

I tried to explain this in the comment above.

WebProcessPool::updateProcessAssertions gets called at creation of the token, but the token is not yet assigned to WebProcessProxy.
If calling updateServiceWorkerProcessAssertion(), the WebProcessProxy will be seen as not holding an assertion although it will be just after.

[youenn fablet]

Let's go with registerServiceWorkerClientProcess/unregisterServiceWorkerClientProcess

[youenn fablet]

Created attachment 383463 [details]
Patch for landing

[youenn fablet]

Created attachment 383466 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 383466 [details]
Patch for landing

Clearing flags on attachment: 383466

Committed r252418: <https://trac.webkit.org/changeset/252418>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.