[webkit-reviews] review granted: [Bug 116514] New Flickr doesn't get fast scrolling but should : [Attachment 202379] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 116514: New Flickr doesn't get fast scrolling but should
https://bugs.webkit.org/show_bug.cgi?id=116514

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

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


> Source/WebCore/rendering/RenderObject.cpp:130
> +    UNUSED_PARAM(frameView);

Seems wrong to have this UNUSED_PARAM be unconditional, when the use of the
parameter is conditional.

>> Source/WebCore/rendering/RenderObject.cpp:133
>> +#if PLATFORM(QT)
> 
> Do not add platform specific code in WebCore outside of platform. 
[build/webcore_platform_layering_violation] [5]

I don’t think the style bot is being helpful here.

> Source/WebCore/rendering/RenderObject.cpp:2490
> +    if (view()->frameView()) {

I think we should put the FrameView into a local because it’s used two more
times below and the code would be more concise without repeating it.

> Source/WebCore/rendering/RenderObject.cpp:2491
> +	   bool repaintFixedBackgroundsOnScroll =
shouldRepaintFixedBackgroundsOnScroll(view()->frameView());

Why the local variable here? I think it makes the code harder to read.