rwt should allow service worker to load localhost HTTPS resources with any certificate
Created attachment 325358 [details] Patch
Comment on attachment 325358 [details] Patch Attachment 325358 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5041249 New failing tests: http/tests/workers/service/service-worker-fetch.https.html
Created attachment 325361 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 325358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325358&action=review > Source/WebKit/UIProcess/WebProcessPool.cpp:1394 > +#if ENABLE(SERVICE_WORKERS) ah ah ah You have an extra S at the end of WORKER so this code does nothing :D
Comment on attachment 325358 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325358&action=review >> Source/WebKit/UIProcess/WebProcessPool.cpp:1394 >> +#if ENABLE(SERVICE_WORKERS) > > ah ah ah You have an extra S at the end of WORKER so this code does nothing :D I made the same mistake twice already!
Comment on attachment 325358 [details] Patch Please don't add this or anything like it to WKContext/WKProcessPool. Just respond to the challenge. We're also trying to remove the step of canAuthenticateAgainstProtectionSpace. This would be a strange step in the wrong direction.
(In reply to Alex Christensen from comment #6) > Comment on attachment 325358 [details] > Patch > > Please don't add this or anything like it to WKContext/WKProcessPool. Just > respond to the challenge. > We're also trying to remove the step of > canAuthenticateAgainstProtectionSpace. This would be a strange step in the > wrong direction. Challenge is per page, no? A ServiceWorker is not associated with a page.
A delegate should probably be per-WebsiteDataStore
Created attachment 325388 [details] Patch
Comment on attachment 325388 [details] Patch Attachment 325388 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5044523 New failing tests: http/tests/workers/service/service-worker-fetch.https.html
Created attachment 325395 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 325388 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325388&action=review > Source/WebKit/UIProcess/ServiceWorkerProcessProxy.cpp:61 > + challenge->useCredential(nullptr); Here is what WKTR is doing in this case: if (WKProtectionSpaceGetAuthenticationScheme(protectionSpace) == kWKProtectionSpaceAuthenticationSchemeServerTrustEvaluationRequested) { // Any non-empty credential signals to accept the server trust. Since the cross-platform API // doesn't expose a way to create a credential from server trust, we use a password credential. WKRetainPtr<WKCredentialRef> credential = adoptWK(WKCredentialCreate(toWK("accept server trust").get(), toWK("").get(), kWKCredentialPersistenceNone)); WKAuthenticationDecisionListenerUseCredential(decisionListener, credential.get()); return; } I believe the parameter should not be nullptr. You need to construct credentials as WKTR does.
Created attachment 325451 [details] Patch
Comment on attachment 325451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325451&action=review > Source/WebKit/UIProcess/ServiceWorkerProcessProxy.cpp:69 > + ASSERT_NOT_REACHED(); I don't think it is a valid assertion. What prevents regular HTTP authentication to happen in a service worker? Also, I think we should deal with the challenge either way (either cancel or probably better, do the default behavior).
Created attachment 325455 [details] Tess that can be unskipped for me locally with certificate fix
Comment on attachment 325451 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325451&action=review >> Source/WebKit/UIProcess/ServiceWorkerProcessProxy.cpp:69 >> + ASSERT_NOT_REACHED(); > > I don't think it is a valid assertion. What prevents regular HTTP authentication to happen in a service worker? > Also, I think we should deal with the challenge either way (either cancel or probably better, do the default behavior). I will replace it with notImplemented so that it is clear that things are missing here and that we should really delegate this to the app.
Created attachment 325459 [details] Patch
Created attachment 325474 [details] Patch
Comment on attachment 325474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=325474&action=review r=me > LayoutTests/TestExpectations:140 > webkit.org/b/179035 imported/w3c/web-platform-tests/service-workers/cache-storage [ Skip ] Will likely need rebaselining now that this was fixed.
> > LayoutTests/TestExpectations:140 > > webkit.org/b/179035 imported/w3c/web-platform-tests/service-workers/cache-storage [ Skip ] > > Will likely need rebaselining now that this was fixed. Let's follow up once it is landed.
Comment on attachment 325474 [details] Patch Rejecting attachment 325474 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 325474, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: eb-platform-tests/streams/readable-streams/readable-stream-reader.serviceworker.https-expected.txt patching file LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/tee.serviceworker.https-expected.txt patching file LayoutTests/imported/w3c/web-platform-tests/streams/readable-streams/templated.serviceworker.https-expected.txt Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Chris Dumez']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/5054351
(In reply to youenn fablet from comment #20) > > > LayoutTests/TestExpectations:140 > > > webkit.org/b/179035 imported/w3c/web-platform-tests/service-workers/cache-storage [ Skip ] > > > > Will likely need rebaselining now that this was fixed. > > Let's follow up once it is landed. My point is that it will likely conflict and not land. But let's see if you're lucky.
(In reply to Chris Dumez from comment #22) > (In reply to youenn fablet from comment #20) > > > > LayoutTests/TestExpectations:140 > > > > webkit.org/b/179035 imported/w3c/web-platform-tests/service-workers/cache-storage [ Skip ] > > > > > > Will likely need rebaselining now that this was fixed. > > > > Let's follow up once it is landed. > > My point is that it will likely conflict and not land. But let's see if > you're lucky. Well, surprise :D
Created attachment 325485 [details] Patch for landing
Comment on attachment 325485 [details] Patch for landing You did not unskip the tests under imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/ :(
Created attachment 325495 [details] Patch
Comment on attachment 325495 [details] Patch Clearing flags on attachment: 325495 Committed r224250: <https://trac.webkit.org/changeset/224250>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35567860>