WebCore.framework should restrict allowable_clients

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Aakash Jain]

On Mac OS X, WebCore.framework is beneath the WebKit umbrella framework, which hides its symbols from other frameworks and apps.

We should do something similar on iOS and make WebCore.framework completely private except for linking few specific frameworks and apps.

See <rdar://problem/15145065>.

[Aakash Jain]

Created attachment 308066 [details]
Proposed patch

[Aakash Jain]

Created attachment 308067 [details]
Proposed patch

Comment on attachment 308067 [details]
Proposed patch

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

> Source/WebCore/Configurations/WebCore.xcconfig:81
> +OTHER_LDFLAGS_BASE = -lsqlite3 -lobjc -lANGLE -framework Metal -allowable_client WebCoreTestSupport -allowable_client WebKit2 -allowable_client WebKitLegacy;

“-allowable_client WebKit2” doesn’t make sense because there’s no client with that name. Perhaps you meant “WebKit”.

[Alexey Proskuryakov]

Comment on attachment 308067 [details]
Proposed patch

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

> Source/WebCore/Configurations/WebCore.xcconfig:-85
> -OTHER_LDFLAGS_PLATFORM[sdk=macosx*] = $(OTHER_LDFLAGS_BASE) -sub_library libobjc -umbrella WebKit -allowable_client WebCoreTestSupport -allowable_client WebKit2 -allowable_client WebKitLegacy -framework ApplicationServices -framework AudioUnit -framework Carbon -framework Cocoa -framework CoreAudio -framework DataDetectorsCore -framework IOSurface -framework OpenGL -framework SystemConfiguration $(LIBWEBRTC_LDFLAGS);

Funny that "-allowable_client WebKit2" is existing code!

[Alexey Proskuryakov]

Comment on attachment 308067 [details]
Proposed patch

r- for Dan's comment, lack of TVBooks, and also this is breaking the build.

[Aakash Jain]

Created attachment 308118 [details]
Updated patch

Added WebKit, TVBooks as allowable_client.

[Alexey Proskuryakov]

Comment on attachment 308118 [details]
Updated patch

This is still breaking iOS build.

[Aakash Jain]

Created attachment 308126 [details]
Updated patch

Added DumpRenderTree as allowable_client, still testing the patch on my local machine.

[Alexey Proskuryakov]

Comment on attachment 308126 [details]
Updated patch

Now failing with TestWebKitAPI.

I wonder if we even need to link these to WebCore now.

[Aakash Jain]

Created attachment 308134 [details]
Updated patch

Added WebKitTestRunner and TestWebKitAPI as allowable_client.

Will check separately if TestWebKitAPI (and other similar projects) really need to link to WebCore. Will track in separate bugs.

[Aakash Jain]

Created attachment 308151 [details]
Updated patch

Removed WebKit from macOS to match previous configuration.

[Alexey Proskuryakov]

Comment on attachment 308151 [details]
Updated patch

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

Looks good to me.

> Source/WebCore/Configurations/WebCore.xcconfig:82
> +OTHER_LDFLAGS_BASE_ios = $(OTHER_LDFLAGS_BASE) -framework CFNetwork -framework CoreGraphics -framework CoreText -framework Foundation -framework ImageIO -framework MobileCoreServices -framework OpenGLES -lMobileGestalt $(WK_IOS_BINCOMPAT_LDFLAGS) -allowable_client WebKit -allowable_client iTunesU -allowable_client Casablanca -allowable_client Remote -allowable_client TVBooks -allowable_client DumpRenderTree -allowable_client WebKitTestRunner -allowable_client TestWebKitAPI;

Could you please add a comment with radar numbers for the ones we expect to remove eventually?

[Aakash Jain]

Created attachment 308160 [details]
Updated patch

Added FIXME.

[WebKit Commit Bot]

Comment on attachment 308160 [details]
Updated patch

Clearing flags on attachment: 308160

Committed r215769: <http://trac.webkit.org/changeset/215769>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.