Bug 30118 – [Qt] QWebPage autotest crash: createViewlessPlugin

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Jędrzej Nowacki]

tst_QWebPage::createViewlessPlugin autotest crash.

Traceback:

(gdb) bt
#0  0xb71a626e in WebCore::FrameLoaderClientQt::createPlugin (this=0x8dcbec0, pluginSize=@0xbfda232c, element=0x8decb00, url=@0xbfda2380, paramNames=@0xbfda24e8, paramValues=@0xbfda24dc, mimeType=@0xbfda24f4, loadManually=false)
    at /home/nierob/dev/webkit/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp:1233
#1  0xb6e98082 in WebCore::FrameLoader::loadPlugin (this=0x8dcc08c, renderer=0x8dce614, url=@0xbfda2380, mimeType=@0xbfda24f4, paramNames=@0xbfda24e8, paramValues=@0xbfda24dc, useFallback=false) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:1470
#2  0xb6ea533a in WebCore::FrameLoader::requestObject (this=0x8dcc08c, renderer=0x8dce614, url=@0xbfda24f8, frameName=@0xb80b25d4, mimeType=@0xbfda24f4, paramNames=@0xbfda24e8, paramValues=@0xbfda24dc) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:1405
#3  0xb70ce921 in WebCore::RenderPartObject::updateWidget (this=0x8dce614, onlyCreateNonNetscapePlugins=true) at /home/nierob/dev/webkit/WebCore/rendering/RenderPartObject.cpp:252
#4  0xb6dcb206 in WebCore::HTMLObjectElement::updateWidget (this=0x8decb00) at /home/nierob/dev/webkit/WebCore/html/HTMLObjectElement.cpp:170
#5  0xb6dddbea in WebCore::HTMLPlugInElement::updateWidgetCallback (n=0x8decb00) at /home/nierob/dev/webkit/WebCore/html/HTMLPlugInElement.cpp:180
#6  0xb6bebe2d in WebCore::ContainerNode::dispatchPostAttachCallbacks () at /home/nierob/dev/webkit/WebCore/dom/ContainerNode.cpp:572
#7  0xb6bec23a in WebCore::ContainerNode::resumePostAttachCallbacks (this=0x8dccbb0) at /home/nierob/dev/webkit/WebCore/dom/ContainerNode.cpp:544
#8  0xb6c25a9e in WebCore::Document::recalcStyle (this=0x8dccbb0, change=WebCore::Node::NoChange) at /home/nierob/dev/webkit/WebCore/dom/Document.cpp:1292
#9  0xb6c21ab1 in WebCore::Document::updateStyleIfNeeded (this=0x8dccbb0) at /home/nierob/dev/webkit/WebCore/dom/Document.cpp:1315
#10 0xb6c14505 in WebCore::Document::updateStyleForAllDocuments () at /home/nierob/dev/webkit/WebCore/dom/Document.cpp:1332
#11 0xb6c65c71 in WebCore::Node::dispatchGenericEvent (this=0x8dccbb0, prpEvent={m_ptr = 0xbfda2858}) at /home/nierob/dev/webkit/WebCore/dom/Node.cpp:2566
#12 0xb6c65e0a in WebCore::Node::dispatchEvent (this=0x8dccbb0, prpEvent={m_ptr = 0xbfda2890}) at /home/nierob/dev/webkit/WebCore/dom/Node.cpp:2447
#13 0xb6c2279e in WebCore::Document::finishedParsing (this=0x8dccbb0) at /home/nierob/dev/webkit/WebCore/dom/Document.cpp:4018
#14 0xb6dda2a4 in WebCore::HTMLParser::finished (this=0x8dde810) at /home/nierob/dev/webkit/WebCore/html/HTMLParser.cpp:1635
#15 0xb6df5684 in WebCore::HTMLTokenizer::end (this=0x8dcdc18) at /home/nierob/dev/webkit/WebCore/html/HTMLTokenizer.cpp:1859
#16 0xb6df5b5c in WebCore::HTMLTokenizer::finish (this=0x8dcdc18) at /home/nierob/dev/webkit/WebCore/html/HTMLTokenizer.cpp:1899
#17 0xb6bef199 in WebCore::Document::finishParsing (this=0x8dccbb0) at /home/nierob/dev/webkit/WebCore/dom/Document.cpp:1860
#18 0xb6e9a51a in WebCore::FrameLoader::endIfNotLoadingMainResource (this=0x8dcc08c) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:986
#19 0xb6e9a579 in WebCore::FrameLoader::end (this=0x8dcc08c) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:971
#20 0xb6e7dfba in WebCore::DocumentLoader::finishedLoading (this=0x8dd39e8) at /home/nierob/dev/webkit/WebCore/loader/DocumentLoader.cpp:330
#21 0xb6e920f7 in WebCore::FrameLoader::finishedLoading (this=0x8dcc08c) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:2875
#22 0xb6ebfe12 in WebCore::MainResourceLoader::didFinishLoading (this=0x8dd4c80) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:375
#23 0xb6ec1065 in WebCore::MainResourceLoader::continueAfterContentPolicy (this=0x8dd4c80, contentPolicy=WebCore::PolicyUse, r=@0x8dd4c98) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:262
#24 0xb6ec12c1 in WebCore::MainResourceLoader::continueAfterContentPolicy (this=0x8dd4c80, policy=WebCore::PolicyUse) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:278
#25 0xb6ec12fa in WebCore::MainResourceLoader::callContinueAfterContentPolicy (argument=0x8dd4c80, policy=WebCore::PolicyUse) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:270
#26 0xb6e9411d in WebCore::FrameLoader::checkContentPolicy (this=0x8dcc08c, MIMEType=@0x8dd4cc8, function=0xb6ec12d6 <WebCore::MainResourceLoader::callContinueAfterContentPolicy(void*, WebCore::PolicyAction)>, argument=0x8dd4c80)
    at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:2241
