Bug 204119

Summary: Take service worker assertions based on client processes assertion states
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, ews-watchlist, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=225598
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews214 for win-future
none
Patch for landing
none
Patch for landing none

Description youenn fablet 2019-11-12 09:57:57 PST
Take service worker assertions based on client processes assertion states
Comment 1 youenn fablet 2019-11-12 10:04:45 PST
Created attachment 383362 [details]
Patch
Comment 2 youenn fablet 2019-11-12 10:05:47 PST
<rdar://problem/57048834>
Comment 3 youenn fablet 2019-11-12 11:46:16 PST
Created attachment 383367 [details]
Patch
Comment 4 youenn fablet 2019-11-12 15:17:18 PST
Created attachment 383392 [details]
Patch
Comment 5 youenn fablet 2019-11-12 17:43:48 PST
Created attachment 383413 [details]
Patch
Comment 6 EWS Watchlist 2019-11-13 04:53:47 PST
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
Comment 7 EWS Watchlist 2019-11-13 04:53:49 PST
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
Comment 8 Chris Dumez 2019-11-13 08:14:56 PST
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?
Comment 9 youenn fablet 2019-11-13 08:47:07 PST
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.
Comment 10 youenn fablet 2019-11-13 09:28:11 PST
Let's go with registerServiceWorkerClientProcess/unregisterServiceWorkerClientProcess
Comment 11 youenn fablet 2019-11-13 09:45:44 PST
Created attachment 383463 [details]
Patch for landing
Comment 12 youenn fablet 2019-11-13 10:15:54 PST
Created attachment 383466 [details]
Patch for landing
Comment 13 WebKit Commit Bot 2019-11-13 11:15:20 PST
Comment on attachment 383466 [details]
Patch for landing

Clearing flags on attachment: 383466

Committed r252418: <https://trac.webkit.org/changeset/252418>
Comment 14 WebKit Commit Bot 2019-11-13 11:15:22 PST
All reviewed patches have been landed.  Closing bug.