[Chromium] Add WebThemeEngineDRTMac so that Chromium DRT scrollbar rendering can match the Mac port's
Created attachment 77649 [details] Patch
Created attachment 77657 [details] Patch
This depends on the changes in http://codereview.chromium.org/6090002/. I'll need to wait for that to be checked in and then adjust the DEPS roll to at least that revision.
Attachment 77657 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7318277
Created attachment 77693 [details] Patch
(In reply to comment #4) > Attachment 77657 [details] did not build on chromium: > Build output: http://queues.webkit.org/results/7318277 The Chromium changes that this depends on landed as http://crrev.com/70319, so I updated the DEPS roll to that. The patch should build now.
Comment on attachment 77693 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=77693&action=review r+ though there are some style errors. > Tools/DumpRenderTree/chromium/TestShellMac.mm:38 > +// Theme engine This comment doesn't add any useful information. Please remove it. > Tools/DumpRenderTree/chromium/TestShellMac.mm:133 > + // Set theme engine. ditto. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.h:43 > +private: Please add a blank line before private:. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:67 > + if (alwaysActiveWindow == nil) { > + alwaysActiveWindow = [[self alloc] initWithActiveControls:YES]; > + } {} is not needed. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:75 > + if (alwaysInactiveWindow == nil) { > + alwaysInactiveWindow = [[self alloc] initWithActiveControls:NO]; > + } ditto. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:86 > +- (BOOL)_hasActiveControls { "{" should be moved to a new line. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:97 > + const WebThemeEngine::ScrollbarInfo& scrollbarInfo) { ditto. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:105 > +static ThemeTrackEnableState stateToHIEnableState(WebThemeEngine::State state) { ditto. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:113 > + switch (state) { > + case WebThemeEngine::StateDisabled: > + return kThemeTrackDisabled; > + case WebThemeEngine::StateInactive: > + return kThemeTrackInactive; > + default: > + return kThemeTrackActive; > + } indentation error. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:123 > + const WebThemeEngine::ScrollbarInfo& scrollbarInfo) { "{" should be moved to a new line. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:149 > + const WebThemeEngine::ScrollbarInfo& scrollbarInfo) { "{" should be moved to a new line. > Tools/DumpRenderTree/chromium/WebThemeEngineDRTMac.mm:163 > + double value = double(scrollbarInfo.currentValue)/double(scrollbarInfo.maxValue); > + [scroller setDoubleValue: value]; > + > + float knobProportion = float(scrollbarInfo.visibleSize)/float(scrollbarInfo.totalSize); > + [scroller setKnobProportion: knobProportion]; Need spaces around "/" operators.
Created attachment 77701 [details] Patch for landing
Thanks for the review, I've fixed all the style issues (I'm still not used to switching between Chromium and WebKit styles when making double-sided changes).
Comment on attachment 77701 [details] Patch for landing Clearing flags on attachment: 77701 Committed r74821: <http://trac.webkit.org/changeset/74821>
All reviewed patches have been landed. Closing bug.