Bug 94392 - [chromium] Put webkit compositor binding files in separate conditionally included gyp section
Summary: [chromium] Put webkit compositor binding files in separate conditionally incl...
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: 2012-08-17 16:16 PDT by James Robinson
Modified: 2012-08-19 21:33 PDT (History)
6 users (show)

See Also:

Attachments
Patch (7.35 KB, patch)
2012-08-17 16:17 PDT, James Robinson 		no flags Details | Formatted Diff | Diff
Patch (7.87 KB, patch)
2012-08-17 17:23 PDT, James Robinson 		no flags Details | Formatted Diff | Diff
Patch (7.88 KB, patch)
2012-08-17 18:06 PDT, James Robinson 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2012-08-17 16:16:29 PDT 
[chromium] Put webkit compositor binding files in separate conditionally included gyp section
Comment 1 James Robinson 2012-08-17 16:17:14 PDT 
Created attachment 159224 [details]
Patch
Comment 2 James Robinson 2012-08-17 16:18:04 PDT 
We unconditionally pull 'webkit/' from the chromium repo, so a separate DEPS entry isn't needed for webkit/compositor/
Comment 3 WebKit Review Bot 2012-08-17 16:34:42 PDT 
Comment on attachment 159224 [details]
Patch

Attachment 159224 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13514999
Comment 4 Peter Beverloo (cr-android ews) 2012-08-17 16:39:16 PDT 
Comment on attachment 159224 [details]
Patch

Attachment 159224 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13533071
Comment 5 James Robinson 2012-08-17 17:08:35 PDT 
Hmm, I thought that gyp var would have been picked up from WebCore.gypi.  Interesting....
Comment 6 James Robinson 2012-08-17 17:23:32 PDT 
Created attachment 159239 [details]
Patch
Comment 7 James Robinson 2012-08-17 17:25:02 PDT 
Duh, I default-initialized the variable in WebCore.gyp.  This inits it as well in WebKit.gyp - I'm not sure if that is the more idomatic thing or if I should set it in WebCore.gypi.  Advantage of the latter is I'll only have to flip the default in one place when it comes to that, but the disadvantage is it'll "leak" to a bunch more places.  What's the standard practice here?
Comment 8 Dirk Pranke 2012-08-17 17:34:59 PDT 
Comment on attachment 159239 [details]
Patch

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

dunno what the "standard practice" is ...

> Source/WebKit/chromium/WebKit.gyp:841
> +                        'webkit_compositor_bindings_files',

think this needs to be <@(webkit_compositor_bindings_files)
Comment 9 James Robinson 2012-08-17 18:06:51 PDT 
Created attachment 159249 [details]
Patch
Comment 10 WebKit Review Bot 2012-08-19 21:32:56 PDT 
Comment on attachment 159249 [details]
Patch

Clearing flags on attachment: 159249

Committed r125994: <http://trac.webkit.org/changeset/125994>
Comment 11 WebKit Review Bot 2012-08-19 21:33:01 PDT 
All reviewed patches have been landed.  Closing bug.