RESOLVED FIXED 47278
Add WebThemeEngine api for chromium/linux
https://bugs.webkit.org/show_bug.cgi?id=47278
Summary Add WebThemeEngine api for chromium/linux
Dave Moore
Reported 2010-10-06 10:36:26 PDT
Currently all scrollbar drawing for chromium / linux is done within platform/chromium/ScrollbarThemeChromiumLinux.cpp. This prevents us from customizing it from the chromium project. We need to add new api to support this.
Attachments
Patch (8.53 KB, patch)
2010-10-06 10:48 PDT, Dave Moore
no flags
Patch (8.55 KB, patch)
2010-10-06 11:06 PDT, Dave Moore
no flags
Patch (9.51 KB, patch)
2010-10-06 11:20 PDT, Dave Moore
no flags
Patch (10.25 KB, patch)
2010-10-06 11:36 PDT, Dave Moore
no flags
Patch (10.26 KB, patch)
2010-10-06 11:44 PDT, Dave Moore
no flags
Patch (10.26 KB, patch)
2010-10-06 17:02 PDT, Dave Moore
no flags
Patch (10.25 KB, patch)
2010-10-07 07:46 PDT, Dave Moore
no flags
Dave Moore
Comment 1 2010-10-06 10:48:43 PDT
WebKit Review Bot
Comment 2 2010-10-06 10:55:53 PDT
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.
Dave Moore
Comment 3 2010-10-06 11:06:59 PDT
Tony Chang
Comment 4 2010-10-06 11:12:50 PDT
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.
Dave Moore
Comment 5 2010-10-06 11:20:42 PDT
Dave Moore
Comment 6 2010-10-06 11:21:53 PDT
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.
Tony Chang
Comment 7 2010-10-06 11:26:55 PDT
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).
Dave Moore
Comment 8 2010-10-06 11:36:29 PDT
WebKit Review Bot
Comment 9 2010-10-06 11:42:37 PDT
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.
Dave Moore
Comment 10 2010-10-06 11:44:34 PDT
Tony Chang
Comment 11 2010-10-06 12:08:50 PDT
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.)
Darin Fisher (:fishd, Google)
Comment 12 2010-10-06 16:17:23 PDT
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
Dave Moore
Comment 13 2010-10-06 17:02:21 PDT
Kent Tamura
Comment 14 2010-10-06 17:09:02 PDT
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.
Dave Moore
Comment 15 2010-10-07 07:46:54 PDT
WebKit Commit Bot
Comment 16 2010-10-07 08:08:17 PDT
Comment on attachment 70090 [details] Patch Clearing flags on attachment: 70090 Committed r69311: <http://trac.webkit.org/changeset/69311>
WebKit Commit Bot
Comment 17 2010-10-07 08:08:24 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.