[webkit-reviews] review granted: [Bug 78401] Move ScrollingCoordinator out of ENABLE(THREADED_SCROLLING) ifdef and enable on all platforms : [Attachment 127631] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 17 16:10:58 PST 2012


Adam Barth <abarth at webkit.org> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 78401: Move ScrollingCoordinator out of ENABLE(THREADED_SCROLLING) ifdef
and enable on all platforms
https://bugs.webkit.org/show_bug.cgi?id=78401

Attachment 127631: Patch
https://bugs.webkit.org/attachment.cgi?id=127631&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=127631&action=review


Ok.  Generally, this approach is better than having two headers with the same
name, although we do use that approach elsewhere (e.g., ResourceHandle).

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:219
> +#if ENABLE(THREADED_SCROLLING)

btw, THREADED_SCROLLING should be a USE not an ENABLE.

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:294
> +#if !PLATFORM(MAC) && !PLATFORM(CHROMIUM)

These ifdefs are slightly unfortunate.	It would be better if there was a way
to capture this set without using PLATFORM names.

> Source/WebCore/page/scrolling/ScrollingTreeState.cpp:27
> +

nit: this change looks spurious


More information about the webkit-reviews mailing list