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
Created attachment 385313 [details] Patch
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?
(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!
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.
(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 :)
The commit-queue encountered the following flaky tests while processing attachment 385313 [details]: 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.
Comment on attachment 385313 [details] Patch Clearing flags on attachment: 385313 Committed r253355: <https://trac.webkit.org/changeset/253355>
All reviewed patches have been landed. Closing bug.
<rdar://problem/57816052>