WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
137371
[iOS] Make WebKit2 build with public iOS SDK and more build fixes for DRT
https://bugs.webkit.org/show_bug.cgi?id=137371
Summary
[iOS] Make WebKit2 build with public iOS SDK and more build fixes for DRT
Daniel Bates
Reported
2014-10-02 16:46:33 PDT
We should make WebKit2 build with the public iOS SDK.
Attachments
Work-in-progress patch
(227.45 KB, patch)
2014-10-02 16:48 PDT
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(119.71 KB, patch)
2014-12-22 17:09 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch
(115.64 KB, patch)
2014-12-23 10:44 PST
,
Daniel Bates
no flags
Details
Formatted Diff
Diff
Patch 1 of 4: Add new files
(49.85 KB, patch)
2015-01-07 13:56 PST
,
Daniel Bates
ddkilzer
: review+
Details
Formatted Diff
Diff
Patch 2 of 4: WebCore and WebKit2 Xcode changes
(20.04 KB, patch)
2015-01-07 13:57 PST
,
Daniel Bates
ddkilzer
: review+
Details
Formatted Diff
Diff
Patch 3 of 4: DumpRenderTree changes
(6.28 KB, patch)
2015-01-07 13:58 PST
,
Daniel Bates
ddkilzer
: review+
Details
Formatted Diff
Diff
Patch 4 of 4: Substitute SPI wrapper headers for actual headers; add WTF EXTERN_C_{BEGIN, END}
(42.31 KB, patch)
2015-01-07 13:59 PST
,
Daniel Bates
ddkilzer
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Daniel Bates
Comment 1
2014-10-02 16:48:50 PDT
Created
attachment 239156
[details]
Work-in-progress patch
Daniel Bates
Comment 2
2014-12-22 17:09:34 PST
Created
attachment 243652
[details]
Patch
WebKit Commit Bot
Comment 3
2014-12-22 17:11:48 PST
Attachment 243652
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:37: name_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 64 files If any of these errors are false positives, please file a bug against check-webkit-style.
Daniel Bates
Comment 4
2014-12-23 10:44:49 PST
Created
attachment 243679
[details]
Patch
WebKit Commit Bot
Comment 5
2014-12-23 10:47:41 PST
Attachment 243679
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:37: name_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:43: bootstrap_look_up is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:44: bootstrap_register2 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 64 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 6
2014-12-27 14:12:24 PST
Comment on
attachment 243679
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=243679&action=review
> Source/WebCore/platform/spi/cocoa/ServersSPI.h:33 > +#include <servers/bootstrap.h>
Is this really SPI everywhere? I think at least on OS X, this file is API.
> Source/WebCore/platform/spi/cocoa/ServersSPI.h:44 > +kern_return_t bootstrap_register2(mach_port_t, name_t, mach_port_t, uint64_t);
I don't think you are including a header for this. It is defined in bootstrap_priv.h,
> Source/WebCore/platform/spi/ios/UIKitSPI.h:28 > +// FIXME: Move this header to the WebKit2 framework once we remove DumpRenderTree's > +// dependence on it. It is a layering violation to include this header or any UIKit > +// headers in a WebCore or WebKitLegacy file as UIKit depends on WebKitLegacy.
Why not just make DumpRenderTree get it from WebKit2?
Daniel Bates
Comment 7
2015-01-07 12:29:40 PST
(In reply to
comment #6
)
> Comment on
attachment 243679
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=243679&action=review
> > > Source/WebCore/platform/spi/cocoa/ServersSPI.h:33 > > +#include <servers/bootstrap.h> > > Is this really SPI everywhere? I think at least on OS X, this file is API. >
You're right! Will fix as this is API on OS X.
> > Source/WebCore/platform/spi/cocoa/ServersSPI.h:44 > > +kern_return_t bootstrap_register2(mach_port_t, name_t, mach_port_t, uint64_t); > > I don't think you are including a header for this. It is defined in > bootstrap_priv.h, >
Will include header bootstrap_priv.h when building with the Apple Internal SDK.
> > Source/WebCore/platform/spi/ios/UIKitSPI.h:28 > > +// FIXME: Move this header to the WebKit2 framework once we remove DumpRenderTree's > > +// dependence on it. It is a layering violation to include this header or any UIKit > > +// headers in a WebCore or WebKitLegacy file as UIKit depends on WebKitLegacy. > > Why not just make DumpRenderTree get it from WebKit2?
I can do this. When I wrote the patch I thought it would be considered bad form to have a WebKit1-based application, DumpRenderTree, depend on a WebKit2 header and I did not want to add to the bad practices used by DumpRenderTree (including referencing headers from WebCore).
Daniel Bates
Comment 8
2015-01-07 13:56:40 PST
Created
attachment 244190
[details]
Patch 1 of 4: Add new files Adds SPI wrapper headers and consolidates the content of header _UIHighlightViewSPI.h into the newly added header UIKitSPI.h.
Daniel Bates
Comment 9
2015-01-07 13:57:39 PST
Created
attachment 244192
[details]
Patch 2 of 4: WebCore and WebKit2 Xcode changes
WebKit Commit Bot
Comment 10
2015-01-07 13:58:31 PST
Attachment 244190
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:37: name_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:49: bootstrap_look_up is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/platform/spi/cocoa/ServersSPI.h:50: bootstrap_register2 is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Daniel Bates
Comment 11
2015-01-07 13:58:49 PST
Created
attachment 244193
[details]
Patch 3 of 4: DumpRenderTree changes
Daniel Bates
Comment 12
2015-01-07 13:59:22 PST
Created
attachment 244194
[details]
Patch 4 of 4: Substitute SPI wrapper headers for actual headers; add WTF EXTERN_C_{BEGIN, END}
WebKit Commit Bot
Comment 13
2015-01-07 14:01:09 PST
Attachment 244194
[details]
did not pass style-queue: ERROR: Source/WebKit2/UIProcess/ios/SmartMagnificationController.mm:43: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Kilzer (:ddkilzer)
Comment 14
2015-01-07 15:38:53 PST
Comment on
attachment 244190
[details]
Patch 1 of 4: Add new files rs=me
David Kilzer (:ddkilzer)
Comment 15
2015-01-07 15:40:53 PST
Comment on
attachment 244192
[details]
Patch 2 of 4: WebCore and WebKit2 Xcode changes r=me
David Kilzer (:ddkilzer)
Comment 16
2015-01-07 15:47:07 PST
Comment on
attachment 244193
[details]
Patch 3 of 4: DumpRenderTree changes View in context:
https://bugs.webkit.org/attachment.cgi?id=244193&action=review
r=me
> Tools/ChangeLog:3 > + [iOS] Make WebKit2 build with public iOS SDK
I know this is the title of the bug, but the patch actually makes DumpRenderTree build. Maybe better to change "WebKit2" to "DumpRenderTree"?
David Kilzer (:ddkilzer)
Comment 17
2015-01-07 15:55:13 PST
Comment on
attachment 244194
[details]
Patch 4 of 4: Substitute SPI wrapper headers for actual headers; add WTF EXTERN_C_{BEGIN, END} View in context:
https://bugs.webkit.org/attachment.cgi?id=244194&action=review
r=me
> Source/WTF/wtf/Compiler.h:179 > #ifdef __cplusplus > +#define EXTERN_C_BEGIN extern "C" { > +#define EXTERN_C_END } > +#else > +#define EXTERN_C_BEGIN > +#define EXTERN_C_END > +#endif
Not to be pedantic, but do these need to be named WTF_EXTERN_C_BEGIN and WTF_EXTERN_C_END so we don't conflict with someone else's headers?
> Source/WebKit2/ChangeLog:10 > + * Configurations/BaseTarget.xcconfig: Append directory $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/PrivateFrameworks to > + the list of directories to search for frameworks so that we the linker can find and link against iOS private > + frameworks (e.g. AssertionServices).
This was moved to a different patch, I think.
Daniel Bates
Comment 18
2015-01-07 16:02:22 PST
(In reply to
comment #16
)
> Comment on
attachment 244193
[details]
> Patch 3 of 4: DumpRenderTree changes > > Tools/ChangeLog:3 > > + [iOS] Make WebKit2 build with public iOS SDK > > I know this is the title of the bug, but the patch actually makes > DumpRenderTree build. Maybe better to change "WebKit2" to "DumpRenderTree"?
No, more build fixes are needed to make DumpRenderTree build with the public SDK.
Daniel Bates
Comment 19
2015-01-07 16:09:01 PST
(In reply to
comment #17
)
> > Source/WTF/wtf/Compiler.h:179 > > #ifdef __cplusplus > > +#define EXTERN_C_BEGIN extern "C" { > > +#define EXTERN_C_END } > > +#else > > +#define EXTERN_C_BEGIN > > +#define EXTERN_C_END > > +#endif > > Not to be pedantic, but do these need to be named WTF_EXTERN_C_BEGIN and > WTF_EXTERN_C_END so we don't conflict with someone else's headers? >
Will rename EXTERN_C_BEGIN and EXTERN_C_END to WTF_EXTERN_C_BEGIN and WTF_EXTERN_C_END, respectively.
> > Source/WebKit2/ChangeLog:10 > > + * Configurations/BaseTarget.xcconfig: Append directory $(SDKROOT)$(SYSTEM_LIBRARY_DIR)/PrivateFrameworks to > > + the list of directories to search for frameworks so that we the linker can find and link against iOS private > > + frameworks (e.g. AssertionServices). > > This was moved to a different patch, I think.
This change is in "Patch 2 of 4: WebCore and WebKit2 Xcode changes" (
attachment 244192
[details]
).
Daniel Bates
Comment 20
2015-01-07 16:36:01 PST
Committed
r178068
: <
http://trac.webkit.org/changeset/178068
>
WebKit Commit Bot
Comment 21
2015-01-07 17:59:28 PST
Re-opened since this is blocked by
bug 140235
Tim Horton
Comment 22
2015-01-07 18:49:56 PST
Comment on
attachment 244194
[details]
Patch 4 of 4: Substitute SPI wrapper headers for actual headers; add WTF EXTERN_C_{BEGIN, END} View in context:
https://bugs.webkit.org/attachment.cgi?id=244194&action=review
> Source/WTF/wtf/Compiler.h:181 > +// FIXME: Remove this once we have transitioned to EXTERN_C_BEGIN/EXTERN_C_END.
This file *has* to use /* */ style comments because it gets included in the sandbox profile and the sandbox profile compiler doesn't understand // comments.
Daniel Bates
Comment 23
2015-01-07 20:17:04 PST
Committed
r178080
: <
http://trac.webkit.org/changeset/178080
>
Daniel Bates
Comment 24
2015-01-07 22:07:59 PST
(In reply to
comment #22
)
> Comment on
attachment 244194
[details]
> Patch 4 of 4: Substitute SPI wrapper headers for actual headers; add WTF > EXTERN_C_{BEGIN, END} > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=244194&action=review
> > > Source/WTF/wtf/Compiler.h:181 > > +// FIXME: Remove this once we have transitioned to EXTERN_C_BEGIN/EXTERN_C_END. > > This file *has* to use /* */ style comments because it gets included in the > sandbox profile and the sandbox profile compiler doesn't understand // > comments.
Fixed in <
http://trac.webkit.org/changeset/178104
>.
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