[webkit-reviews] review granted: [Bug 42796] Add Xcode support for compiling WebKit against iOS SDKs. : [Attachment 63094] patch

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Andy Estes
<aestes at apple.com>'s request for review:
Bug 42796: Add Xcode support for compiling WebKit against iOS SDKs.
https://bugs.webkit.org/show_bug.cgi?id=42796

Attachment 63094: patch
https://bugs.webkit.org/attachment.cgi?id=63094&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
WebCore/Configurations/WebCore.xcconfig:39
 +  FRAMEWORK_SEARCH_PATHS_macosx =
$(SYSTEM_LIBRARY_DIR)/Frameworks/Carbon.framework/Frameworks
$(SYSTEM_LIBRARY_DIR)/Frameworks/ApplicationServices.framework/Frameworks
$(FRAMEWORK_SEARCH_PATHS);

I've been trying to put *_macosx macros after *_iphoneos and *_iphonesimulator
macros.


WebCore/Configurations/WebCore.xcconfig:41
 +  FRAMEWORK_SEARCH_PATHS_iphonesimulator =
$(FRAMEWORK_SEARCH_PATHS_iphoneos_$(CONFIGURATION));

This should come after the *_iphoneos macro.


WebCore/Configurations/WebCore.xcconfig:53
 +  UMBRELLA_LDFLAGS = -umbrella WebKit;

I would just inline this variable into OTHER_LDFLAGS_macosx since it's only
used once.


WebCore/Configurations/WebCore.xcconfig:81
 +  EXCLUDED_SOURCE_FILE_NAMES_iphoneos = *.tiff *Cursor.png
localizedStrings.js AccessibilityObjectMac.mm AccessibilityObjectWrapper.mm
AXObjectCacheMac.mm ClipboardMac.mm ColorMac.mm CursorMac.mm EditorMac.mm
EventHandlerMac.mm EventLoopMac.mm FontMacATSUI.mm GeolocationServiceMac.mm
IconDatabase.cpp IconMac.mm KeyEventMac.mm LocalCurrentGraphicsContext.mm
LocalizedStringsMac.mm MIMETypeRegistryMac.mm MediaPlayerPrivateQTKit.mm
NetworkStateNotifierMac.cpp PasteboardMac.mm PlatformMouseEventMac.mm
PlatformScreenMac.mm PluginMainThreadScheduler.cpp PopupMenuMac.mm
ScriptObjectQuarantine.cpp ScrollViewMac.mm ScrollbarThemeMac.mm
SharedTimerMac.mm SoundMac.mm SystemTimeMac.cpp ThemeMac.mm ThreadCheck.mm
WebCoreSystemInterface.mm WebCoreURLResponse.mm WebCoreView.m WebFontCache.mm
WheelEventMac.mm WidgetMac.mm;

Please remove source files (*.mm, *.cpp) files from this list.	Long term, I
think we want to use #if PLATFORM(IPHONE)/#endif and #if PLATFORM(MAC)/#endif
macros around the source files and "compile everything".


WebCore/Configurations/WebCore.xcconfig:83
 +  EXCLUDED_SOURCE_FILE_NAMES_macosx = ;

This variable may be removed since it's empty.


WebKit/mac/Configurations/DebugRelease.xcconfig:55
 +  WEBKIT_SYSTEM_INTERFACE_LIBRARY_macosx =
$(WEBKIT_SYSTEM_INTERFACE_LIBRARY_macosx_$(MAC_OS_X_VERSION_MAJOR));

This should use TARGET_MAC_OS_X_VERSION_MAJOR.


WebKit/mac/Configurations/DebugRelease.xcconfig:60
 +  WEBKIT_SYSTEM_INTERFACE_LIBRARY_1070 = WebKitSystemInterfaceSnowLeopard;

This needs to be named WEBKIT_SYSTEM_INTERFACE_LIBRARY_macosx_1070.


WebKit/mac/Configurations/WebKit.xcconfig:28
 +  EXCLUDED_SOURCE_FILE_NAMES_iphoneos = *.nib *.tiff CarbonUtils.m
CarbonWindowAdapter.mm CarbonWindowContentView.m CarbonWindowFrame.m
HIViewAdapter.m HIWebView.mm MainThreadObjectDeallocator.mm WebClipView.m
WebDragClient.mm WebDynamicScrollBarsView.mm WebInspector.mm
WebInspectorClient.mm WebNetscapeContainerCheckContextInfo.mm
WebNetscapeContainerCheckPrivate.mm WebNodeHighlight.mm WebNodeHighlightView.mm
WebRenderNode.mm WebStringTruncator.mm WebTextCompletionController.mm;

Please remove source file names (*.m, *.mm, *.cpp) from this list.  As
mentioned previously, we want to handle this using #if PLATFORM(IPHONE)/#endif
and #if PLATFORM(MAC)/#endif macros instead.


WebKit/mac/Configurations/WebKit.xcconfig:30
 +  EXCLUDED_SOURCE_FILE_NAMES_macosx = ;

This line may be removed since it's blank.


Nice job.  I know that was a lot of work!  r=me with the fixes above.