#27 0xb6ec0242 in WebCore::MainResourceLoader::didReceiveResponse (this=0x8dd4c80, r=@0xbfda2cec) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:323
#28 0xb6ebee92 in WebCore::MainResourceLoader::handleDataLoadNow (this=0x8dd4c80) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:420
#29 0xb6ebefcc in WebCore::MainResourceLoader::handleDataLoadSoon (this=0x8dd4c80, r=@0xbfda2f18) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:440
#30 0xb6ebf7a6 in WebCore::MainResourceLoader::loadNow (this=0x8dd4c80, r=@0xbfda2f18) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:467
#31 0xb6ebfc68 in WebCore::MainResourceLoader::load (this=0x8dd4c80, r=@0x8dd3c00, substituteData=@0x8dd3af4) at /home/nierob/dev/webkit/WebCore/loader/MainResourceLoader.cpp:495
#32 0xb6e7a675 in WebCore::DocumentLoader::startLoadingMainResource (this=0x8dd39e8, identifier=1) at /home/nierob/dev/webkit/WebCore/loader/DocumentLoader.cpp:790
#33 0xb6e8fa83 in WebCore::FrameLoader::continueLoadAfterWillSubmitForm (this=0x8dcc08c) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:3174
#34 0xb6ea4cd4 in WebCore::FrameLoader::continueLoadAfterNavigationPolicy (this=0x8dcc08c, formState={m_ptr = 0xbfda30f4}, shouldContinue=true) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:3808
#35 0xb6ea4d2f in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy (argument=0x8dcc08c, request=@0x8dd3c00, formState={m_ptr = 0xbfda31c8}, shouldContinue=true) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:3739
#36 0xb6e8d6d2 in WebCore::FrameLoader::checkNavigationPolicy (this=0x8dcc08c, request=@0x8dd3c00, loader=0x8dd39e8, formState={m_ptr = 0xbfda329c},
    function=0xb6ea4cdc <WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>, argument=0x8dcc08c) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:3681
