Bug 97922 - [BlackBerry] Extend composited in-region scrolling to iframes/frames
Summary: [BlackBerry] Extend composited in-region scrolling to iframes/frames
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antonio Gomes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-28 09:55 PDT by Antonio Gomes
Modified: 2012-09-28 10:23 PDT (History)
4 users (show)

See Also:


Attachments
patch (11.44 KB, patch)
2012-09-28 09:59 PDT, Antonio Gomes
yong.li.webkit: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2012-09-28 09:55:22 PDT
SSIA
Comment 1 Antonio Gomes 2012-09-28 09:59:08 PDT
Created attachment 166268 [details]
patch
Comment 2 WebKit Review Bot 2012-09-28 10:02:24 PDT
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.
Comment 3 Yong Li 2012-09-28 10:08:13 PDT
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?
Comment 4 Antonio Gomes 2012-09-28 10:23:18 PDT
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.
Comment 5 Antonio Gomes 2012-09-28 10:23:56 PDT
https://trac.webkit.org/changeset/129916