Bug 203141 - ServiceWorker tests should use TCPServer instead of WKURLSchemeHandler
Summary: ServiceWorker tests should use TCPServer instead of WKURLSchemeHandler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-17 22:24 PDT by Alex Christensen
Modified: 2019-10-21 14:33 PDT (History)
3 users (show)

See Also:


Attachments
Patch (15.11 KB, patch)
2019-10-17 22:25 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (112.87 KB, patch)
2019-10-18 22:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (112.42 KB, patch)
2019-10-18 23:26 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (112.92 KB, patch)
2019-10-21 11:52 PDT, Alex Christensen
youennf: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-10-17 22:24:05 PDT
ServiceWorker tests should use TCPServer instead of WKURLSchemeHandler
Comment 1 Alex Christensen 2019-10-17 22:25:28 PDT
Created attachment 381273 [details]
Patch
Comment 2 Alex Christensen 2019-10-17 22:26:21 PDT
Comment on attachment 381273 [details]
Patch

This isn't done yet, but the end patch will be more of the same.  Feel free to give feedback.
Comment 3 Alex Christensen 2019-10-18 22:07:41 PDT
Created attachment 381367 [details]
Patch
Comment 4 Alex Christensen 2019-10-18 23:26:43 PDT
Created attachment 381368 [details]
Patch
Comment 5 youenn fablet 2019-10-21 11:30:27 PDT
Comment on attachment 381368 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        which is preventing us from moving logic to the NetworkProcess.  See bug 203055.

I did a quick hack for this one to work.
But it is simpler if we do not support interception for custom schemes.
I'll remove it in a follow-up.

> Source/WebCore/workers/service/server/SWServer.cpp:988
> +    return false;

Might early return with false in equalLettersIgnoringASCIICase(scheme.substring(0, 4), "http").

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1422
> +    [webView loadRequest:server.request()];

How do we get that request URL is regularPageGrabbingCacheStorageDirectory.html and not main.html which would go to mainBytes?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1990
> +    ServiceWorkerTCPServer server2({

How do we get host vs. host2 with server1 and server2?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:36
> +    ServiceWorkerTCPServer(Vector<ResourceInfo>&& vector)

explicit.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerTCPServer.h:69
> +    size_t userAgentsChecked() { return m_userAgentsChecked; }

const
Comment 6 Alex Christensen 2019-10-21 11:47:55 PDT
Comment on attachment 381368 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1422
>> +    [webView loadRequest:server.request()];
> 
> How do we get that request URL is regularPageGrabbingCacheStorageDirectory.html and not main.html which would go to mainBytes?

The server is set to respond to the second main resource request with the contents of regularPageGrabbingCacheStorageDirectory regardless of the request.  So the server does see two requests to /main.html but this test behaves as it did.
Comment 7 Alex Christensen 2019-10-21 11:52:10 PDT
Created attachment 381424 [details]
Patch
Comment 8 Alex Christensen 2019-10-21 11:55:21 PDT
Comment on attachment 381368 [details]
Patch

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

>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:1990
>> +    ServiceWorkerTCPServer server2({
> 
> How do we get host vs. host2 with server1 and server2?

The two servers had different ports, but I changed one to use localhost to be more explicit.
Comment 9 Alex Christensen 2019-10-21 13:15:01 PDT
EWS is green.  r?
Comment 10 Alex Christensen 2019-10-21 14:29:46 PDT
http://trac.webkit.org/r251384

(In reply to youenn fablet from comment #5)
> I did a quick hack for this one to work.
Where is this hack?
Comment 11 Radar WebKit Bug Importer 2019-10-21 14:30:25 PDT
<rdar://problem/56476957>
Comment 12 Alex Christensen 2019-10-21 14:33:07 PDT
(In reply to Alex Christensen from comment #10)
> Where is this hack?
I found it.  Bug 202309, r251124