Summary: | Take service worker assertions based on client processes assertion states | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||||||||
Component: | Service Workers | Assignee: | 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
youenn fablet
2019-11-12 09:57:57 PST
Created attachment 383362 [details]
Patch
Created attachment 383367 [details]
Patch
Created attachment 383392 [details]
Patch
Created attachment 383413 [details]
Patch
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 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 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 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. Let's go with registerServiceWorkerClientProcess/unregisterServiceWorkerClientProcess Created attachment 383463 [details]
Patch for landing
Created attachment 383466 [details]
Patch for landing
Comment on attachment 383466 [details] Patch for landing Clearing flags on attachment: 383466 Committed r252418: <https://trac.webkit.org/changeset/252418> All reviewed patches have been landed. Closing bug. |