[webkit-reviews] review granted: [Bug 91674] [chromium] Implement scrollbar theme for Android : [Attachment 153103] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Tien-Ren Chen
<trchen at chromium.org>'s request for review:
Bug 91674: [chromium] Implement scrollbar theme for Android
https://bugs.webkit.org/show_bug.cgi?id=91674

Attachment 153103: Patch
https://bugs.webkit.org/attachment.cgi?id=153103&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
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).