Bug 64739 – [Qt] Make the WebContextMenuProxyQt handle the full interactions between the views and the WebPageProxy

• Component: WebKit2
• Status: RESOLVED FIXED

[Benjamin Poulain]

The current design of WebContextMenuProxyQt still suffer from the old QWKPage:
-the actions of the menu are still QtWebPageProxy::WebAction which was a terrible idea (because those actions are really only valid in when a context menu is up)
-the functions of the views are called through the QtWebPageProxy wich defeat the purpose of WebContextMenuProxy

The WebContextMenuProxyQt needs to be promoted to a real proxy on its own with the full responsibility of the context menu.

[Benjamin Poulain]

Created attachment 101183 [details]
Patch

[Andreas Kling]

Comment on attachment 101183 [details]
Patch

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

> Source/WebKit2/UIProcess/qt/WebContextMenuProxyQt.cpp:105
> +            connect(qtAction, SIGNAL(triggered(bool)), this, SLOT(actionTriggered(bool)), Qt::DirectConnection);

The Qt::DirectConnection here can be omitted, since the object is on the same thread.

[Andreas Kling]

Comment on attachment 101183 [details]
Patch

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

> Source/WebKit2/ChangeLog:3
> +        [Qt] Make the WebContextMenuProxyQt handle the full interactions between the views and the WebPageProxy

Where's the customary [WK2] huh? Unacceptable, sir!

>> Source/WebKit2/UIProcess/qt/WebContextMenuProxyQt.cpp:105
>> +            connect(qtAction, SIGNAL(triggered(bool)), this, SLOT(actionTriggered(bool)), Qt::DirectConnection);
> 
> The Qt::DirectConnection here can be omitted, since the object is on the same thread.

Apparently this is meant to protect against future refactorings, since the slot assumes that QObject::sender() is valid, which may not be the case given a queued connection. That's somewhat reasonable, so I'm retracting this moan.

[Benjamin Poulain]

Committed r91758: <http://trac.webkit.org/changeset/91758>