162962 2016-10-05 02:09:23 -0700 [SOUP] Ensure NetworkProcess is ready before whitelisting TLS certificates 2017-02-02 03:30:25 -0800 1 1 1 Unclassified WebKit WebKit2 WebKit Local Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 emanuele.aina webkit-unassigned achristensen andersca cgarcia fpizlo ggaren mcatanzaro sam simon.fraser oldest_to_newest 1236577 0 emanuele.aina 2016-10-05 02:09:23 -0700 If the API user tries to whitelist TLS certificates before any web view has been created, the action will be ignored because the NetworkProcess hasn't been fired up yet. For example, the snippet below using the GTK+ API does not work, unless the whitelisting is moved after the web view creation: webkit_web_context_allow_tls_certificate_for_host(webkit_web_context_get_default(), crt, host); webView = webkit_web_view_new(); 1236578 1 290698 emanuele.aina 2016-10-05 02:11:28 -0700 Created attachment 290698 Patch 1236634 2 achristensen 2016-10-05 08:42:51 -0700 I'm not opposed to this change, but long-term we're hoping to get rid of it. See discussion in https://bugs.webkit.org/show_bug.cgi?id=162910 Is it still wanted? 1236808 3 emanuele.aina 2016-10-05 15:59:45 -0700 I just stumbled on this unexpected behaviour where the whitelisting silently failed, I'll leave any other consideration to Michael and Carlos. :) 1236810 4 290698 achristensen 2016-10-05 16:01:05 -0700 Comment on attachment 290698 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290698&action=review r=me, but hopefully we'll remove this whole feature soon. You can decide if you want to land it. > w/Source/WebKit2/UIProcess/WebProcessPool.cpp:1149 > + ASSERT(m_networkProcess); not necessary 1236829 5 mcatanzaro 2016-10-05 16:38:39 -0700 We can't remove the feature, it's part of our API and we intend to support it indefinitely. What we can do is reimplement the feature at the GTK API layer using whatever UI process callbacks get added, then it can be removed from the cross-platform WebKit2 code. 1236831 6 290698 mcatanzaro 2016-10-05 16:46:36 -0700 Comment on attachment 290698 Patch Indeed, the ASSERT is not helpful here. This looks easy to test, so if you don't want to risk it breaking in the next refactoring, it would be nice if you could try to add a test for it in TestSSL.cpp. The only trick is that you can't use the nice SSLTest class since that doesn't give you the ability to use the WebKitWebContext before the WebKitWebView is created; you'd have to roll your own that inherits from Test rather than LoadTrackingTest/WebViewTest. 1253122 7 mcatanzaro 2016-11-19 17:07:15 -0800 Emanuele, I think you should commit this (without the ASSERT) even if you don't have time to write a test. 1262555 8 mcatanzaro 2016-12-23 09:11:26 -0800 Did this get forgotten? 1262816 9 mcatanzaro 2016-12-27 09:57:50 -0800 Let's land this. 1262817 10 mcatanzaro 2016-12-27 09:58:08 -0800 Committed r210180: <http://trac.webkit.org/changeset/210180> 1272426 11 emanuele.aina 2017-02-02 03:30:25 -0800 I tried to come up with a test, but then I failed to find the time to finish it. Thank you for taking care of the patch! 290698 2016-10-05 02:11:28 -0700 2016-10-05 16:46:36 -0700 Patch bug-162962-20161005110815.patch text/plain 2320 emanuele.aina U3VidmVyc2lvbiBSZXZpc2lvbjogMjA2ODA0CmRpZmYgLS1naXQgYy9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgdy9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggODBhOWE0YmYzODFiMGVi YjAyNzk4NWYwMjVkMDgwMGQ2OTU2OTZmYS4uOWJmOTc4N2NjZDFiMzJjZWI1OWRjZGQyZjVkNjAy ZjY5NjM3NGIxZSAxMDA2NDQKLS0tIGMvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyB3L1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2IEBACisyMDE2LTEwLTA1ICBFbWFu dWVsZSBBaW5hICA8ZW1hbnVlbGUuYWluYUBjb2xsYWJvcmEuY29tPgorCisgICAgICAgIEVuc3Vy ZSBOZXR3b3JrUHJvY2VzcyBpcyByZWFkeSBiZWZvcmUgd2hpdGVsaXN0aW5nIFRMUyBjZXJ0aWZp Y2F0ZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2 Mjk2MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIElm IHRoZSBBUEkgdXNlciB0cmllcyB0byB3aGl0ZWxpc3QgVExTIGNlcnRpZmljYXRlcyBiZWZvcmUg YW55IHdlYgorICAgICAgICB2aWV3IGhhcyBiZWVuIGNyZWF0ZWQsIHRoZSBhY3Rpb24gd2lsbCBi ZSBpZ25vcmVkIGJlY2F1c2UgdGhlCisgICAgICAgIE5ldHdvcmtQcm9jZXNzIGhhc24ndCBiZWVu IGZpcmVkIHVwIHlldC4KKworICAgICAgICBGb3IgZXhhbXBsZSwgdGhlIHNuaXBwZXQgYmVsb3cg dXNpbmcgdGhlIEdUSysgQVBJIGRvZXMgbm90IHdvcmssCisgICAgICAgIHVubGVzcyB0aGUgd2hp dGVsaXN0aW5nIGlzIG1vdmVkIGFmdGVyIHRoZSB3ZWIgdmlldyBjcmVhdGlvbjoKKworICAgICAg ICAgIHdlYmtpdF93ZWJfY29udGV4dF9hbGxvd190bHNfY2VydGlmaWNhdGVfZm9yX2hvc3Qod2Vi a2l0X3dlYl9jb250ZXh0X2dldF9kZWZhdWx0KCksIGNydCwgaG9zdCk7CisgICAgICAgICAgd2Vi VmlldyA9IHdlYmtpdF93ZWJfdmlld19uZXcoKTsKKworICAgICAgICAqIFVJUHJvY2Vzcy9XZWJQ cm9jZXNzUG9vbC5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYlByb2Nlc3NQb29sOjphbGxvd1Nw ZWNpZmljSFRUUFNDZXJ0aWZpY2F0ZUZvckhvc3QpOiBFbnN1cmUKKyAgICAgICAgdGhhdCB0aGUg TmV0d29ya1Byb2Nlc3MgaGFzIGJlZW4gYWxyZWFkeSBmaXJlZCB1cCB3aGVuIGFkZGluZworICAg ICAgICBjZXJ0aWZpY2F0ZXMsIHNvIHRoZXkgY2FuIGJlIHdoaXRlbGlzdGVkIGJlZm9yZSB0aGUg Zmlyc3Qgd2ViIHZpZXcgaXMKKyAgICAgICAgaW5zdGFudGlhdGVkLgorCiAyMDE2LTEwLTA0ICBD aHJpcyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgSW1wbGVtZW50IEtleWJv YXJkRXZlbnQuY29kZSBmcm9tIHRoZSBVSSBFdmVudCBzcGVjCmRpZmYgLS1naXQgYy9Tb3VyY2Uv V2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vzc1Bvb2wuY3BwIHcvU291cmNlL1dlYktpdDIvVUlQ cm9jZXNzL1dlYlByb2Nlc3NQb29sLmNwcAppbmRleCA0YjIzN2Y3NGY5MmVlNzdmNTQyMmM1NGYz YzJkMmFkZWYzYmE5NWJkLi4xMzk1OTI5MGU2ZWY2NzgxMmUyNGRlZTdlODdiZDc4ZTNhOGUyYzli IDEwMDY0NAotLS0gYy9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUHJvY2Vzc1Bvb2wuY3Bw CisrKyB3L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQcm9jZXNzUG9vbC5jcHAKQEAgLTEx NDUsOCArMTE0NSw5IEBAIHZvaWQgV2ViUHJvY2Vzc1Bvb2w6OnRlcm1pbmF0ZURhdGFiYXNlUHJv Y2VzcygpCiAKIHZvaWQgV2ViUHJvY2Vzc1Bvb2w6OmFsbG93U3BlY2lmaWNIVFRQU0NlcnRpZmlj YXRlRm9ySG9zdChjb25zdCBXZWJDZXJ0aWZpY2F0ZUluZm8qIGNlcnRpZmljYXRlLCBjb25zdCBT dHJpbmcmIGhvc3QpCiB7Ci0gICAgaWYgKG1fbmV0d29ya1Byb2Nlc3MpCi0gICAgICAgIG1fbmV0 d29ya1Byb2Nlc3MtPnNlbmQoTWVzc2FnZXM6Ok5ldHdvcmtQcm9jZXNzOjpBbGxvd1NwZWNpZmlj SFRUUFNDZXJ0aWZpY2F0ZUZvckhvc3QoY2VydGlmaWNhdGUtPmNlcnRpZmljYXRlSW5mbygpLCBo b3N0KSwgMCk7CisgICAgZW5zdXJlTmV0d29ya1Byb2Nlc3MoKTsKKyAgICBBU1NFUlQobV9uZXR3 b3JrUHJvY2Vzcyk7CisgICAgbV9uZXR3b3JrUHJvY2Vzcy0+c2VuZChNZXNzYWdlczo6TmV0d29y a1Byb2Nlc3M6OkFsbG93U3BlY2lmaWNIVFRQU0NlcnRpZmljYXRlRm9ySG9zdChjZXJ0aWZpY2F0 ZS0+Y2VydGlmaWNhdGVJbmZvKCksIGhvc3QpLCAwKTsKIH0KIAogdm9pZCBXZWJQcm9jZXNzUG9v bDo6dXBkYXRlQXV0b21hdGlvbkNhcGFiaWxpdGllcygpIGNvbnN0Cg==