Bug 97922 – [BlackBerry] Extend composited in-region scrolling to iframes/frames

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit BlackBerry

[Antonio Gomes]

SSIA

[Antonio Gomes]

Created attachment 166268 [details]
patch

[WebKit Review Bot]

Attachment 166268 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/blackberry/Api/InRegionScrol..." exit_code: 1
Source/WebKit/blackberry/Api/InRegionScroller.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yong Li]

Comment on attachment 166268 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166268&action=review

r+ with some comments

> Source/WebKit/blackberry/Api/InRegionScroller.cpp:154
> +            if (scrollTarget == Platform::ScrollViewBase::BlockElement) {
> +                RenderLayerBacking* backing = static_cast<RenderLayerBacking*>(graphicsLayer->client());
> +                layer = backing->owningLayer();
> +            } else {
> +                RenderLayerCompositor* compositor = static_cast<RenderLayerCompositor*>(graphicsLayer->client());
> +                layer = compositor->rootRenderLayer();
> +            }

static_cast<> worries me. Is it possible to add something to GraphicsLayerClient?

> Source/WebKit/blackberry/Api/InRegionScroller.h:41
> +    bool setScrollPositionWebKitThread(unsigned camouflagedLayer, const Platform::IntPoint& /*scrollPosition*/,
> +        bool /*acceleratedScrolling*/, Platform::ScrollViewBase::ScrollTarget);

why are we using /* */ here?

[Antonio Gomes]

Comment on attachment 166268 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166268&action=review

>> Source/WebKit/blackberry/Api/InRegionScroller.cpp:33
>> +#include "RenderLayerCompositor.h"
> 
> Alphabetical sorting problem.  [build/include_order] [4]

fixed

>> Source/WebKit/blackberry/Api/InRegionScroller.cpp:154
>> +            }
> 
> static_cast<> worries me. Is it possible to add something to GraphicsLayerClient?

I guess so: we can add ::isRenderLayerCompositing() ::isRenderLayerBacking() methods to GraphicsLayerClient.h maybe. Will file a bug.

>> Source/WebKit/blackberry/Api/InRegionScroller.h:41
>> +        bool /*acceleratedScrolling*/, Platform::ScrollViewBase::ScrollTarget);
> 
> why are we using /* */ here?

will change.

[Antonio Gomes]

https://trac.webkit.org/changeset/129916