Bug 205089 – Fix API test failure after r253351

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Per Arne Vollan]

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

[Per Arne Vollan]

Created attachment 385313 [details]
Patch

[Brent Fulgham]

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?

[Per Arne Vollan]

(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!

[Brent Fulgham]

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.

[Per Arne Vollan]

(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 :)

[WebKit Commit Bot]

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: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 385313 [details]
Patch

Clearing flags on attachment: 385313

Committed r253355: <https://trac.webkit.org/changeset/253355>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57816052>