WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
139831
[iOS] Add WebKitSystemInterface for iOS 8.1
https://bugs.webkit.org/show_bug.cgi?id=139831
Summary
[iOS] Add WebKitSystemInterface for iOS 8.1
Daniel Bates
Reported
2014-12-19 12:59:15 PST
Towards building iOS WebKit with the public iOS SDK we need to add WebKitSystemInterface for iOS.
Attachments
Patch
(164.44 KB, patch)
2014-12-19 13:00 PST
,
Daniel Bates
ap
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2014-12-19 13:00:16 PST
Created
attachment 243570
[details]
Patch
Andy Estes
Comment 2
2014-12-19 13:04:15 PST
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.
Daniel Bates
Comment 3
2014-12-19 15:10:50 PST
(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.
Alexey Proskuryakov
Comment 4
2014-12-19 15:15:17 PST
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.
Daniel Bates
Comment 5
2014-12-19 15:25:20 PST
(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.
Daniel Bates
Comment 6
2014-12-19 16:17:44 PST
Committed
r177603
: <
http://trac.webkit.org/changeset/177603
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug