Bug 112850

Summary: [Qt][WK2] Remove direct references to WebPageProxy from QQuickWebPage.
Product: WebKit Reporter: Michael Brüning <michael.bruning>
Component: New BugsAssignee: Michael Brüning <michael.bruning>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, allan.jensen, cmarcelo, jturcotte, menard, webkit.review.bot
Priority: P2    
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+

Michael Brüning
Reported 2013-03-20 15:11:08 PDT
[Qt][WK2] Remove direct references to WebPageProxy from QQuickWebPage.
Attachments
Patch (17.86 KB, patch)
2013-03-20 15:19 PDT, Michael Brüning
no flags
Disassembly. (5.01 KB, text/plain)
2013-03-21 04:55 PDT, Michael Brüning
no flags
Patch (17.57 KB, patch)
2013-03-21 09:30 PDT, Michael Brüning
no flags
Patch (17.57 KB, patch)
2013-03-21 11:58 PDT, Michael Brüning
kling: review+
Michael Brüning
Comment 1 2013-03-20 15:19:54 PDT
Michael Brüning
Comment 2 2013-03-21 04:55:19 PDT
Created attachment 194229 [details] Disassembly.
Michael Brüning
Comment 3 2013-03-21 04:56:06 PDT
Comment on attachment 194229 [details] Disassembly. Please disregard disassembly - wrong window...
Jocelyn Turcotte
Comment 4 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.
Michael Brüning
Comment 5 2013-03-21 09:30:20 PDT
Jocelyn Turcotte
Comment 6 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.
Michael Brüning
Comment 7 2013-03-21 11:58:18 PDT
Michael Brüning
Comment 8 2013-04-09 04:52:08 PDT
Note You need to log in before you can comment on or make changes to this bug.