#37 0xb6e90798 in WebCore::FrameLoader::loadWithDocumentLoader (this=0x8dcc08c, loader=0x8dd39e8, type=WebCore::FrameLoadTypeStandard, prpFormState={m_ptr = 0xbfda32e0}) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:2154
#38 0xb6e90978 in WebCore::FrameLoader::load (this=0x8dcc08c, newDocumentLoader=0x8dd39e8) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:2115
#39 0xb6e90b8b in WebCore::FrameLoader::load (this=0x8dcc08c, request=@0xbfda33b8, substituteData=@0xbfda3458, lockHistory=false) at /home/nierob/dev/webkit/WebCore/loader/FrameLoader.cpp:2056
#40 0xb71b4755 in QWebFrame::setHtml (this=0x8dca958, html=@0xbfda35b0, baseUrl=@0xbfda35b4) at /home/nierob/dev/webkit/WebKit/qt/Api/qwebframe.cpp:724
#41 0x08055099 in tst_QWebPage::createViewlessPlugin (this=0xbfda42f4) at /home/nierob/dev/webkit/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:664
#42 0x08066127 in tst_QWebPage::qt_metacall (this=0xbfda42f4, _c=QMetaObject::InvokeMetaMethod, _id=15, _a=0xbfda3690) at /home/nierob/dev/webkit/WebKitBuild/Debug/WebKit/qt/tests/qwebpage/./tst_qwebpage.moc:132
#43 0xb49add87 in QMetaObject::metacall (object=0xbfda42f4, cl=QMetaObject::InvokeMetaMethod, idx=19, argv=0xbfda3690) at /home/nierob/dev/qt/src/corelib/kernel/qmetaobject.cpp:237
#44 0xb49b156f in QMetaMethod::invoke (this=0xbfda3a14, object=0xbfda42f4, connectionType=Qt::DirectConnection, returnValue={<QGenericArgument> = {_data = 0x0, _name = 0x0}, <No data fields>}, val0={_data = 0x0, _name = 0x0}, val1={_data = 0x0, _name = 0x0}, val2=
      {_data = 0x0, _name = 0x0}, val3={_data = 0x0, _name = 0x0}, val4={_data = 0x0, _name = 0x0}, val5={_data = 0x0, _name = 0x0}, val6={_data = 0x0, _name = 0x0}, val7={_data = 0x0, _name = 0x0}, val8={_data = 0x0, _name = 0x0}, val9={_data = 0x0, _name = 0x0})
    at /home/nierob/dev/qt/src/corelib/kernel/qmetaobject.cpp:1533
#45 0xb49b1d51 in QMetaObject::invokeMethod (obj=0xbfda42f4, member=0x8d5db90 "createViewlessPlugin", type=Qt::DirectConnection, ret={<QGenericArgument> = {_data = 0x0, _name = 0x0}, <No data fields>}, val0={_data = 0x0, _name = 0x0}, val1={_data = 0x0, _name = 0x0},
    val2={_data = 0x0, _name = 0x0}, val3={_data = 0x0, _name = 0x0}, val4={_data = 0x0, _name = 0x0}, val5={_data = 0x0, _name = 0x0}, val6={_data = 0x0, _name = 0x0}, val7={_data = 0x0, _name = 0x0}, val8={_data = 0x0, _name = 0x0}, val9={_data = 0x0, _name = 0x0})
    at /home/nierob/dev/qt/src/corelib/kernel/qmetaobject.cpp:1113
#46 0xb58e8776 in QMetaObject::invokeMethod (obj=0xbfda42f4, member=0x8d5db90 "createViewlessPlugin", type=Qt::DirectConnection, val0={_data = 0x0, _name = 0x0}, val1={_data = 0x0, _name = 0x0}, val2={_data = 0x0, _name = 0x0}, val3={_data = 0x0, _name = 0x0}, val4=
      {_data = 0x0, _name = 0x0}, val5={_data = 0x0, _name = 0x0}, val6={_data = 0x0, _name = 0x0}, val7={_data = 0x0, _name = 0x0}, val8={_data = 0x0, _name = 0x0}, val9={_data = 0x0, _name = 0x0}) at ../../include/QtCore/../../src/corelib/kernel/qobjectdefs.h:396
#47 0xb58e2f78 in qInvokeTestMethodDataEntry (slot=0x8d5db90 "createViewlessPlugin") at /home/nierob/dev/qt/src/testlib/qtestcase.cpp:1213
#48 0xb58e4452 in qInvokeTestMethod (slotName=0x8072329 "createViewlessPlugin()", data=0x0) at /home/nierob/dev/qt/src/testlib/qtestcase.cpp:1308
#49 0xb58e4be9 in qInvokeTestMethods (testObject=0xbfda42f4) at /home/nierob/dev/qt/src/testlib/qtestcase.cpp:1453
#50 0xb58e5cc3 in QTest::qExec (testObject=0xbfda42f4, argc=2, argv=0xbfda43b4) at /home/nierob/dev/qt/src/testlib/qtestcase.cpp:1666
#51 0x08052943 in main (argc=2, argv=0xbfda43b4) at /home/nierob/dev/webkit/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp:1422
(gdb)

