The GTK API test WebKit2Gtk/TestSSL is failing since r173423 <http://trac.webkit.org/r173423> $ Tools/Scripts/run-gtk-tests --verbose --release WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestSSL TEST: WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestSSL... (pid=32700) /webkit2/WebKitWebView/ssl: ** ERROR:../../Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestSSL.cpp:72:void testSSL(SSLTest*, gconstpointer): assertion failed: (test->m_certificate) FAIL GTester: last random seed: R02Sc9ca19c165138113981a6793268440f9 (pid=312) /webkit2/WebKitWebView/insecure-content: OK /webkit2/WebKitWebView/tls-errors-policy: OK /webkit2/WebKitWebView/tls-errors-redirect-to-http: OK /webkit2/WebKitWebView/load-failed-with-tls-errors: OK FAIL: WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestSSL Tests failed (1): WebKitBuild/Release/bin/TestWebKitAPI/WebKit2Gtk/TestSSL
We don't include certificate info to subresources anymore. Maybe just the assert needs fixing?
(In reply to comment #1) > We don't include certificate info to subresources anymore. Maybe just the assert needs fixing? Maybe. I guess that cgarcia can check better than me if this is the case. In the meanwhile. I skipped the test on http://trac.webkit.org/changeset/173435
(In reply to comment #1) > We don't include certificate info to subresources anymore. Maybe just the assert needs fixing? No, we are not testing the certificate of a subresource, there aren't even subresources in the html of the test. What we are testing here is that the view has a certifciate once the load has been committed, and we use mainFrame->certificateInfo()->certificateInfo() for that.
The problem is that only the network process calls ResourceResponse::includeCertificateInfo(), and our test is using the single shared secondary process model.
Ah ok. Would it be difficult for you to move to using network process everywhere? Eliminating WK2-without-network-process mode would simplify the code and avoid this sort of issues.
(In reply to comment #5) > Ah ok. Would it be difficult for you to move to using network process everywhere? Eliminating WK2-without-network-process mode would simplify the code and avoid this sort of issues. Yes, I'm afraid so, we have the single shared secondary process model exposed in our API, and used by applications. I think it's the best model for most applications, except web browsers. Are there any plans to remove that model? Would it be possible to implement this in WebCore by adding a ResourceLoaderOption? a CertificateInfoPolicy that will be only set as IncludeCertificateInfo for the main resource loader.
I don't think there are any plans to remove it. Having less configurations to support would be nice though. Yeah, solution along those lines makes sense.
(In reply to comment #7) > I don't think there are any plans to remove it. Having less configurations to support would be nice though. phew, good to know, thanks :-) > Yeah, solution along those lines makes sense. Ok, I'll try that approach then
Created attachment 238023 [details] Patch
Created attachment 238024 [details] Rebased to apply on current git master
Comment on attachment 238024 [details] Rebased to apply on current git master View in context: https://bugs.webkit.org/attachment.cgi?id=238024&action=review > Source/WebCore/loader/DocumentLoader.cpp:1435 > + static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AskClientForAllCredentials, SkipSecurityCheck, UseDefaultOriginRestrictionsForType, IncludeCertificateInfo); It would be nice to refactor ResourceLoaderOptions to be in the style of ViewState::Flags and LayerFlushThrottleState::Flags. The current struct is very expansion unfriendly.
(In reply to comment #11) > (From update of attachment 238024 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238024&action=review Thanks for the review. > > Source/WebCore/loader/DocumentLoader.cpp:1435 > > + static NeverDestroyed<ResourceLoaderOptions> mainResourceLoadOptions(SendCallbacks, SniffContent, BufferData, AllowStoredCredentials, AskClientForAllCredentials, SkipSecurityCheck, UseDefaultOriginRestrictionsForType, IncludeCertificateInfo); > > It would be nice to refactor ResourceLoaderOptions to be in the style of ViewState::Flags and LayerFlushThrottleState::Flags. The current struct is very expansion unfriendly. Definitely
Committed r173558: <http://trac.webkit.org/changeset/173558>