[webkit-reviews] review granted: [Bug 59048] Need to track whether overlay scrollbar is currently visible and in lower-righthand corner : [Attachment 90589] Patch that sends a rect through the client

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 21 14:44:52 PDT 2011


mitz at webkit.org has granted Beth Dakin <bdakin at apple.com>'s request for review:
Bug 59048: Need to track whether overlay scrollbar is currently visible and in
lower-righthand corner
https://bugs.webkit.org/show_bug.cgi?id=59048

Attachment 90589: Patch that sends a rect through the client
https://bugs.webkit.org/attachment.cgi?id=90589&action=review

------- Additional Comments from mitz at webkit.org
View in context: https://bugs.webkit.org/attachment.cgi?id=90589&action=review

> Source/WebCore/platform/mac/ScrollAnimatorMac.mm:426
> +	       _animator->setVisibleScrollerThumbRect(WebCore::IntRect());

I don’t think the WebCore:: qualifier is needed here!

> Source/WebCore/platform/mac/ScrollAnimatorMac.mm:506
> +    , m_visibleScrollerThumbRect(IntRect())

No need to initialize an IntRect member variable

> Source/WebKit2/UIProcess/WebPageProxy.cpp:156
> +    , m_visibleScrollerThumbRect(IntRect())

Unnecessary initialization

> Source/WebKit2/UIProcess/WebPageProxy.h:494
> +    const WebCore::IntRect& visibleScrollerThumbRect() const { return
m_visibleScrollerThumbRect; }

I think typically functions that return an IntRect just return it by value, not
a by (const) reference.

> Source/WebKit2/UIProcess/API/mac/WKView.mm:1599
> +    const IntRect& visibleThumbRect =
_data->_page->visibleScrollerThumbRect();
> +    NSRect visibleThumbNSRect = NSMakeRect(visibleThumbRect.x(),
visibleThumbRect.y(), visibleThumbRect.width(), visibleThumbRect.height());

IntRect has an operator NSRect() so I think the conversion can just happen
automatically or with a cast.


More information about the webkit-reviews mailing list