205089 2019-12-10 16:02:45 -0800 Fix API test failure after r253351 2019-12-10 17:20:43 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 pvollan pvollan bfulgham commit-queue tsavell webkit-bug-importer oldest_to_newest 1597396 0 pvollan 2019-12-10 16:02:45 -0800 TestWebKitAPI.ContentFiltering.LazilyLoadPlatformFrameworks /Volumes/Data/worker/macOS-High-Sierra-Release-Build-EWS/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:364 Expected equality of these values: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: true static_cast<bool>(networkExtensionLoaded) Which is: false /Volumes/Data/worker/macOS-High-Sierra-Release-Build-EWS/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:364 Expected equality of these values: static_cast<bool>(networkExtensionShouldBeLoaded) Which is: true static_cast<bool>(networkExtensionLoaded) Which is: false 1597397 1 385313 pvollan 2019-12-10 16:04:56 -0800 Created attachment 385313 Patch 1597398 2 385313 bfulgham 2019-12-10 16:11:41 -0800 Comment on attachment 385313 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385313&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:392 > + method_setImplementation(method, reinterpret_cast<IMP>(filterRequired)); Could we have other clients that might need this? Or is this truly just a case of our test infrastructure needing this turned on. How do client applications that will need content filtering signal this fact (e.g., Safari) opt into this feature? 1597402 3 pvollan 2019-12-10 16:19:25 -0800 (In reply to Brent Fulgham from comment #2) > Comment on attachment 385313 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385313&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:392 > > + method_setImplementation(method, reinterpret_cast<IMP>(filterRequired)); > > Could we have other clients that might need this? Or is this truly just a > case of our test infrastructure needing this turned on. > > How do client applications that will need content filtering signal this fact > (e.g., Safari) opt into this feature? I don't believe other clients need this. The API test is testing whether the network extension framework is loaded in the WebContent process, and the patch in r253351 avoids loading the framework unless it is needed, introducing the failure. The test fix swizzles [NEFilterSource filterRequired] in the UI process, which will cause the framework to be loaded in the WebContent process. Thanks for reviewing! 1597404 4 385313 bfulgham 2019-12-10 16:22:11 -0800 Comment on attachment 385313 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385313&action=review r=me >>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:392 >>> + method_setImplementation(method, reinterpret_cast<IMP>(filterRequired)); >> >> Could we have other clients that might need this? Or is this truly just a case of our test infrastructure needing this turned on. >> >> How do client applications that will need content filtering signal this fact (e.g., Safari) opt into this feature? > > I don't believe other clients need this. The API test is testing whether the network extension framework is loaded in the WebContent process, and the patch in r253351 avoids loading the framework unless it is needed, introducing the failure. The test fix swizzles [NEFilterSource filterRequired] in the UI process, which will cause the framework to be loaded in the WebContent process. > > Thanks for reviewing! Sounds good. 1597419 5 pvollan 2019-12-10 16:52:19 -0800 (In reply to Brent Fulgham from comment #4) > Comment on attachment 385313 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=385313&action=review > > r=me > > >>> Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:392 > >>> + method_setImplementation(method, reinterpret_cast<IMP>(filterRequired)); > >> > >> Could we have other clients that might need this? Or is this truly just a case of our test infrastructure needing this turned on. > >> > >> How do client applications that will need content filtering signal this fact (e.g., Safari) opt into this feature? > > > > I don't believe other clients need this. The API test is testing whether the network extension framework is loaded in the WebContent process, and the patch in r253351 avoids loading the framework unless it is needed, introducing the failure. The test fix swizzles [NEFilterSource filterRequired] in the UI process, which will cause the framework to be loaded in the WebContent process. > > > > Thanks for reviewing! > > Sounds good. Thanks for reviewing :) 1597438 6 commit-queue 2019-12-10 17:18:22 -0800 The commit-queue encountered the following flaky tests while processing attachment 385313: imported/w3c/web-platform-tests/content-security-policy/unsafe-eval/eval-scripts-setTimeout-blocked.sub.html bug 203973 (author: dbates@webkit.org) The commit-queue is continuing to process your patch. 1597440 7 385313 commit-queue 2019-12-10 17:19:08 -0800 Comment on attachment 385313 Patch Clearing flags on attachment: 385313 Committed r253355: <https://trac.webkit.org/changeset/253355> 1597441 8 commit-queue 2019-12-10 17:19:09 -0800 All reviewed patches have been landed. Closing bug. 1597442 9 webkit-bug-importer 2019-12-10 17:20:43 -0800 <rdar://problem/57816052> 385313 2019-12-10 16:04:56 -0800 2019-12-10 17:19:08 -0800 Patch bug-205089-20191210160456.patch text/plain 2391 pvollan SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI1MzM1NCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE3IEBACisyMDE5LTEyLTEwICBQZXIgQXJuZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNv bT4KKworICAgICAgICBGaXggQVBJIHRlc3QgZmFpbHVyZSBhZnRlciByMjUzMzUxCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDUwODkKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBXaXRoIHRoZSBwYXRjaCBs YW5kZWQgaW4gcjI1MzM1MSwgdGhlIE5ldHdvcmtFeHRlbnNpb24gZnJhbWV3b3JrIHdpbGwgbm90 IGJlIGxvYWRlZCBpbiB0aGUgV2ViQ29udGVudCBwcm9jZXNzIHVubGVzcworICAgICAgICBbTkVG aWx0ZXJTb3VyY2UgZmlsdGVyUmVxdWlyZWRdIHJldHVybnMgWUVTLiBTd2l6emxlIHRoaXMgbWV0 aG9kIHRvIHJldHVybiBZRVMgaW4gdGhlIFVJIHByb2Nlc3MuCisKKyAgICAgICAgKiBUZXN0V2Vi S2l0QVBJL1Rlc3RzL1dlYktpdENvY29hL0NvbnRlbnRGaWx0ZXJpbmcubW06CisgICAgICAgIChm aWx0ZXJSZXF1aXJlZCk6CisgICAgICAgIChURVNUKToKKwogMjAxOS0xMi0xMCAgQ2hyaXMgRHVt ZXogIDxjZHVtZXpAYXBwbGUuY29tPgogCiAgICAgICAgIEd5cm9zY29wZSAoRGV2aWNlTW90aW9u IHJlbGF0ZWQpIHBlcm1pc3Npb24gY2Fubm90IGJlIHJlcXVlc3RlZCBpbiBXS1dlYlZpZXcgdW5s ZXNzIHRoZSBjbGllbnQgYXBwIHNldHMgYSBXS1VJRGVsZWdhdGUKSW5kZXg6IFRvb2xzL1Rlc3RX ZWJLaXRBUEkvVGVzdHMvV2ViS2l0Q29jb2EvQ29udGVudEZpbHRlcmluZy5tbQo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBUb29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dlYktpdENvY29hL0NvbnRlbnRGaWx0ZXJp bmcubW0JKHJldmlzaW9uIDI1MzM1MykKKysrIFRvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV2Vi S2l0Q29jb2EvQ29udGVudEZpbHRlcmluZy5tbQkod29ya2luZyBjb3B5KQpAQCAtMzgsNyArMzgs MTIgQEAKICNpbXBvcnQgPFdlYktpdC9fV0tEb3dubG9hZERlbGVnYXRlLmg+CiAjaW1wb3J0IDxX ZWJLaXQvX1dLUmVtb3RlT2JqZWN0SW50ZXJmYWNlLmg+CiAjaW1wb3J0IDxXZWJLaXQvX1dLUmVt b3RlT2JqZWN0UmVnaXN0cnkuaD4KKyNpbXBvcnQgPHBhbC9zcGkvY29jb2EvTkVGaWx0ZXJTb3Vy Y2VTUEkuaD4KICNpbXBvcnQgPHd0Zi9SZXRhaW5QdHIuaD4KKyNpbXBvcnQgPHd0Zi9Tb2Z0TGlu a2luZy5oPgorCitTT0ZUX0xJTktfRlJBTUVXT1JLX09QVElPTkFMKE5ldHdvcmtFeHRlbnNpb24p OworU09GVF9MSU5LX0NMQVNTX09QVElPTkFMKE5ldHdvcmtFeHRlbnNpb24sIE5FRmlsdGVyU291 cmNlKTsKIAogdXNpbmcgRGVjaXNpb24gPSBXZWJDb3JlOjpNb2NrQ29udGVudEZpbHRlclNldHRp bmdzOjpEZWNpc2lvbjsKIHVzaW5nIERlY2lzaW9uUG9pbnQgPSBXZWJDb3JlOjpNb2NrQ29udGVu dEZpbHRlclNldHRpbmdzOjpEZWNpc2lvblBvaW50OwpAQCAtMzc1LDggKzM4MCwxNyBAQCAtICh2 b2lkKXdlYlZpZXc6KFdLV2ViVmlldyAqKXdlYlZpZXcgZGlkCiAKIEBlbmQKIAorc3RhdGljIEJP T0wgZmlsdGVyUmVxdWlyZWQoaWQgc2VsZiwgU0VMIF9jbWQpCit7CisgICAgcmV0dXJuIFlFUzsK K30KKwogVEVTVChDb250ZW50RmlsdGVyaW5nLCBMYXppbHlMb2FkUGxhdGZvcm1GcmFtZXdvcmtz KQogeworICAgIC8vIFN3aXp6bGUgW05FRmlsdGVyU291cmNlIGZpbHRlclJlcXVpcmVkXSB0byBy ZXR1cm4gWUVTIGluIHRoZSBVSSBwcm9jZXNzIHNpbmNlIE5ldHdvcmtFeHRlbnNpb24gd2lsbCBu b3QgYmUgbG9hZGVkIG90aGVyd2lzZS4KKyAgICBNZXRob2QgbWV0aG9kID0gY2xhc3NfZ2V0Q2xh c3NNZXRob2QoZ2V0TkVGaWx0ZXJTb3VyY2VDbGFzcygpLCBAc2VsZWN0b3IoZmlsdGVyUmVxdWly ZWQpKTsKKyAgICBtZXRob2Rfc2V0SW1wbGVtZW50YXRpb24obWV0aG9kLCByZWludGVycHJldF9j YXN0PElNUD4oZmlsdGVyUmVxdWlyZWQpKTsKKwogICAgIEBhdXRvcmVsZWFzZXBvb2wgewogICAg ICAgICBhdXRvIGNvbnRyb2xsZXIgPSBhZG9wdE5TKFtbTGF6aWx5TG9hZFBsYXRmb3JtRnJhbWV3 b3Jrc0NvbnRyb2xsZXIgYWxsb2NdIGluaXRdKTsKICAgICAgICAgW2NvbnRyb2xsZXIgZXhwZWN0 UGFyZW50YWxDb250cm9sc0xvYWRlZDpOTyBuZXR3b3JrRXh0ZW5zaW9uTG9hZGVkOk5PXTsK