[webkit-reviews] review granted: [Bug 93306] Convert ScrollableArea ASSERT_NOT_REACHED virtuals into pure virtuals : [Attachment 157350] Implement most funcs in ScrollableArea

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 14 12:48:09 PDT 2012


Darin Adler <darin at apple.com> has granted Adrienne Walker <enne at google.com>'s
request for review:
Bug 93306: Convert ScrollableArea ASSERT_NOT_REACHED virtuals into pure
virtuals
https://bugs.webkit.org/show_bug.cgi?id=93306

Attachment 157350: Implement most funcs in ScrollableArea
https://bugs.webkit.org/attachment.cgi?id=157350&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=157350&action=review


Looks good. Please don’t land without fixing Mac and Windows build. Please
consider addressing my comments below as well.

> Source/WebCore/platform/ScrollView.h:173
> +    virtual IntSize scrollOffset() const { return
visibleContentRect().location() - IntPoint(); } // Gets the scrolled position
as an IntSize. Convenient for adding to other sizes.

Why does this function need to be virtual? Was it virtual before?

> Source/WebCore/platform/ScrollableArea.cpp:408
> +    return IntRect(scrollPosition().x(),
> +		      scrollPosition().y(),
> +		      std::max(0, visibleWidth() + verticalScrollbarWidth),
> +		      std::max(0, visibleHeight() +
horizontalScrollbarHeight));

WebKit coding style explicitly suggests avoiding lining up subsequent lines
like this. Please change to match the coding style, or argume for getting that
changed.

> Source/WebCore/platform/chromium/FramelessScrollView.cpp:68
> +    // FIXME

More detail here?


More information about the webkit-reviews mailing list