Bug 73729 - [chromium] Remove forwarding headers for WebLayer APIs
Summary: [chromium] Remove forwarding headers for WebLayer APIs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-02 18:25 PST by James Robinson
Modified: 2011-12-09 17:31 PST (History)
4 users (show)

See Also:


Attachments
Patch (26.31 KB, patch)
2011-12-02 18:26 PST, James Robinson
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2011-12-02 18:25:27 PST
[chromium] Remove forwarding headers for WebLayer APIs
Comment 1 James Robinson 2011-12-02 18:26:04 PST
Created attachment 117729 [details]
Patch
Comment 2 James Robinson 2011-12-02 18:27:58 PST
This can't land until http://codereview.chromium.org/8788008/ is landed, or it'll break the chromium compile. We do not have to wait for anything to roll into Source/WebKit/chromium/DEPS, just make sure it's committed.
Comment 3 WebKit Review Bot 2011-12-02 18:29:44 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 4 Antoine Labour 2011-12-02 18:31:30 PST
LGTM FWIW
Comment 5 Adam Barth 2011-12-02 23:32:56 PST
Comment on attachment 117729 [details]
Patch

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

> Source/WebKit/chromium/src/WebLayerTreeView.cpp:32
> +#include "platform/WebLayer.h"

The main question is whether we want to add platform to the include path.  I think having the directory name here is helpful, but that's not precisely webkit style.
Comment 6 Darin Fisher (:fishd, Google) 2011-12-04 23:17:38 PST
(In reply to comment #5)
> (From update of attachment 117729 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117729&action=review
> 
> > Source/WebKit/chromium/src/WebLayerTreeView.cpp:32
> > +#include "platform/WebLayer.h"
> 
> The main question is whether we want to add platform to the include path.  I think having the directory name here is helpful, but that's not precisely webkit style.

I really like the directory name too.  I like it because it reminds you of the module you are depending on.  WebKit uses this approach for including files from WTF, so it is not entirely foreign to WebKit.
Comment 7 Darin Fisher (:fishd, Google) 2011-12-04 23:19:27 PST
(In reply to comment #6)
> I really like the directory name too.  I like it because it reminds you of the module you are depending on.  WebKit uses this approach for including files from WTF, so it is not entirely foreign to WebKit.

However, one thing bugs me a bit.  If we expect to move WebKit/chromium/public/platform/ to Platform/chromium/public/ someday, then the "platform/foo" includes would all need to be fixed up.  Don't care?
Comment 8 James Robinson 2011-12-05 00:08:03 PST
I kind of like that, actually - it'll be easier to find all the #includes across the codebase and make sure the move will behave as expected.
Comment 9 James Robinson 2011-12-05 11:12:44 PST
Committed r102019: <http://trac.webkit.org/changeset/102019>
Comment 10 James Robinson 2011-12-09 17:31:30 PST
Committed r102500: <http://trac.webkit.org/changeset/102500>