RESOLVED FIXED 108826
[Qt][WK2] Replace more uses of WebPageProxy with WKPage in QQuickWebView 
https://bugs.webkit.org/show_bug.cgi?id=108826
Summary [Qt][WK2] Replace more uses of WebPageProxy with WKPage in QQuickWebView
Simon Hausmann
Reported 2013-02-04 08:08:12 PST
[Qt][WK2] Replace more uses of WebPageProxy with WKPage in QQuickWebView
Attachments
Patch (12.72 KB, patch)
2013-02-04 08:10 PST, Simon Hausmann 		kenneth: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Simon Hausmann
Comment 1 2013-02-04 08:10:04 PST
Created attachment 186377 [details] Patch
Kenneth Rohde Christiansen
Comment 2 2013-02-04 08:15:13 PST
Comment on attachment 186377 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186377&action=review LGTM > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1016 > + static WKStringRef messageName = WKStringCreateWithUTF8CString("MessageToNavigatorQtObject"); hmm ok, so static and no adoption here... guess that is fine > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1514 > + WKPageGoBack(d->webPage.get()); why not add an inline wkPage() const accessor?
Simon Hausmann
Comment 3 2013-02-05 00:37:25 PST
Comment on attachment 186377 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186377&action=review >> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:1514 >> + WKPageGoBack(d->webPage.get()); > > why not add an inline wkPage() const accessor? Not sure that it's worth it :)
Benjamin Poulain
Comment 4 2013-02-05 01:07:29 PST
Comment on attachment 186377 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186377&action=review This looks like a step in the right direction. I sign off on this for WebKit2. > Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:165 > RefPtr<WebKit::WebPageProxy> webPageProxy; > + WKRetainPtr<WKPageRef> webPage; Why keep a reference to webPageProxy? This defeats the purpose. :(
Simon Hausmann
Comment 5 2013-02-05 01:17:03 PST
Comment on attachment 186377 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186377&action=review Thanks for the sign-off, Benjamin! >> Source/WebKit2/UIProcess/API/qt/qquickwebview_p_p.h:165 >> + WKRetainPtr<WKPageRef> webPage; > > Why keep a reference to webPageProxy? This defeats the purpose. :( There are still a few references left, our plan is to definitely get rid of this member as the last step of the increments.
Kenneth Rohde Christiansen
Comment 6 2013-02-05 02:14:38 PST
Comment on attachment 186377 [details] Patch r=me as signed off by Benjamin
Simon Hausmann
Comment 7 2013-02-05 03:00:35 PST
Committed r141882: <http://trac.webkit.org/changeset/141882>
Note You need to log in before you can comment on or make changes to this bug.