Summary: | Add WebThemeEngine api for chromium/linux | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Moore <davemoore> | ||||||||||||||||
Component: | WebKit API | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | commit-queue, fishd, tony, webkit.review.bot | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Other | ||||||||||||||||||
OS: | Linux | ||||||||||||||||||
Attachments: |
|
Description
Dave Moore
2010-10-06 10:36:26 PDT
Created attachment 69958 [details]
Patch
Attachment 69958 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ChangeLog:7: Line contains tab character. [whitespace/tab] [5]
ChangeLog:8: Line contains tab character. [whitespace/tab] [5]
ChangeLog:9: Line contains tab character. [whitespace/tab] [5]
Total errors found: 3 in 4 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 69960 [details]
Patch
Comment on attachment 69960 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69960&action=review Can you also update WebKit.gyp to include the new files (maybe remove the old file?) and update WebKit/chromium/src/ChromiumBridge.cpp to include win/WebThemeEngine.h? > ChangeLog:11 > + * ../../WebKit/chromium/public/WebThemeEngine.h: This path is incorrect. Maybe webkit-patch is confused about being an svn checkout in a chromium git checkout? This path should be relative to the ChangeLog file. Created attachment 69967 [details]
Patch
Comment on attachment 69967 [details]
Patch
I can't remove the old WebThemeEngine.h without breaking the chromium build. I'll do it once I've added support for the new api there, and changed its includes.
Comment on attachment 69967 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69967&action=review > I can't remove the old WebThemeEngine.h without breaking the chromium build. I'll do it once I've added support for the new api there, and changed its includes. Right, I just mean to remove it from the gyp file (I think that's harmless, it just changes what file the IDE tries to open). You can still change the include in WebKit/chromium/src/ChromiumBridge.cpp, right? r- for ChangeLog > ChangeLog:1 > +2010-10-06 Dave Moore <davemoore@chromium.org> Actually, this is the wrong ChangeLog file. You want to edit WebKit/ChangeLog not the root ChangeLog. I think if you're going to use webkit-patch, you need to have a full WebKit checkout. Alternately, you can probably use the svn based scripts from your WebKit/WebKit/ directory (i.e., manually run prepare-ChangeLog and svn-create-diff). Created attachment 69969 [details]
Patch
Attachment 69969 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
ChangeLog:12: Line contains tab character. [whitespace/tab] [5]
Total errors found: 1 in 6 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 69971 [details]
Patch
Comment on attachment 69971 [details]
Patch
LGTM. Darin, can you do a quick review too? (Not setting review flag so Darin can review and so cr-linux can run.)
Comment on attachment 69971 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69971&action=review > WebKit/chromium/public/WebThemeEngine.h:42 > +// This file has been moved to the win subdirectory as it's entirely windows nit: please prefix this with "FIXME:" for easy grepping later > WebKit/chromium/public/linux/WebThemeEngine.h:67 > + // The bounds of the entire track, as opposed to the part being painted. can you use WebRect here? WebRect trackBounds Created attachment 70013 [details]
Patch
Comment on attachment 70013 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70013&action=review > ChangeLog:1 > +2010-10-06 Dave Moore <davemoore@chromium.org> The patch still update a wrong ChangeLog. It should be WebKit/chromium/ChangeLog, not the root ChangeLog. Created attachment 70090 [details]
Patch
Comment on attachment 70090 [details] Patch Clearing flags on attachment: 70090 Committed r69311: <http://trac.webkit.org/changeset/69311> All reviewed patches have been landed. Closing bug. |