2009-11-23 Simon Hausmann <simon.hausmann@nokia.com>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Nov 2009 12:58:30 +0000 (12:58 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 23 Nov 2009 12:58:30 +0000 (12:58 +0000)
        Reviewed by Kenneth Rohde Christiansen.

        [Qt] Wrong runtime instance objects of wrapped QObjects may be used if
        the wrapped object died before the gc removed the instance.

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

        Before using a cached instance, verify that its wrapped QObject is
        still alive.

        * bridge/qt/qt_instance.cpp:
        (JSC::Bindings::QtInstance::getQtInstance):
        * bridge/qt/qt_instance.h:
        (JSC::Bindings::QtInstance::hashKey):
2009-11-23  Simon Hausmann  <simon.hausmann@nokia.com>

        Reviewed by Kenneth Rohde Christiansen.

        [Qt] Wrong runtime instance objects of wrapped QObjects may be used if
        the wrapped object died before the gc removed the instance.

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

        Added a unit-test to verify that wrapping a QObject with the
        same identity as a previously but now dead object works.

        * tests/qwebframe/tst_qwebframe.cpp:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@51306 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bridge/qt/qt_instance.cpp
WebCore/bridge/qt/qt_instance.h
WebKit/qt/ChangeLog
WebKit/qt/tests/qwebframe/tst_qwebframe.cpp

index 9acaadfb51ffc060ab6e6bb4d2630d8c427decd4..456078c1b4fec72c9f35f287a1adc13f989c2609 100644 (file)
@@ -1,3 +1,20 @@
+2009-11-23  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        [Qt] Wrong runtime instance objects of wrapped QObjects may be used if
+        the wrapped object died before the gc removed the instance.
+
+        https://bugs.webkit.org/show_bug.cgi?id=31681
+
+        Before using a cached instance, verify that its wrapped QObject is
+        still alive.
+
+        * bridge/qt/qt_instance.cpp:
+        (JSC::Bindings::QtInstance::getQtInstance):
+        * bridge/qt/qt_instance.h:
+        (JSC::Bindings::QtInstance::hashKey):
+
 2009-11-22  Chris Fleizach  <cfleizach@apple.com>
 
         Reviewed by Oliver Hunt.
index 26fd7011fbb2803a31092a19fff52553ece5ad6f..c6185e94cfceb74078947df95dd0532bf1018cb8 100644 (file)
@@ -119,10 +119,17 @@ PassRefPtr<QtInstance> QtInstance::getQtInstance(QObject* o, PassRefPtr<RootObje
 {
     JSLock lock(SilenceAssertionsOnly);
 
-    foreach(QtInstance* instance, cachedInstances.values(o)) {
-        if (instance->rootObject() == rootObject)
-            return instance;
-    }
+    foreach(QtInstance* instance, cachedInstances.values(o))
+        if (instance->rootObject() == rootObject) {
+            // The garbage collector removes instances, but it may happen that the wrapped
+            // QObject dies before the gc kicks in. To handle that case we have to do an additional
+            // check if to see if the instance's wrapped object is still alive. If it isn't, then
+            // we have to create a new wrapper.
+            if (!instance->getObject())
+                cachedInstances.remove(instance->hashKey());
+            else
+                return instance;
+        }
 
     RefPtr<QtInstance> ret = QtInstance::create(o, rootObject, ownership);
     cachedInstances.insert(o, ret.get());
index 00aaa5bfcee71f22a80d50a9c0983971a43a7dd0..0afc6c754f7bcd8ae90a2f25661e11d3ae8387ab 100644 (file)
@@ -59,6 +59,7 @@ public:
     JSValue booleanValue() const;
 
     QObject* getObject() const { return m_object; }
+    QObject* hashKey() const { return m_hashkey; }
 
     static PassRefPtr<QtInstance> getQtInstance(QObject*, PassRefPtr<RootObject>, QScriptEngine::ValueOwnership ownership);
 
index 7ed361b80819aec9a76aa429d96600bbaa3fd818..d28e9f9e72f97d15289629735dc4c21ce8faff0d 100644 (file)
@@ -1,3 +1,17 @@
+2009-11-23  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        [Qt] Wrong runtime instance objects of wrapped QObjects may be used if
+        the wrapped object died before the gc removed the instance.
+
+        https://bugs.webkit.org/show_bug.cgi?id=31681
+
+        Added a unit-test to verify that wrapping a QObject with the
+        same identity as a previously but now dead object works.
+
+        * tests/qwebframe/tst_qwebframe.cpp:
+
 2009-11-19  Jocelyn Turcotte  <jocelyn.turcotte@nokia.com>
 
         Reviewed by Kenneth Rohde Christiansen.
index f6f23025fc60d2c4c7d5edf78f8a710b105662ee..cb35bc1daa53498b88a967c774828cc37b463c06 100644 (file)
@@ -604,6 +604,7 @@ private slots:
     void render();
     void scrollPosition();
     void evaluateWillCauseRepaint();
+    void qObjectWrapperWithSameIdentity();
 
 private:
     QString  evalJS(const QString&s) {
@@ -2758,6 +2759,43 @@ void tst_QWebFrame::evaluateWillCauseRepaint()
     QTest::qWait(2000);
 }
 
+class TestFactory : public QObject
+{
+    Q_OBJECT
+public:
+    TestFactory()
+        : obj(0), counter(0)
+    {}
+
+    Q_INVOKABLE QObject* getNewObject()
+    {
+        delete obj;
+        obj = new QObject(this);
+        obj->setObjectName(QLatin1String("test") + QString::number(++counter));
+        return obj;
+
+    }
+
+    QObject* obj;
+    int counter;
+};
+
+void tst_QWebFrame::qObjectWrapperWithSameIdentity()
+{
+    m_view->setHtml("<script>function triggerBug() { document.getElementById('span1').innerText = test.getNewObject().objectName; }</script>"
+                    "<body><span id='span1'>test</span></body>");
+
+    QWebFrame* mainFrame = m_view->page()->mainFrame();
+    QCOMPARE(mainFrame->toPlainText(), QString("test"));
+
+    mainFrame->addToJavaScriptWindowObject("test", new TestFactory, QScriptEngine::ScriptOwnership);
+
+    mainFrame->evaluateJavaScript("triggerBug();");
+    QCOMPARE(mainFrame->toPlainText(), QString("test1"));
+
+    mainFrame->evaluateJavaScript("triggerBug();");
+    QCOMPARE(mainFrame->toPlainText(), QString("test2"));
+}
 
 QTEST_MAIN(tst_QWebFrame)
 #include "tst_qwebframe.moc"