Bug 91674 – [chromium] Implement scrollbar theme for Android

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tien-Ren Chen]

[chromium] Implement scrollbar theme for Android

[Tien-Ren Chen]

Created attachment 153103 [details]
Patch

[Peter Beverloo]

Please be sure that your patches block bug 66687, that helps us to keep track. Thanks!

[Adam Barth]

Comment on attachment 153103 [details]
Patch

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

Some minor style nits below.

> Source/WebCore/platform/chromium/ScrollbarThemeChromiumAndroid.cpp:66
> +    float proportion = (float)scrollbar->currentPos() / scrollbar->totalSize();

WebKit preferes C++ style casts, so static_cast<float>(...)

> Source/WebCore/platform/chromium/ScrollbarThemeChromiumAndroid.cpp:103
> +namespace {

WebKit tends to use static functions rather than anonymous namespaces.

> Source/WebCore/platform/chromium/ScrollbarThemeChromiumAndroid.h:53
> +    static const int s_scrollbarWidth = 8;
> +    static const int s_scrollbarMargin = 5;

WebKit doesn't use the s_ prefix for static variables.  We just use bare names like scrollbarWidth (and we tend to put them in the implementation file rather than the header if they're not used outside the implementation).

[Adam Barth]

Created attachment 153885 [details]
Patch for landing

[Adam Barth]

@trchen: I fixed the nits for you.  Please let me know if I've goofed them up.  Thanks for the patch.

[WebKit Review Bot]

Comment on attachment 153885 [details]
Patch for landing

Clearing flags on attachment: 153885

Committed r123402: <http://trac.webkit.org/changeset/123402>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.