[webkit-reviews] review granted: [Bug 90528] [chromium] Remove dependency on ScrollbarTheme from the compositor : [Attachment 154687] Rebased, static casts

[bugzilla-daemon at webkit.org]

James Robinson (vacation until 7-25-2012) <jamesr at chromium.org> has granted
Adrienne Walker <enne at google.com>'s request for review:
Bug 90528: [chromium] Remove dependency on ScrollbarTheme from the compositor
https://bugs.webkit.org/show_bug.cgi?id=90528

Attachment 154687: Rebased, static casts
https://bugs.webkit.org/attachment.cgi?id=154687&action=review

------- Additional Comments from James Robinson (vacation until 7-25-2012)
<jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=154687&action=review


R=me (with many comments)

> Source/Platform/chromium/public/WebScrollbar.h:47
> +#if WEBKIT_IMPLEMENTATION
> +    WEBKIT_EXPORT static PassOwnPtr<WebScrollbar>
create(WebCore::Scrollbar*);
> +#endif

I think WEBKIT_IMPLEMENTATION blocks are more commonly at the end of the
public: section, with the rationale being that most people who read the
interface don't need to know or care about implementation details

> Source/Platform/chromium/public/WebScrollbarThemeGeometry.h:29
> +#include <public/WebRect.h>

just "WebRect.h" - it's in the same directory so a non-qualified #include will
work and I'm not sure that Source/Platform/chromium/ is on the include path of
every file that might end up transitively including this header.

> Source/Platform/chromium/public/WebScrollbarThemeGeometry.h:72
> +    WebScrollbarThemeGeometry(WebCore::ScrollbarThemeComposite*);

do we want explicit? (I'm not sure, it might be helpful to have implicit
conversions somewhere)

> Source/Platform/chromium/public/WebScrollbarThemePainter.h:29
> +#include <public/WebCanvas.h>

"WebCanvas.h"

> Source/WebKit/chromium/src/WebScrollbarImpl.h:40
> +    // Implement WebKit::WebScrollbar methods

OVERRIDEs, in that case?

> Source/WebKit/chromium/src/WebScrollbarThemeClientImpl.h:47
> +    virtual int x() const;

OVERRIDEs, please