Bug 66668 – [Qt][WK2] Split the QtWebPageProxy into PageClient and QtPageProxy

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

[Benjamin Poulain]

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?).

[Kenneth Rohde Christiansen]

woot!

[Jesus Sanchez-Palencia]

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 ***

[Jesus Sanchez-Palencia]

(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.

[Jesus Sanchez-Palencia]

Created attachment 117465 [details]
Patch

[Simon Hausmann]

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.

[Simon Hausmann]

(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? :)

[Benjamin Poulain]

(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.

[Simon Hausmann]

Comment on attachment 117465 [details]
Patch

r=me

[Jesus Sanchez-Palencia]

Committed r101788: <http://trac.webkit.org/changeset/101788>