| Summary: | [iOS] Add WebKitSystemInterface for iOS 8.1 | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
| Component: | Tools / Tests | Assignee: | Daniel Bates <dbates> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aestes, ap, darin, ddkilzer, lforschler, mjs, mrowe, simon.fraser | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | iPhone / iPad | ||||||
| OS: | iOS 8.1 | ||||||
| Bug Depends on: | 139829 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Daniel Bates
2014-12-19 12:59:15 PST
Created attachment 243570 [details]
Patch
Comment on attachment 243570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243570&action=review > WebKitLibraries/WebKitSystemInterfaceIOS.h:2 > + * WebKitSystemInterfaceIOS.mm Oops. (In reply to comment #2) > Comment on attachment 243570 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243570&action=review > > > WebKitLibraries/WebKitSystemInterfaceIOS.h:2 > > + * WebKitSystemInterfaceIOS.mm > > Oops. Will fix before landing. Will also remove extraneous whitespace from the file. Comment on attachment 243570 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243570&action=review I'm going to say r=me, but I don't understand why we need more WKSI. We wanted to get rid of it completely, not to add more flavors. > Tools/Scripts/copy-webkitlibraries-to-product-directory:147 > + if (willUseIOSDeviceSDKWhenBuilding()) { Is there a downside to copying them all ,like we do for OS X? At least the code would be simpler. > Tools/Scripts/copy-webkitlibraries-to-product-directory:174 > dittoHeaders("WebKitLibraries/WebKitSystemInterface.h", "$productDir/usr/local/include/WebKitSystemInterface.h"); > + dittoHeaders("WebKitLibraries/WebKitSystemInterfaceIOS.h", "$productDir/usr/local/include/WebKitSystemInterfaceIOS.h") if isIOSWebKit(); This setup is sort of weird. We have iOS ifdefs in WebKitSystemInterface.h, plus we also have a separate header. I don't know if it would be better to make a separate Mac file, or to put everything into WebKitSystemInterface.h, but what we have now is asymmetric. (In reply to comment #4) > I'm going to say r=me, but I don't understand why we need more WKSI. We > wanted to get rid of it completely, not to add more flavors. > In the long term we want to remove WKSI. For now, I'll upstream WKSI for iOS so as to expedite the bring up of iOS WebKit using the public iOS SDK. > > Tools/Scripts/copy-webkitlibraries-to-product-directory:147 > > + if (willUseIOSDeviceSDKWhenBuilding()) { > > Is there a downside to copying them all ,like we do for OS X? At least the > code would be simpler. > Disregarding disk space (the files are small), there isn't a downside to copying both the device and simulator WKSI. Will simplify code to copy both the device and simulator WKSI. > > Tools/Scripts/copy-webkitlibraries-to-product-directory:174 > > dittoHeaders("WebKitLibraries/WebKitSystemInterface.h", "$productDir/usr/local/include/WebKitSystemInterface.h"); > > + dittoHeaders("WebKitLibraries/WebKitSystemInterfaceIOS.h", "$productDir/usr/local/include/WebKitSystemInterfaceIOS.h") if isIOSWebKit(); > > This setup is sort of weird. We have iOS ifdefs in WebKitSystemInterface.h, > plus we also have a separate header. I don't know if it would be better to > make a separate Mac file, or to put everything into WebKitSystemInterface.h, > but what we have now is asymmetric. I agree. We can clean this up in another iteration or, even better, work to remove WKSI. Committed r177603: <http://trac.webkit.org/changeset/177603> |