Bug 35841 – WebSystemInterface.m should not explicitly include <wtf/Platform.h>

• Status: NEW
• Component: WebKit Misc.

[Mark Rowe (bdash)]

The Mac build of the WebKit framework specifies a prefix header that includes <wtf/Platform.h> so there’s no reason for individual files within the project to do so.  This was added in r52356 in a change labeled as a build fix.  From what I can gather from the commit message and related bug (bug 32753) this appears to have been added out of some confusion between the concept of a prefix header and a precompiled header.  The ChangeLog talks about precompiled header files and distcc.  There’s no requirement that WebKit’s prefix header be precompiled and therefore, as far as I can tell, no reason why this file is being explicitly included rather than using the prefix header like every other compilation unit in the Mac WebKit build.

[Peter Kasting]

Thanks for filing this.  Given the IRC discussion last time (whose confusion I think you accurately characterize; everybody involved thought PCH rather than prefix header), it would be nice to publicly state whatever the policy is going forward, once people agree.  Hopefully that'll reduce future confusion.

FWIW, I think a world where files are explicit about all headers they need is better than a world in which you need a prefix header to make things compile.  OTOH, I don't have much to do with the Mac builds of Safari _or_ Chromium, so my opinion doesn't really matter.  I was only involved last time insofar as trying to find knowledgeable people on both sides to decide what the fix should be.

(Note: Most of r55633 seems OK either way, since it notes that most of the modified headers pull in Platform.h via config.h, and that's obviously redundant and fine to fix regardless of the resolution on this bug.)

[Mark Rowe (bdash)]

(In reply to comment #1)
> Thanks for filing this.  Given the IRC discussion last time (whose confusion I
> think you accurately characterize; everybody involved thought PCH rather than
> prefix header), it would be nice to publicly state whatever the policy is going
> forward, once people agree.  Hopefully that'll reduce future confusion.

It seems to me that the policy for WebKit/mac is already perfectly clear: the compilers used by the Mac build all support prefix headers, so we always use a prefix header rather than explicitly including a header like “config.h”.  I see this as something people need to agree on.  It’s how things work.

> (Note: Most of r55633 seems OK either way, since it notes that most of the
> modified headers pull in Platform.h via config.h, and that's obviously
> redundant and fine to fix regardless of the resolution on this bug.)

I don’t know what you’re saying here.  All of r55633 was ok.  It removed includes that are unnecessary either because config.h included them, or because the code in question is always built with a prefix header that includes them.