[Webkit-unassigned] [Bug 78028] Not For Review: Decouple scrollbar painting from Scrollbar object

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78028





--- Comment #13 from Adrienne Walker <enne at google.com>  2012-02-15 16:00:57 PST ---
(From update of attachment 127239)
View in context: https://bugs.webkit.org/attachment.cgi?id=127239&action=review

> Source/WebCore/platform/Scrollbar.cpp:556
> +PassOwnPtr<Vector<IntRect> > Scrollbar::getTickmarks() const

Can you keep the same signature as ScrollableArea::getTickmarks? There's no need to wrap a Vector in an OwnPtr here.

> Source/WebCore/platform/Widget.h:131
> +    virtual int x() const { return frameRect().x(); }
> +    virtual int y() const { return frameRect().y(); }
> +    virtual int width() const { return frameRect().width(); }
> +    virtual int height() const { return frameRect().height(); }
> +    virtual IntSize size() const { return frameRect().size(); }
> +    virtual IntPoint location() const { return frameRect().location(); }

I think you should revert these and other virtual modifications you made to Widget.h.  They're not necessary if you're going to explicitly thunk back to them in Scrollbar.

> Source/WebCore/rendering/RenderScrollbar.h:42
> +class RenderScrollbar : public Scrollbar,
> +                        public RenderScrollbarThemeClient {

This is an awkward inheritance hierarchy that forces you to redeclare virtuals from Scrollbar because it includes ScrollbarThemeClient from two different ancestors.

Honestly, I would prefer to punt on RenderScrollbar.  ScrollbarThemeClient is a small bag of data and ScrollbarTheme is a set of functions to to process that.  To serialize Scrollbar into a new ScrollbarData class (that implements ScrollbarThemeClient) is trivial, as you had done in your previous patch.  Doing something like that for RenderScrollbar, which is made up of RenderScrollbarPart (which is-a RenderBlock) is much less so.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.