Bug 112850

Summary:

[Qt][WK2] Remove direct references to WebPageProxy from QQuickWebPage.

Product:

WebKit

Reporter:

Michael Brüning <michael.bruning>

Component:

New Bugs

Assignee:

Michael Brüning <michael.bruning>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abecsi, allan.jensen, cmarcelo, jturcotte, menard, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

109204

Attachments:

Description	Flags
Patch	none
Disassembly.	none
Patch	none
Patch	kling: review+

Description Michael Brüning 2013-03-20 15:11:08 PDT

[Qt][WK2] Remove direct references to WebPageProxy from QQuickWebPage.

Comment 1 Michael Brüning 2013-03-20 15:19:54 PDT

Created attachment 194133 [details]
Patch

Comment 2 Michael Brüning 2013-03-21 04:55:19 PDT

Created attachment 194229 [details]
Disassembly.

Comment 3 Michael Brüning 2013-03-21 04:56:06 PDT

Comment on attachment 194229 [details]
Disassembly.

Please disregard disassembly - wrong window...

Comment 4 Jocelyn Turcotte 2013-03-21 08:09:57 PDT

Comment on attachment 194133 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:56
>  QQuickWebPagePrivate::QQuickWebPagePrivate(QQuickWebPage* q, QQuickWebView* viewportItem)
>      : q(q)
>      , viewportItem(viewportItem)
> -    , webPageProxy(0)
> +    , webViewPrivate(0)

We already have a pointer to viewportItem, so maybe we could either replace it everywhere with your webViewPrivate->q_func() or access the webViewPrivate through viewportItem->d_func()?
Whichever is more convenient.

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:946
> +    if (!webPageProxy)
> +        return 0;
> +    if (!webPageProxy->drawingArea() || !webPageProxy->drawingArea()->coordinatedLayerTreeHostProxy())
> +        return 0;

Those can be merged I believe.

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:142
> +    // FIXME: The following three methods are only here for splitting the webview move into several patches.
> +    // They be removed then and their counterparts in QRawWebView will be used.

This should rather be written in the ChangeLog.

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:146
> +    void setIntrinsicDeviceScaleFactor(float);
>  protected:

Missing empty line.

Comment 5 Michael Brüning 2013-03-21 09:30:20 PDT

Created attachment 194280 [details]
Patch

Comment 6 Jocelyn Turcotte 2013-03-21 10:05:53 PDT

Comment on attachment 194280 [details]
Patch

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

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:942
> +    if (!webPageProxy || !webPageProxy->drawingArea() || !webPageProxy->drawingArea()->coordinatedLayerTreeHostProxy())
> +        return 0;

Nit: It would read slightly better to move the return 0; at the end, remove the negations and return the correct pointer here.

LGTM otherwise.

Comment 7 Michael Brüning 2013-03-21 11:58:18 PDT

Created attachment 194311 [details]
Patch

Comment 8 Michael Brüning 2013-04-09 04:52:08 PDT

Committed r148011: <http://trac.webkit.org/changeset/148011>