Summary: | [Cocoa] Remove soft linking of NetworkExtension.framework | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||||
Component: | New Bugs | Assignee: | Aditya Keerthi <akeerthi> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aestes, akeerthi, mitz, thorton, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=218289 | ||||||||||||
Attachments: |
|
Description
Ryan Haddad
2020-10-28 16:20:44 PDT
Test history suggests this started with https://trac.webkit.org/changeset/269109/webkit Created attachment 412602 [details]
Patch
Created attachment 412606 [details]
Patch
Created attachment 412607 [details]
Patch
Will need to add a HAVE macro to avoid compiling NetworkExtensionContentFilter on tvOS. 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. (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. (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). Created attachment 412656 [details]
Patch
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. (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. |