Bug 218314 – [Cocoa] Remove soft linking of NetworkExtension.framework

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

TestWebKitAPI.ContentFiltering.LazilyLoadPlatformFrameworks
        
        /Volumes/Data/slave/catalina-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:374
        Expected equality of these values:
          static_cast<bool>(networkExtensionShouldBeLoaded)
            Which is: false
          static_cast<bool>(networkExtensionLoaded)
            Which is: true
        
        
        /Volumes/Data/slave/catalina-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:374
        Expected equality of these values:
          static_cast<bool>(networkExtensionShouldBeLoaded)
            Which is: false
          static_cast<bool>(networkExtensionLoaded)
            Which is: true
        
        
        /Volumes/Data/slave/catalina-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:374
        Expected equality of these values:
          static_cast<bool>(networkExtensionShouldBeLoaded)
            Which is: false
          static_cast<bool>(networkExtensionLoaded)
            Which is: true
        
        
        /Volumes/Data/slave/catalina-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:374
        Expected equality of these values:
          static_cast<bool>(networkExtensionShouldBeLoaded)
            Which is: false
          static_cast<bool>(networkExtensionLoaded)
            Which is: true
        
        
        /Volumes/Data/slave/catalina-release/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/ContentFiltering.mm:374
        Expected equality of these values:
          static_cast<bool>(networkExtensionShouldBeLoaded)
            Which is: false
          static_cast<bool>(networkExtensionLoaded)
            Which is: true

https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.ContentFiltering.LazilyLoadPlatformFrameworks

[Ryan Haddad]

Test history suggests this started with https://trac.webkit.org/changeset/269109/webkit

[Radar WebKit Bug Importer]

<rdar://problem/70785239>

[Aditya Keerthi]

Created attachment 412602 [details]
Patch

[Aditya Keerthi]

Created attachment 412606 [details]
Patch

[Aditya Keerthi]

Created attachment 412607 [details]
Patch

[Aditya Keerthi]

Will need to add a HAVE macro to avoid compiling NetworkExtensionContentFilter on tvOS.

[Andy Estes]

Comment on attachment 412607 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412607&action=review

> Source/WebCore/Configurations/WebCore.xcconfig:114
> +WK_NETWORK_EXTENSION_LD_FLAGS = $(WK_NETWORK_EXTENSION_LD_FLAGS_$(WK_PLATFORM_NAME));
> +WK_NETWORK_EXTENSION_LD_FLAGS_iphoneos = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_iphonesimulator = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_watchos = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_watchsimulator = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_maccatalyst = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_macosx = -weak_framework NetworkExtension;

A simplification of this would be:

    WK_NETWORK_EXTENSION_LD_FLAGS = $(WK_NETWORK_EXTENSION_LD_FLAGS_$(WK_COCOA_TOUCH));
    WK_NETWORK_EXTENSION_LD_FLAGS_cocoatouch = -framework NetworkExtension;
    WK_NETWORK_EXTENSION_LD_FLAGS_ = -weak_framework NetworkExtension;

This would also fix the tvOS build.

> Source/WebCore/platform/cocoa/NetworkExtensionContentFilter.h:57
> +    WEBCORE_EXPORT static bool filterSourceRequiresFilter();

Nit: I think `filterSourceRequired` or `isRequired` would say enough.

[Andy Estes]

(In reply to Aditya Keerthi from comment #6)
> Will need to add a HAVE macro to avoid compiling
> NetworkExtensionContentFilter on tvOS.

I think the problem is just that you aren't linking the framework on tvOS.

[Aditya Keerthi]

(In reply to Andy Estes from comment #8)
> (In reply to Aditya Keerthi from comment #6)
> > Will need to add a HAVE macro to avoid compiling
> > NetworkExtensionContentFilter on tvOS.
> 
> I think the problem is just that you aren't linking the framework on tvOS.

The framework is not available on tvOS (https://developer.apple.com/documentation/networkextension).

[Aditya Keerthi]

Created attachment 412656 [details]
Patch

[EWS]

Committed r269173: <https://trac.webkit.org/changeset/269173>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412656 [details].

Comment on attachment 412656 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412656&action=review

> Source/WebCore/Configurations/WebCore.xcconfig:114
> +WK_NETWORK_EXTENSION_LD_FLAGS = $(WK_NETWORK_EXTENSION_LD_FLAGS_$(WK_PLATFORM_NAME));
> +WK_NETWORK_EXTENSION_LD_FLAGS_iphoneos = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_iphonesimulator = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_watchos = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_watchsimulator = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_maccatalyst = -framework NetworkExtension;
> +WK_NETWORK_EXTENSION_LD_FLAGS_macosx = -weak_framework NetworkExtension;

If this is going to be a recurring pattern, you should consider instead introducing something like

WK_MACOS_WEAK_FRAMEWORK = -framework;
WK_MACOS_WEAK_FRAMEWORK[sdk=macosx*] = -weak_framework;

in Base.xcconfig and then just adding “$(WK_MACOS_WEAK_FRAMEWORK) NetworkExtension” to OTHER_LDFLAGS.

[Aditya Keerthi]

(In reply to mitz from comment #12)
> Comment on attachment 412656 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=412656&action=review
> 
> > Source/WebCore/Configurations/WebCore.xcconfig:114
> > +WK_NETWORK_EXTENSION_LD_FLAGS = $(WK_NETWORK_EXTENSION_LD_FLAGS_$(WK_PLATFORM_NAME));
> > +WK_NETWORK_EXTENSION_LD_FLAGS_iphoneos = -framework NetworkExtension;
> > +WK_NETWORK_EXTENSION_LD_FLAGS_iphonesimulator = -framework NetworkExtension;
> > +WK_NETWORK_EXTENSION_LD_FLAGS_watchos = -framework NetworkExtension;
> > +WK_NETWORK_EXTENSION_LD_FLAGS_watchsimulator = -framework NetworkExtension;
> > +WK_NETWORK_EXTENSION_LD_FLAGS_maccatalyst = -framework NetworkExtension;
> > +WK_NETWORK_EXTENSION_LD_FLAGS_macosx = -weak_framework NetworkExtension;
> 
> If this is going to be a recurring pattern, you should consider instead
> introducing something like
> 
> WK_MACOS_WEAK_FRAMEWORK = -framework;
> WK_MACOS_WEAK_FRAMEWORK[sdk=macosx*] = -weak_framework;
> 
> in Base.xcconfig and then just adding “$(WK_MACOS_WEAK_FRAMEWORK)
> NetworkExtension” to OTHER_LDFLAGS.

Thanks! Will keep that in mind for future changes in this area.

Though in this case we cannot unconditionally add “$(WK_MACOS_WEAK_FRAMEWORK) NetworkExtension” to OTHER_LDFLAGS, due to the exclusion of tvOS.