Bug 81884

Created attachment 133210 [details] Patch

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 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?

(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?

Committed r111696: <http://trac.webkit.org/changeset/111696>

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.

(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.

(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.

(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.

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. :)