[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