Changeset 49440 in webkit

Timestamp:

Oct 12, 2009 8:05:59 AM (15 years ago)

Author:

eric@webkit.org

Message:

2009-10-12 Jedrzej Nowacki <​jedrzej.nowacki@nokia.com>

Reviewed by Simon Hausmann.

QWebPage's createViewlessPlugin autotest crash fix.

It is possible that plugins that are QWidgets or QGraphicsWidgets
are created before a view has been assigned to a QWebPage. The
plug-ins won't be fully functional, as by design, they should
visualise something, but they won't crash and will stay in memory.

An autotest that covers this use-case, is included.

​https://bugs.webkit.org/show_bug.cgi?id=30118

• WebCoreSupport/FrameLoaderClientQt.cpp: (WebCore::FrameLoaderClientQt::createPlugin):
• tests/qwebpage/tst_qwebpage.cpp: (PluginTrackedPageWidget::PluginTrackedPageWidget): (PluginTrackedPageGraphicsWidget::PluginTrackedPageGraphicsWidget): (PluginTrackedPageGraphicsWidget::createPlugin): (tst_QWebPage::destroyPlugin): (tst_QWebPage::createViewlessPlugin):

Location:

trunk/WebKit/qt

Files:

• ChangeLog (modified) (1 diff)
• WebCoreSupport/FrameLoaderClientQt.cpp (modified) (2 diffs)
• tests/qwebpage/tst_qwebpage.cpp (modified) (4 diffs)

trunk/WebKit/qt/ChangeLog

@@ +1 @@
+2009-10-12  Jedrzej Nowacki  <jedrzej.nowacki@nokia.com>
+
+        Reviewed by Simon Hausmann.
+
+        QWebPage's createViewlessPlugin autotest crash fix.
+        
+        It is possible that plugins that are QWidgets or QGraphicsWidgets
+        are created before a view has been assigned to a QWebPage. The
+        plug-ins won't be fully functional, as by design, they should
+        visualise something, but they won't crash and will stay in memory.
+
+        An autotest that covers this use-case, is included.
+
+        https://bugs.webkit.org/show_bug.cgi?id=30118
+
+        * WebCoreSupport/FrameLoaderClientQt.cpp:
+        (WebCore::FrameLoaderClientQt::createPlugin):
+        * tests/qwebpage/tst_qwebpage.cpp:
+        (PluginTrackedPageWidget::PluginTrackedPageWidget):
+        (PluginTrackedPageGraphicsWidget::PluginTrackedPageGraphicsWidget):
+        (PluginTrackedPageGraphicsWidget::createPlugin):
+        (tst_QWebPage::destroyPlugin):
+        (tst_QWebPage::createViewlessPlugin):
+
 2009-10-09  Joe Ligman  <joseph.ligman@nokia.com>
 

trunk/WebKit/qt/WebCoreSupport/FrameLoaderClientQt.cpp

