Bug 81884

Summary: [GTK] Use the angle-bracket form to include wtf headers
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, gustavo, menard, mrobinson, pnormand, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 81844    
Attachments:
Description Flags
Patch eric: review+

Description Carlos Garcia Campos 2012-03-22 02:35:29 PDT
In preparation for the move of WTF to its new home.
Comment 1 Carlos Garcia Campos 2012-03-22 02:41:30 PDT
Created attachment 133210 [details]
Patch
Comment 2 WebKit Review Bot 2012-03-22 02:44:20 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Eric Seidel (no email) 2012-03-22 07:32:15 PDT
Comment on attachment 133210 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133210&action=review

Thanks!

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:-34
> -#include <JavaScriptCore/GRefPtr.h>

How did these used to work?  Do we need to delete some ForwardingHeaders as well?
Comment 4 Carlos Garcia Campos 2012-03-22 07:55:10 PDT
(In reply to comment #3)
> (From update of attachment 133210 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133210&action=review
> 
> Thanks!
> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:-34
> > -#include <JavaScriptCore/GRefPtr.h>
> 
> How did these used to work?  Do we need to delete some ForwardingHeaders as well?

good point, I don't know, Martin?
Comment 5 Carlos Garcia Campos 2012-03-22 07:56:54 PDT
Committed r111696: <http://trac.webkit.org/changeset/111696>
Comment 6 Martin Robinson 2012-03-22 07:59:29 PDT
Comment on attachment 133210 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=133210&action=review

>>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:-34
>>> -#include <JavaScriptCore/GRefPtr.h>
>> 
>> How did these used to work?  Do we need to delete some ForwardingHeaders as well?
> 
> good point, I don't know, Martin?

WebKit2 uses framework style includes (where all the headers are in a single directory), so this should probably become #include <wtf/GRefPtr.h>. The directory of headers is generated by the generate-forwarding-headers script during build. I can fix up all of these includes in a followup commit.
Comment 7 Carlos Garcia Campos 2012-03-22 08:04:40 PDT
(In reply to comment #6)
> (From update of attachment 133210 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=133210&action=review
> 
> >>> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:-34
> >>> -#include <JavaScriptCore/GRefPtr.h>
> >> 
> >> How did these used to work?  Do we need to delete some ForwardingHeaders as well?
> > 
> > good point, I don't know, Martin?
> 
> WebKit2 uses framework style includes (where all the headers are in a single directory), so this should probably become #include <wtf/GRefPtr.h>. The directory of headers is generated by the generate-forwarding-headers script during build. I can fix up all of these includes in a followup commit.

is it worth it? what's the problem of using <wtf/text/CString.h>, it looks more clear to me.
Comment 8 Martin Robinson 2012-03-22 08:08:39 PDT
(In reply to comment #7)

> is it worth it? what's the problem of using <wtf/text/CString.h>, it looks more clear to me.

I think it's good to be consistent within WebKit2. We don't really have the option of using complete paths in platform-independent code (we suggested that early on, but Apple developers pushed back) so this is the only thing we can do to ensure consistency.
Comment 9 Martin Robinson 2012-03-22 08:09:56 PDT
(In reply to comment #8)
> (In reply to comment #7)
> 
> > is it worth it? what's the problem of using <wtf/text/CString.h>, it looks more clear to me.
> 
> I think it's good to be consistent within WebKit2. We don't really have the option of using complete paths in platform-independent code (we suggested that early on, but Apple developers pushed back) so this is the only thing we can do to ensure consistency.

I see now that the new WTF includes contain full paths everywhere. You're right that we shouldn't change this now, unless it's a  problem for the people who maintain the platform-independent bits.
Comment 10 Eric Seidel (no email) 2012-03-22 11:20:19 PDT
Some of this comes from how the Mac build works.

On Mac, JavaScirptCore and WebCore are Frameworks, with flat header includes.

WTF however is installed in /usr/local/include on Mac (which is present in Apple internal builds, but stripped for consumer builds).

the AppleWin port follows similarly, faking "frameworks" with some similarly-flat include directories under the build directories.

You're correct that WTF is the only part of WebKit for which we use full include paths.  Now you know why. :)