Allow .pragma library scripts to import other scripts

Previously, a .pragma library script would have a new context which
did not have an engine set.  If the script then imported other scripts
a crash would occur due to dereferencing the (null) engine ptr.

This commit ensures that even if no parent context is used (eg, for
shared scripts which don't import the parent context) the engine from
the parent context is used as the engine in the new context.

Finally, unit tests for the .pragma library import with imports cases
were added to tst_qdeclarativeecmascript.

Task-number: QTBUG-21620
Change-Id: I671ffc9eee98a69cce7c169ce5b9d5aae4d1ff0d
Reviewed-on: http://codereview.qt-project.org/5421
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

diff --git a/src/declarative/qml/qdeclarativevme.cpp b/src/declarative/qml/qdeclarativevme.cpp
index bf8bd29..8cc8fa0 100644 (file)
--- a/src/declarative/qml/qdeclarativevme.cpp
+++ b/src/declarative/qml/qdeclarativevme.cpp
@@ -1174,6 +1174,7 @@ v8::Persistent<v8::Object> QDeclarativeVME::run(QDeclarativeContextData *parentC
     if (script->m_loaded)
         return qPersistentNew<v8::Object>(script->m_value);
 
+    Q_ASSERT(parentCtxt && parentCtxt->engine);
     QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(parentCtxt->engine);
     QV8Engine *v8engine = ep->v8engine();
 
@@ -1208,8 +1209,11 @@ v8::Persistent<v8::Object> QDeclarativeVME::run(QDeclarativeContextData *parentC
         ctxt->imports->addref();
     }
 
-    if (effectiveCtxt)
+    if (effectiveCtxt) {
         ctxt->setParent(effectiveCtxt, true);
+    } else {
+        ctxt->engine = parentCtxt->engine; // Fix for QTBUG-21620
+    }
 
     for (int ii = 0; ii < script->scripts.count(); ++ii) {
         ctxt->importedScripts << run(ctxt, script->scripts.at(ii)->scriptData());

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithImports.js b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithImports.js
new file mode 100644 (file)
index 0000000..3f2e658
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithImports.js
@@ -0,0 +1,9 @@
+.pragma library
+.import "importFive.js" as ImportFive
+
+var i = 4;
+
+function importIncrementedValue() {
+    i = i + 1;
+    return (i + ImportFive.importFiveFunction()); // i + '5' (not i+5)
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithPragmaLibraryImports.js b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithPragmaLibraryImports.js
new file mode 100644 (file)
index 0000000..fa6497d
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/importPragmaLibraryWithPragmaLibraryImports.js
@@ -0,0 +1,11 @@
+.pragma library
+.import "importPragmaLibrary.js" as LibraryImport
+
+var i = 10;
+
+function importIncrementedValue() {
+    i = i + 1;
+    // because LibraryImport is shared, and used in previous tests,
+    // the value will be large (already incremented a bunch of times).
+    return (i + LibraryImport.importIncrementedValue());
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithImports.qml b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithImports.qml
new file mode 100644 (file)
index 0000000..6a7459d
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithImports.qml
@@ -0,0 +1,7 @@
+import QtQuick 2.0
+import "importPragmaLibraryWithImports.js" as LibraryImport
+
+QtObject {
+    id: root
+    property int testValue: LibraryImport.importIncrementedValue(); // valueOf(4 + 1 + '5') = valueOf('55') = 55
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithPragmaLibraryImports.qml b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithPragmaLibraryImports.qml
new file mode 100644 (file)
index 0000000..01f08db
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/jsimport/testImportPragmaLibraryWithPragmaLibraryImports.qml
@@ -0,0 +1,7 @@
+import QtQuick 2.0
+import "importPragmaLibraryWithPragmaLibraryImports.js" as LibraryImport
+
+QtObject {
+    id: root
+    property int testValue: LibraryImport.importIncrementedValue(); // 10 + 1 + (7 due to previous tests) = 18
+}

diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index c660054..5739ec9 100644 (file)
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -147,6 +147,7 @@ private slots:
     void signalWithJSValueInVariant_twoEngines();
     void moduleApi_data();
     void moduleApi();
+    void importScripts_data();
     void importScripts();
     void scarceResources();
     void propertyChangeSlots();
@@ -3014,78 +3015,125 @@ void tst_qdeclarativeecmascript::moduleApi()
     }
 }
 
-void tst_qdeclarativeecmascript::importScripts()
+void tst_qdeclarativeecmascript::importScripts_data()
 {
-    QObject *object = 0;
+    QTest::addColumn<QUrl>("testfile");
+    QTest::addColumn<QString>("errorMessage");
+    QTest::addColumn<QStringList>("warningMessages");
+    QTest::addColumn<QStringList>("propertyNames");
+    QTest::addColumn<QVariantList>("propertyValues");
 
-    // first, ensure that the required behaviour works.
-    QDeclarativeComponent component(&engine, TEST_FILE("jsimport/testImport.qml"));
-    object = component.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("importedScriptStringValue"), QVariant(QString(QLatin1String("Hello, World!"))));
-    QCOMPARE(object->property("importedScriptFunctionValue"), QVariant(20));
-    QCOMPARE(object->property("importedModuleAttachedPropertyValue"), QVariant(19));
-    QCOMPARE(object->property("importedModuleEnumValue"), QVariant(2));
-    delete object;
+    QTest::newRow("basic functionality")
+            << TEST_FILE("jsimport/testImport.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("importedScriptStringValue")
+                              << QLatin1String("importedScriptFunctionValue")
+                              << QLatin1String("importedModuleAttachedPropertyValue")
+                              << QLatin1String("importedModuleEnumValue"))
+            << (QVariantList() << QVariant(QLatin1String("Hello, World!"))
+                               << QVariant(20)
+                               << QVariant(19)
+                               << QVariant(2));
+
+    QTest::newRow("import scoping")
+            << TEST_FILE("jsimport/testImportScoping.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("componentError"))
+            << (QVariantList() << QVariant(5));
 
-    QDeclarativeComponent componentTwo(&engine, TEST_FILE("jsimport/testImportScoping.qml"));
-    object = componentTwo.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("componentError"), QVariant(5));
-    delete object;
+    QTest::newRow("parent scope shouldn't be inherited by import with imports")
+            << TEST_FILE("jsimportfail/failOne.qml")
+            << QString()
+            << (QStringList() << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/failOne.qml").toLocalFile() + QLatin1String(":6: TypeError: Cannot call method 'greetingString' of undefined")))
+            << (QStringList() << QLatin1String("importScriptFunctionValue"))
+            << (QVariantList() << QVariant(QString()));
 
-    // then, ensure that unintended behaviour does not work.
-    QDeclarativeComponent failOneComponent(&engine, TEST_FILE("jsimportfail/failOne.qml"));
-    QString expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/failOne.qml").toLocalFile() + QLatin1String(":6: TypeError: Cannot call method 'greetingString' of undefined");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    object = failOneComponent.create();
-    QVERIFY(object != 0);
-    QVERIFY(object->property("importScriptFunctionValue").toString().isEmpty());
-    delete object;
-    QDeclarativeComponent failTwoComponent(&engine, TEST_FILE("jsimportfail/failTwo.qml"));
-    expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/failTwo.qml").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: ImportOneJs");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    object = failTwoComponent.create();
-    QVERIFY(object != 0);
-    QVERIFY(object->property("importScriptFunctionValue").toString().isEmpty());
-    delete object;
-    QDeclarativeComponent failThreeComponent(&engine, TEST_FILE("jsimportfail/failThree.qml"));
-    expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/failThree.qml").toLocalFile() + QLatin1String(":7: TypeError: Cannot read property 'JsQtTest' of undefined");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    object = failThreeComponent.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("importedModuleAttachedPropertyValue"), QVariant(false));
-    delete object;
-    QDeclarativeComponent failFourComponent(&engine, TEST_FILE("jsimportfail/failFour.qml"));
-    expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/failFour.qml").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: JsQtTest");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    object = failFourComponent.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("importedModuleEnumValue"), QVariant(0));
-    delete object;
-    QDeclarativeComponent failFiveComponent(&engine, TEST_FILE("jsimportfail/failFive.qml"));
-    expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/importWithImports.js").toLocalFile() + QLatin1String(":8: ReferenceError: Can't find variable: Component");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    expectedWarning = QLatin1String("file://") + TEST_FILE("jsimportfail/importPragmaLibrary.js").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: Component");
-    QTest::ignoreMessage(QtWarningMsg, expectedWarning.toAscii().constData());
-    object = failFiveComponent.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("componentError"), QVariant(0));
-    delete object;
+    QTest::newRow("javascript imports in an import should be private to the import scope")
+            << TEST_FILE("jsimportfail/failTwo.qml")
+            << QString()
+            << (QStringList() << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/failTwo.qml").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: ImportOneJs")))
+            << (QStringList() << QLatin1String("importScriptFunctionValue"))
+            << (QVariantList() << QVariant(QString()));
 
-    // also, test that importing scripts with .pragma library works as required
-    QDeclarativeComponent pragmaLibraryComponent(&engine, TEST_FILE("jsimport/testImportPragmaLibrary.qml"));
-    object = pragmaLibraryComponent.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("testValue"), QVariant(31));
-    delete object;
+    QTest::newRow("module imports in an import should be private to the import scope")
+            << TEST_FILE("jsimportfail/failThree.qml")
+            << QString()
+            << (QStringList() << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/failThree.qml").toLocalFile() + QLatin1String(":7: TypeError: Cannot read property 'JsQtTest' of undefined")))
+            << (QStringList() << QLatin1String("importedModuleAttachedPropertyValue"))
+            << (QVariantList() << QVariant(false));
 
-    // and that .pragma library scripts don't inherit imports from any .qml file
-    QDeclarativeComponent pragmaLibraryComponentTwo(&engine, TEST_FILE("jsimportfail/testImportPragmaLibrary.qml"));
-    object = pragmaLibraryComponentTwo.create();
-    QVERIFY(object != 0);
-    QCOMPARE(object->property("testValue"), QVariant(0));
-    delete object;
+    QTest::newRow("typenames in an import should be private to the import scope")
+            << TEST_FILE("jsimportfail/failFour.qml")
+            << QString()
+            << (QStringList() << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/failFour.qml").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: JsQtTest")))
+            << (QStringList() << QLatin1String("importedModuleEnumValue"))
+            << (QVariantList() << QVariant(0));
+
+    QTest::newRow("import with imports has it's own activation scope")
+            << TEST_FILE("jsimportfail/failFive.qml")
+            << QString()
+            << (QStringList() << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/importWithImports.js").toLocalFile() + QLatin1String(":8: ReferenceError: Can't find variable: Component"))
+                              << QString(QLatin1String("file://") + TEST_FILE("jsimportfail/importPragmaLibrary.js").toLocalFile() + QLatin1String(":6: ReferenceError: Can't find variable: Component")))
+            << (QStringList() << QLatin1String("componentError"))
+            << (QVariantList() << QVariant(0));
+
+    QTest::newRow("import pragma library script")
+            << TEST_FILE("jsimport/testImportPragmaLibrary.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("testValue"))
+            << (QVariantList() << QVariant(31));
+
+    QTest::newRow("pragma library imports shouldn't inherit parent imports or scope")
+            << TEST_FILE("jsimportfail/testImportPragmaLibrary.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("testValue"))
+            << (QVariantList() << QVariant(0));
+
+    QTest::newRow("import pragma library script which has an import")
+            << TEST_FILE("jsimport/testImportPragmaLibraryWithImports.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("testValue"))
+            << (QVariantList() << QVariant(55));
+
+    QTest::newRow("import pragma library script which has a pragma library import")
+            << TEST_FILE("jsimport/testImportPragmaLibraryWithPragmaLibraryImports.qml")
+            << QString()
+            << QStringList()
+            << (QStringList() << QLatin1String("testValue"))
+            << (QVariantList() << QVariant(18));
+}
+
+void tst_qdeclarativeecmascript::importScripts()
+{
+    QFETCH(QUrl, testfile);
+    QFETCH(QString, errorMessage);
+    QFETCH(QStringList, warningMessages);
+    QFETCH(QStringList, propertyNames);
+    QFETCH(QVariantList, propertyValues);
+
+    QDeclarativeComponent component(&engine, testfile);
+
+    if (!errorMessage.isEmpty())
+        QTest::ignoreMessage(QtWarningMsg, errorMessage.toAscii().constData());
+
+    if (warningMessages.size())
+        foreach (const QString &warning, warningMessages)
+            QTest::ignoreMessage(QtWarningMsg, warning.toAscii().constData());
+
+    QObject *object = component.create();
+    if (!errorMessage.isEmpty()) {
+        QVERIFY(object == 0);
+    } else {
+        QVERIFY(object != 0);
+        for (int i = 0; i < propertyNames.size(); ++i)
+            QCOMPARE(object->property(propertyNames.at(i).toAscii().constData()), propertyValues.at(i));
+        delete object;
+    }
 }
 
 void tst_qdeclarativeecmascript::scarceResources()