@@ -1231 +1231 @@
             QWidget* widget = qobject_cast<QWidget*>(object);
             if (widget) {
-                QWidget* parentWidget = qobject_cast<QWidget*>(m_webFrame->page()->d->client->pluginParent());
-                if (parentWidget)
-                    widget->setParent(parentWidget);
+                QWidget* parentWidget;
+                if (m_webFrame->page()->d->client)
+                    parentWidget = qobject_cast<QWidget*>(m_webFrame->page()->d->client->pluginParent());
+                else
+                    parentWidget = 0;  // The plug-in won't be fully functional because the QWebView doesn't exist.
+                widget->setParent(parentWidget);
                 RefPtr<QtPluginWidget> w = adoptRef(new QtPluginWidget());
                 w->setPlatformWidget(widget);
@@ -1243 +1246 @@
             QGraphicsWidget* graphicsWidget = qobject_cast<QGraphicsWidget*>(object);
             if (graphicsWidget) {
+                QGraphicsObject* parentWidget;
+                if (m_webFrame->page()->d->client)
+                    parentWidget = qobject_cast<QGraphicsObject*>(m_webFrame->page()->d->client->pluginParent());
+                else
+                    parentWidget = 0;  // The plug-in won't be fully functional because the QWebView doesn't exist.
                 graphicsWidget->hide();
-                graphicsWidget->setParentItem(qobject_cast<QGraphicsObject*>(m_webFrame->page()->d->client->pluginParent()));
+                graphicsWidget->setParentItem(parentWidget);
                 RefPtr<QtPluginGraphicsWidget> w = QtPluginGraphicsWidget::create(graphicsWidget);
                 // Make sure it's invisible until properly placed into the layout

trunk/WebKit/qt/tests/qwebpage/tst_qwebpage.cpp

@@ -24 +24 @@
 #include <qwebpage.h>
 #include <qwidget.h>
+#include <QGraphicsWidget>
 #include <qwebview.h>
 #include <qwebframe.h>
@@ -102 +103 @@
     void database();
     void createPlugin();
+    void destroyPlugin_data();
     void destroyPlugin();
+    void createViewlessPlugin_data();
     void createViewlessPlugin();
     void multiplePageGroupsAndLocalStorage();
@@ -622 +625 @@
 }
 
-class PluginTrackedPage : public QWebPage
-{
+
+// Standard base class for template PluginTracerPage. In tests it is used as interface.
+class PluginCounterPage : public QWebPage {
 public:
-
-    int count;
-    QPointer<QWidget> widget;
-
-    PluginTrackedPage(QWidget *parent = 0) : QWebPage(parent), count(0) {
+    int m_count;
+    QPointer<QObject> m_widget;
+    PluginCounterPage(QObject* parent = 0) : QWebPage(parent), m_count(0), m_widget(0)
+    {
        settings()->setAttribute(QWebSettings::PluginsEnabled, true);
     }
-
-    virtual QObject* createPlugin(const QString&, const QUrl&, const QStringList&, const QStringList&) {
-       count++;
-       QWidget *w = new QWidget;
-       widget = w;
-       return w;
-    }
 };
 
+template<class T>
+class PluginTracerPage : public PluginCounterPage {
+public:
+    PluginTracerPage(QObject* parent = 0) : PluginCounterPage(parent) {}
+    virtual QObject* createPlugin(const QString&, const QUrl&, const QStringList&, const QStringList&)
+    {
+        m_count++;
+        return m_widget = new T();
+    }
+};
+
+class PluginFactory {
+public:
+    enum FactoredType {QWidgetType, QGraphicsWidgetType};
+    static PluginCounterPage* create(FactoredType type, QObject* parent = 0)
+    {
+        PluginCounterPage* result = 0;
+        switch (type) {
+        case QWidgetType:
+            result = new PluginTracerPage<QWidget>(parent);
+            break;
+        case QGraphicsWidgetType:
+            result = new PluginTracerPage<QGraphicsWidget>(parent);
+            break;
+        default: {/*Oops*/};
+        }
+        return result;
+    }
+
+    static void prepareTestData()
+    {
+        QTest::addColumn<int>("type");
+        QTest::newRow("QWidget") << (int)PluginFactory::QWidgetType;
+        QTest::newRow("QGraphicsWidget") << (int)PluginFactory::QGraphicsWidgetType;
+    }
+};
+
+void tst_QWebPage::destroyPlugin_data()
+{
+    PluginFactory::prepareTestData();
+}
+
 void tst_QWebPage::destroyPlugin()
 {
-    PluginTrackedPage* page = new PluginTrackedPage(m_view);
+    QFETCH(int, type);
+    PluginCounterPage* page = PluginFactory::create((PluginFactory::FactoredType)type, m_view);
     m_view->setPage(page);
 
@@ -649 +688 @@
     QString content("<html><body><object type=\"application/x-qt-plugin\" classid=\"QProgressBar\"></object></body></html>");
     m_view->setHtml(content);
-    QVERIFY(page->widget != 0);
-    QCOMPARE(page->count, 1);
+    QVERIFY(page->m_widget);
+    QCOMPARE(page->m_count, 1);
 
     // navigate away, the plugin widget should be destructed
     m_view->setHtml("<html><body>Hi</body></html>");
     QTestEventLoop::instance().enterLoop(1);
-    QVERIFY(page->widget == 0);
+    QVERIFY(!page->m_widget);
+}
+
+void tst_QWebPage::createViewlessPlugin_data()
+{
+    PluginFactory::prepareTestData();
 }
 
 void tst_QWebPage::createViewlessPlugin()
 {
-    PluginTrackedPage* page = new PluginTrackedPage;
+    QFETCH(int, type);
+    PluginCounterPage* page = PluginFactory::create((PluginFactory::FactoredType)type);
     QString content("<html><body><object type=\"application/x-qt-plugin\" classid=\"QProgressBar\"></object></body></html>");
     page->mainFrame()->setHtml(content);
-    QCOMPARE(page->count, 1);
-    QVERIFY(page->widget != 0);
+    QCOMPARE(page->m_count, 1);
+    QVERIFY(page->m_widget);
     delete page;
+
 }