Bug 46345 – [Qt] Refactor QtWebKitPlatformPlugin interface

• Component: New Bugs
• Status: RESOLVED FIXED

[Tor Arne Vestbø]

[Qt] Refactor QtWebKitPlatformPlugin interface

[Tor Arne Vestbø]

Created attachment 68507 [details]
Patch

[Tor Arne Vestbø]

Committed r68128: <http://trac.webkit.org/changeset/68128>

[WebKit Review Bot]

http://trac.webkit.org/changeset/68128 might have broken Qt Linux Release minimal, Qt Linux ARMv5 Release, and Qt Linux ARMv7 Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/68128
http://trac.webkit.org/changeset/68129

[Tor Arne Vestbø]

Commited r68130: <http://trac.webkit.org/changeset/68130>

[Ademar Reis]

Cherry-picking it to 2.1 requires a backport due to conflicts with r64829  <http://trac.webkit.org/changeset/64829>, Bug 43427

The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829:

(WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp)
-QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod()
+QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod()

BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1)

/o\ :-)

Does it make sense to backport only this patch to 2.1, without the previous changes?

[Luiz Agostini]

(In reply to comment #5)
> Cherry-picking it to 2.1 requires a backport due to conflicts with r64829  <http://trac.webkit.org/changeset/64829>, Bug 43427
> 
> The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829:
> 
> (WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp)
> -QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod()
> +QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod()
> 
> BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1)
> 
> /o\ :-)
> 
> Does it make sense to backport only this patch to 2.1, without the previous changes?

I think that in 2.1 it should be:

QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod()
{
     QWebKitPlatformPlugin* p = plugin();
     if (!p)
       return 0;

-    QWebSelectMethod* selector = p->createSelectInputMethod();
+   QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections)) : 0;

    if (!selector)
        return 0;

    return new SelectInputMethodWrapper(selector);
}

[Luiz Agostini]

(In reply to comment #6)
> (In reply to comment #5)
> > Cherry-picking it to 2.1 requires a backport due to conflicts with r64829  <http://trac.webkit.org/changeset/64829>, Bug 43427
> > 
> > The most important change affecting the cherry-pick appears to be the change of the createSelectInputMethod() signature in r64829:
> > 
> > (WebKit/qt/WebCoreSupport/QtPlatformPlugin.cpp)
> > -QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod()
> > +QWebSelectMethod* QtPlatformPlugin::createSelectInputMethod()
> > 
> > BTW, r64829 is a large patch and trying to cherry-pick it I see even more conflicts (6 conflicting files). Apparently it requires the changes from Bug 42592 (also a large patch, with 9 file conflicts when cherry-picking to 2.1)
> > 
> > /o\ :-)
> > 
> > Does it make sense to backport only this patch to 2.1, without the previous changes?
> 
> I think that in 2.1 it should be:
> 
> QtAbstractWebPopup* QtPlatformPlugin::createSelectInputMethod()
> {
>      QWebKitPlatformPlugin* p = plugin();
>      if (!p)
>        return 0;
> 
> -    QWebSelectMethod* selector = p->createSelectInputMethod();
> +   QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections)) : 0;
> 
>     if (!selector)
>         return 0;
> 
>     return new SelectInputMethodWrapper(selector);
> }

please ignore the ": 0" in inserted line in the previous comment. :)

the correct line is:

+   QWebSelectMethod* selector =qobject_cast<QWebSelectMethod*>(p->createExtension(QWebKitPlatformPlugin::MultipleSelections));

[Ademar Reis]

Besides the already discussed change on QtPlatformPlugin::createSelectInputMethod(), we also need the patch from Bug 46402 and the following change:

--- a/WebCore/platform/qt/PlatformHapticsQt.cpp
+++ b/WebCore/platform/qt/PlatformHapticsQt.cpp
@@ -33,7 +33,7 @@ void initHaptics()
 
     if (!pluginLoaded) {
         if (platformPlugin.plugin() && platformPlugin.plugin()->supportsExtension(QWebKitPlatformPlugin::Haptics))
-            player = platformPlugin.plugin()->createHapticFeedbackPlayer();
+            player = qobject_cast<QWebHapticFeedbackPlayer*>(platformPlugin.plugin()->createExtension(QWebKitPlatformPlugin::Haptics));
         pluginLoaded = true;
     }
 }

With those changes I can build and run QtTestBrowser - although I couldn't manage to test the new plugin interface. Anything else?

Does somebody has a testcase or a plugin I can use to test?

[Tor Arne Vestbø]

(In reply to comment #8)
> Besides the already discussed change on QtPlatformPlugin::createSelectInputMethod(), we also need the patch from Bug 46402 and the following change:
> 
> --- a/WebCore/platform/qt/PlatformHapticsQt.cpp
> +++ b/WebCore/platform/qt/PlatformHapticsQt.cpp
> @@ -33,7 +33,7 @@ void initHaptics()
> 
>      if (!pluginLoaded) {
>          if (platformPlugin.plugin() && platformPlugin.plugin()->supportsExtension(QWebKitPlatformPlugin::Haptics))
> -            player = platformPlugin.plugin()->createHapticFeedbackPlayer();
> +            player = qobject_cast<QWebHapticFeedbackPlayer*>(platformPlugin.plugin()->createExtension(QWebKitPlatformPlugin::Haptics));
>          pluginLoaded = true;
>      }
>  }
> 

This part should ideally be refactored into the WebCore-side helper QtPlatformPlugin like the others, so the qobject_cast is hidden.

[Ademar Reis]

(In reply to comment #9)
> (In reply to comment #8)
>
<snip>
> --- a/WebCore/platform/qt/PlatformHapticsQt.cpp
> +++ b/WebCore/platform/qt/PlatformHapticsQt.cpp
<snip>
> 
> This part should ideally be refactored into the WebCore-side helper QtPlatformPlugin like the others, so the qobject_cast is hidden.

Sure. It also applies to trunk. Bug 46402 has been updated (includes patch)

[Ademar Reis]

Revision r68128 cherry-picked into qtwebkit-2.1 with commit 9b5a4b4 <http://gitorious.org/webkit/qtwebkit/commit/9b5a4b4>
Revision r68130 cherry-picked into qtwebkit-2.1 with commit 0f90c4b <http://gitorious.org/webkit/qtwebkit/commit/0f90c4b>