Bug 66668

Summary: [Qt][WK2] Split the QtWebPageProxy into PageClient and QtPageProxy
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: WebKit2Assignee: Jesus Sanchez-Palencia <jesus>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, hausmann, jesus, kenneth, webkit.review.bot, zoltan
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 72943    
Attachments:
Description Flags
Patch hausmann: review+

Description Benjamin Poulain 2011-08-22 06:16:25 PDT
Currently, the QtWebPageProxy does too much by aggregating the calls back and forth between the Views and the Proxy. We should try to split this class into QtPageClient (interfacing to the viewinterface) and the page proxy (QtPageProxy?).
Comment 1 Kenneth Rohde Christiansen 2011-08-22 06:20:22 PDT
woot!
Comment 2 Jesus Sanchez-Palencia 2011-11-25 14:57:06 PST
We are handling this on a Meta bug now.

In fact, we have extended the idea of this clean-up into splitting everything but the PageClient out of QtWebPageProxy. On a first moment, I actually worked on splitting it like that (PageClient and QtPageProxy) but in the end we had two strongly related "monsters".

The new approach turned out to be the best solution so far.

*** This bug has been marked as a duplicate of bug 72943 ***
Comment 3 Jesus Sanchez-Palencia 2011-12-01 11:15:03 PST
(In reply to comment #2)
> *** This bug has been marked as a duplicate of bug 72943 ***

I'm reopening this and marking it as a blocker of https://bugs.webkit.org/show_bug.cgi?id=72943 .

After another try, I believe I have a proposal for this that might make sense and I would like to gather your opinion on this. A patch proposal is coming soon.
Comment 4 Jesus Sanchez-Palencia 2011-12-01 12:51:30 PST
Created attachment 117465 [details]
Patch
Comment 5 Simon Hausmann 2011-12-02 01:29:34 PST
Comment on attachment 117465 [details]
Patch

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

> Source/WebKit2/UIProcess/qt/QtPageClient.cpp:173
> +    m_qtWebPageProxy->didReceiveMessageFromNavigatorQtObject(message);

QtWebPageProxy::didReceiveMessageFromNavigatorQtObject should probably be folded into this class altogether or this should just call directly the qml web view.
Comment 6 Simon Hausmann 2011-12-02 01:37:02 PST
(In reply to comment #0)
> Currently, the QtWebPageProxy does too much by aggregating the calls back and forth between the Views and the Proxy. We should try to split this class into QtPageClient (interfacing to the viewinterface) and the page proxy (QtPageProxy?).

Perhaps we can even avoid a QtPageProxy altogether. The state that I see in that class currently looks movable into the webview or into the concrete clients. So perhaps we can eliminate this proxy altogether in the future. Does that make sense? :)
Comment 7 Benjamin Poulain 2011-12-02 01:57:24 PST
(In reply to comment #6)
> Perhaps we can even avoid a QtPageProxy altogether. The state that I see in that class currently looks movable into the webview or into the concrete clients. So perhaps we can eliminate this proxy altogether in the future. Does that make sense? :)

QtWebPageProxy is an old artifact originating from QWKPage. Since you only have one view left, it probably makes sense to kill that class.
Comment 8 Simon Hausmann 2011-12-02 05:11:38 PST
Comment on attachment 117465 [details]
Patch

r=me
Comment 9 Jesus Sanchez-Palencia 2011-12-02 05:38:18 PST
Committed r101788: <http://trac.webkit.org/changeset/101788>