We have moved/implemented the drawing code into NativeThemeLinux and now need to use WebThemeEngine to hook it up with RenderTheme code.
Created attachment 79628 [details] Proposed code change and new baseline for layout tests.
Comment on attachment 79628 [details] Proposed code change and new baseline for layout tests. View in context: https://bugs.webkit.org/attachment.cgi?id=79628&action=review r- for style nits. Can you upload some of image diffs? I'm having a hard time seeing the difference. Maybe it's just a few rgb points off? > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:58 > + return ChromiumBridge::StateHover; 4 space indent here and above 2 conditions > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:196 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartCheckbox, > + getWebThemeState(this, o), > + rect, > + &extraParams); I think this should just be one long line (maybe one line of wrapping if it's really long). > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:217 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartRadio, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:238 > + extraParams.button.backgroundColor = SkColorSetRGB(0xdd, 0xdd, 0xdd); Should this color be a constant somewhere? > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:244 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartButton, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:294 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartMenuList, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:308 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartSliderTrack, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:323 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartSliderThumb, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:346 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartInnerSpinButton, less wrapping > Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:372 > + ChromiumBridge::paintThemePart( > + i.context, > + ChromiumBridge::PartProgressBar, less wrapping
Created attachment 79676 [details] Layout test results to show image diff This is the layout test results of broken image tests. It's pretty hard to tell the difference with naked eyes. :p
(In reply to comment #3) > Created an attachment (id=79676) [details] > Layout test results to show image diff > > This is the layout test results of broken image tests. It's pretty hard to tell the difference with naked eyes. :p I agree, these all look fine. Thanks for uploading your results!
Created attachment 79678 [details] Address tony's comments @ 2
Attachment 79678 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Source/WebCore/rendering/RenderThemeChromiumLinux.cpp:55: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 1 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Note that this change need to land after http://src.chromium.org/viewvc/chrome?view=rev&revision=71969 is rolled in.
Created attachment 79679 [details] Fix style @ 6
Comment on attachment 79679 [details] Fix style @ 6 static const unsigned kDefaultButtonBackgroundColor = 0xffdddddd; This should be defaultButtonBackgroundColor.
Created attachment 79692 [details] Address comments @ 9
Comment on attachment 79692 [details] Address comments @ 9 Please take another look. Thanks.
Created attachment 79748 [details] Sync and resolve. Synced with ToT and adapted to r76340 (ChromiumBridge -> PlatformBridge rename change).
Comment on attachment 79748 [details] Sync and resolve. Clearing flags on attachment: 79748 Committed r76379: <http://trac.webkit.org/changeset/76379>
All reviewed patches have been landed. Closing bug.