Bug 118585 – [WK2] Share Qt port's codes to find zoomable area with CoordinatedGraphics.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Eunmi Lee]

The codes to find zoomable area will be used in the coordinated graphics, so modify codes for Qt port to share with coordinated graphics.

[Eunmi Lee]

Created attachment 206509 [details]
Patch

[Noam Rosenthal]

Comment on attachment 206509 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2952
> +#if PLATFORM(QT) || USE(COORDINATED_GRAPHICS)

No need for #if PLATFORM(QT) here
Qt already uses coordinated graphics.

> Source/WebKit2/UIProcess/WebPageProxy.cpp:2953
> +void WebPageProxy::findZoomableAreaForPoint(const IntPoint& point, const IntSize& area)

Maybe we can put this in a new file (WebPageProxyCoordinatedGraphics)?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3221
> +    IntRect zoomableArea = node->pixelSnappedBoundingBox();

This seems like a separate fix

[Eunmi Lee]

Comment on attachment 206509 [details]
Patch

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

Thank you for comments :)

>> Source/WebKit2/UIProcess/WebPageProxy.cpp:2952
>> +#if PLATFORM(QT) || USE(COORDINATED_GRAPHICS)
> 
> No need for #if PLATFORM(QT) here
> Qt already uses coordinated graphics.

That's good, I will modify all "PLATFORM(QT) || USE(COORDINATED_GRAPHICS)".

>> Source/WebKit2/UIProcess/WebPageProxy.cpp:2953
>> +void WebPageProxy::findZoomableAreaForPoint(const IntPoint& point, const IntSize& area)
> 
> Maybe we can put this in a new file (WebPageProxyCoordinatedGraphics)?

OK. I will add WebPageProxyCoordinatedGraphics.cpp.

>> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3221
>> +    IntRect zoomableArea = node->pixelSnappedBoundingBox();
> 
> This seems like a separate fix

Right. I will make separate patch for that.

[Eunmi Lee]

Created attachment 206529 [details]
Patch

[Noam Rosenthal]

Comment on attachment 206529 [details]
Patch

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

I'm ok with the change in general, but the Changelog needs to be rewritten and it needs owner approval since it touches WebPage.*.

> Source/WebKit2/ChangeLog:13
> +        The codes to find zoomable area will be used in the coordinated graphics,
> +        so modify codes to be able to be used not only in the Qt port but also
> +        coordinated graphics.
> +
> +        Additionally, Add the WKViewFindZoomableAreaForRect API and didFindZoomableArea
> +        callback for coordinated graphics and they will be used in the EFL port.

Please use correct English.

[Eunmi Lee]

Comment on attachment 206529 [details]
Patch

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

>> Source/WebKit2/ChangeLog:13
>> +        callback for coordinated graphics and they will be used in the EFL port.
> 
> Please use correct English.

OK, I will re-write the changelog.

[Eunmi Lee]

Created attachment 206972 [details]
Patch

Update changelog.

[Noam Rosenthal]

Comment on attachment 206972 [details]
Patch

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

I'm ok with this, but it touches PageClient.h so please ask for WK2 owner review.

> Source/WebKit2/ChangeLog:11
> +        to use that fuction, so extend codes for Qt port to be usable in the
> +        CoordinatedGraphics and add API and callback to use that.

extend codes for Qt port -> extract the code from the Qt port

[Eunmi Lee]

OK, thanks for review!

(In reply to comment #8)
> (From update of attachment 206972 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206972&action=review
> 
> I'm ok with this, but it touches PageClient.h so please ask for WK2 owner review.
> 
> > Source/WebKit2/ChangeLog:11
> > +        to use that fuction, so extend codes for Qt port to be usable in the
> > +        CoordinatedGraphics and add API and callback to use that.
> 
> extend codes for Qt port -> extract the code from the Qt port

[Eunmi Lee]

Created attachment 206982 [details]
Patch

Update changelog.

[Noam Rosenthal]

Comment on attachment 206982 [details]
Patch

LGTM, requires owner approval.

[WebKit Commit Bot]

Comment on attachment 206982 [details]
Patch

Clearing flags on attachment: 206982

Committed r152878: <http://trac.webkit.org/changeset/152878>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.