Bug 66668 - [Qt][WK2] Split the QtWebPageProxy into PageClient and QtPageProxy
Summary: [Qt][WK2] Split the QtWebPageProxy into PageClient and QtPageProxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jesus Sanchez-Palencia
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 72943
  Show dependency treegraph
 
Reported: 2011-08-22 06:16 PDT by Benjamin Poulain
Modified: 2011-12-02 05:38 PST (History)
6 users (show)

See Also:


Attachments
Patch (30.62 KB, patch)
2011-12-01 12:51 PST, Jesus Sanchez-Palencia
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>