Bug 17851 – Should be able to build WebCore without prefix header injection

• Component: WebCore Misc.
• Status: NEW

[Mark Mentovai]

It should be possible to build WebCore without prefix header injection.  WebCorePrefix.h even states:

> * The project should be able to build without this header, although we rarely test that.

Currently, on the Mac, the WebCore build depends on prefix header injection.  Source files that depend on declarations being provided by the prefix header should be modified to include the declarations they depend on themselves.  This doesn't need to be the default build environment, but it should be available as an option.

To provide ongoing testing to ensure that the source is buildable without prefix header injection, it might be a good idea to provide a BuildBot that tests with an empty prefix header, or with GCC_PREFIX_HEADER set to empty.

[Mark Mentovai]

Created attachment 19765 [details]
A list of what needs to be done and how to do it

These are the notes I took as I fixed this, in case anyone's curious about why things were done how they were.

[Mark Mentovai]

Created attachment 19766 [details]
Include headers and provide forward declarations as needed

Attachment 19765 [details] contains a description of these changes.

If bug 17833 lands before this, the SharedTimerMac.cpp diff in this patch should be applied to SharedTimerMac.mm instead.

[Darin Adler]

Comment on attachment 19766 [details]
Include headers and provide forward declarations as needed

This is not a good change. There's no practical way to keep all these things compiling without a prefix header, and no real benefit either.

I've been moving the code in the opposite direction, removing includes added for only theoretical reasons.

Is there any real world advantage to this?

[Mark Mentovai]

Comment on attachment 19766 [details]
Include headers and provide forward declarations as needed

Really?  There's no reason not to support this.  Projects should be able to work with WebKit without depending on the in-WebKit build machinery.  It's a pretty big imposition to ask that this be maintained as a local patch in such cases.

[Mark Rowe (bdash)]

In what manner does the presence of a prefix header prevent any other project from working with WebKit?

[Mark Mentovai]

It's not a matter of other projects working with WebKit, it's a matter of integrating the WebKit source into other build systems.

[Mark Mentovai]

I'm going to make one last plea to fix this bug in the way proposed in attachment 19766 [details].  Specifically, prefix header injection interferes with the way that our build handles dependency scanning.  It doesn't seem that any platform other than the Mac is being required to use prefix header injection; I don't understand the disparity.

[David Kilzer (:ddkilzer)]

(In reply to comment #3)
> (From update of attachment 19766 [details])
> This is not a good change. There's no practical way to keep all these things
> compiling without a prefix header, and no real benefit either.

You could create a buildbot that simply compiles the project without the prefix header (or without the statement that does #import <Cocoa/Cocoa.h>).

> I've been moving the code in the opposite direction, removing includes added
> for only theoretical reasons.
> 
> Is there any real world advantage to this?

If this isn't the direction the project is moving in, then this comment in WebCorePrefix.h should be revised or removed:  <http://trac.webkit.org/browser/trunk/WebCore/WebCorePrefix.h#L21>

(In reply to comment #7)
> I'm going to make one last plea to fix this bug in the way proposed in
> attachment 19766 [details].  Specifically, prefix header injection interferes with the
> way that our build handles dependency scanning.  It doesn't seem that any
> platform other than the Mac is being required to use prefix header injection; I
> don't understand the disparity.

Apparently the Qt and Wx ports are using a prefix header based on code in WebCorePrefix.h.  Is this still an issue for Chromium builds?

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> If this isn't the direction the project is moving in, then this comment in
> WebCorePrefix.h should be revised or removed: 
> <http://trac.webkit.org/browser/trunk/WebCore/WebCorePrefix.h#L21>

That comment was added in r12121:  <http://trac.webkit.org/changeset/12121>

[Mark Mentovai]

I don't strictly require this for Chromium any longer.  The prefix header does not result in a performance advantage for us because we can't use precompiled headers in our build farms.  Our Mac WebKit build would still be slightly faster without prefix header injection.