Pass CORS-enabled schemes through WebProcess instead of having them NetworkProcess-global
Created attachment 380842 [details] Patch
Service Worker API tests are failing, is it related?
I verified the three API test failures do not reproduce on my Mac after applying the patch. They must've been failing because of something else.
Created attachment 380925 [details] Patch
(I re-uploaded the same patch to run through EWS again)
Created attachment 380935 [details] Patch
Comment on attachment 380935 [details] Patch I'm quite confused. I don't see how this patch can break those two tests in that way on EWS and I can't reproduce it locally. Youenn, any ideas? Only the "blob" scheme hits my changed code, and that's not registered or default CORS.
It is indeed strange that iOS is ok but not MacOS. I do not see either why this patch is failing these tests. Looking at the error message, both tests do two page loads. The first one to register a service worker. The second one gets intercepted by the service worker. It seems that the second one is not intercepted by the service worker. We have some optimisations in ServiceWorkerProvider::mayHaveServiceWorkerRegisteredForOrigin to not go to the network process. This is based on WebProcessPool checking for the sql file, which is potentially racy. Plan is to fix that by doing registration matching in network process and remove the need for ServiceWorkerProvider::mayHaveServiceWorkerRegisteredForOrigin. You could try to upload a patch with ServiceWorkerProvider::mayHaveServiceWorkerRegisteredForOrigin always returning true.
Comment on attachment 380935 [details] Patch LGTM other than the failing API test. View in context: https://bugs.webkit.org/attachment.cgi?id=380935&action=review > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:533 > + for (auto&& scheme : WTFMove(schemes)) No specific reason for WTFMove() here, or is it for documentation purposes? > Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:52 > +NetworkLoadChecker::NetworkLoadChecker(NetworkProcess& networkProcess, NetworkSchemeRegistry* schemeRegistry, FetchOptions&& options, PAL::SessionID sessionID, WebPageProxyIdentifier webPageProxyID, HTTPHeaderMap&& originalRequestHeaders, URL&& url, RefPtr<SecurityOrigin>&& sourceOrigin, RefPtr<SecurityOrigin>&& topOrigin, PreflightPolicy preflightPolicy, String&& referrer, bool isHTTPSUpgradeEnabled, bool shouldCaptureExtraNetworkLoadMetrics, LoadType requestLoadType) RefPtr<NetworkSchemeRegistry>&& with WTFMove and I guess Ref<NetworkProces>>&& as well > Source/WebKit/WebProcess/WebProcess.cpp:1182 > + m_networkProcessConnection->connection().send(Messages::NetworkConnectionToWebProcess::RegisterURLSchemesAsCORSEnabled(WebCore::LegacySchemeRegistry::allURLSchemesRegisteredAsCORSEnabled()), 0); We probably do not have tests for that.
(In reply to youenn fablet from comment #8) > You could try to upload a patch with > ServiceWorkerProvider::mayHaveServiceWorkerRegisteredForOrigin always > returning true. That makes the API test ServiceWorkers.SWProcessConnectionCreation always fail.
http://trac.webkit.org/r251138 I'll watch the bots to see if the ServiceWorkers API test actually start failing from this unrelated change and roll out if they do.
Rolled out in https://trac.webkit.org/changeset/251146/webkit because the bots did see the same failure that EWS did. Still can't reproduce. Youenn, could you fix the race condition and I'll retry this?
Removing race condition in https://bugs.webkit.org/show_bug.cgi?id=203055
Created attachment 381582 [details] retry all ews
Created attachment 381819 [details] Patch
ServiceWorker API tests pass. The one timeout on EWS is WebAuthenticationPanel.PanelHidSuccess2 which is quite flaky <https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebAuthenticationPanel.PanelHidSuccess2> and I reported that to the author.
http://trac.webkit.org/r251547
<rdar://problem/56590108>