[Jędrzej Nowacki]

Bug was introduced by patch for bug 29710. 
Apparently QWebPagePrivate::client (m_webFrame->page()->d->client) could be null.

[Jędrzej Nowacki]

Created attachment 40772 [details]
fix

Fix repair autotest crash and memory leak that was before patch for bug 29710.

[Jędrzej Nowacki]

Actually it is regression so I have changed to P1 and added keyword

[Simon Hausmann]

Comment on attachment 40772 [details]
fix

As discussed in the office, the current code doesn't _actually_ leak as the WebCore::Widget subclasses for QWidget and QGraphicsWidget delete the objects in their destructor.

However what is missing in the current code is a null pointer check if page->d->client is a null pointer.

[Jędrzej Nowacki]

Created attachment 40861 [details]
patch v2

Crash fix && Autotest

[Simon Hausmann]

Comment on attachment 40861 [details]
patch v2

> diff --git a/WebKit/qt/ChangeLog b/WebKit/qt/ChangeLog
> index dbaa5a2..bd956ed 100644
> --- a/WebKit/qt/ChangeLog
> +++ b/WebKit/qt/ChangeLog
> @@ -1,3 +1,28 @@
> [phone]  Jedrzej Nowacki  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        QWebPage's createViewlessPlugin autotest crash fix.
> +        
> +        A plug-ins returning widgets (QWidget or QGraphicsWidget) might be
> +        created even in a viewless applications. The plug-ins won't be fully

I suggest to start the first sentence with "It is possible that plugins that are QWidgets or QGraphicsWidgets
are created before a view has been assigned to a QWebPage".

> +        functional, as by design, they should visualise something, but they
> +        won't crash and will stay, work in memory.
> +        
> +        Autotest was developped to cover a viewless applications that create
> +        a plug-ins based on the QGraphicsWidget class.

Typo: developed with one p. How about "An autotest is included that covers
this use-case." ? :)



> -class PluginTrackedPage : public QWebPage
> +class PluginTrackedPageWidget : public QWebPage
>  {
>  public:
>  
>      int count;
>      QPointer<QWidget> widget;
>  
> -    PluginTrackedPage(QWidget *parent = 0) : QWebPage(parent), count(0) {
> +    PluginTrackedPageWidget(QWidget *parent = 0) : QWebPage(parent), count(0) {
>         settings()->setAttribute(QWebSettings::PluginsEnabled, true);
>      }
>  
> @@ -640,9 +641,28 @@ public:
>      }
>  };
>  
> +class PluginTrackedPageGraphicsWidget : public QWebPage
> +{
> +public:
> +
> +    int count;
> +    QPointer<QGraphicsWidget> widget;
> +
> +    PluginTrackedPageGraphicsWidget(QWidget *parent = 0) : QWebPage(parent), count(0) {
> +       settings()->setAttribute(QWebSettings::PluginsEnabled, true);
> +    }

Coding style: In function definitions place each brace on its own line.

> +    virtual QObject* createPlugin(const QString&, const QUrl&, const QStringList&, const QStringList&) {
> +       count++;
> +       QGraphicsWidget *w = new QGraphicsWidget;

Coding style: * placement

> +       widget = w;
> +       return w;

Why not simply write it in two lines? :)

    widget = new QGraphicsWidget;
    return widget;

[Jędrzej Nowacki]

Comment on attachment 40861 [details]
patch v2

Ok, I'll fix change log.

> Coding style
Punishment for copy & paste coding :-)

> Why not simply write it in two lines? :)
> 
>     widget = new QGraphicsWidget;
>     return widget;
In two lines? We can use only one :-)
return widget = new QGraphicsWidget;

Sorry for the errors, I should catch it before...

[Jędrzej Nowacki]

Created attachment 40872 [details]
patch v3

Next attempt.

[Jędrzej Nowacki]

Comment on attachment 40872 [details]
patch v3

Bad coding style.

[Jędrzej Nowacki]

Created attachment 41040 [details]
patch v4

Final version ?

[WebKit Commit Bot]

Comment on attachment 41040 [details]
patch v4

Clearing flags on attachment: 41040

Committed r49440: <http://trac.webkit.org/changeset/49440>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.