From fd01eb1797221fbcc983ed5d4ff644cecdba5e42 Mon Sep 17 00:00:00 2001
From: Simon Hausmann <simon.hausmann@digia.com>
Date: Wed, 5 Jun 2013 09:59:18 +0200
Subject: [PATCH] Improve type error messages when reading properties

This fixes the expected output of the qobjectConnectionListExceptionHandling and
while we're at it this patch also re-enables the importScripts test. The test
expects similar output that now passes. importScripts_data produces two failures
though, that are unrelated to expected error message but indicate real bugs.

Change-Id: I606b8791524d19a4bb20a81c30abc2285a1a0a0f
Reviewed-by: Lars Knoll <lars.knoll@digia.com>
---
 src/qml/qml/v4/qv4runtime.cpp                        | 10 ++++++++++
 tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 19 +++++++------------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/qml/qml/v4/qv4runtime.cpp b/src/qml/qml/v4/qv4runtime.cpp
index 47a41bd..b013f53 100644
--- a/src/qml/qml/v4/qv4runtime.cpp
+++ b/src/qml/qml/v4/qv4runtime.cpp
@@ -567,6 +567,11 @@ void __qmljs_get_element(ExecutionContext *ctx, Value *result, const Value &obje
             }
         }
 
+        if (object.isNull() || object.isUndefined()) {
+            QString message = QStringLiteral("Cannot read property '%1' of %2").arg(index.toQString()).arg(object.toQString());
+            ctx->throwTypeError(message);
+        }
+
         o = __qmljs_convert_to_object(ctx, object);
     }
 
@@ -666,6 +671,11 @@ void __qmljs_get_property(ExecutionContext *ctx, Value *result, const Value &obj
     if (m) {
         res = m->get(ctx, name);
     } else {
+        if (object.isNull() || object.isUndefined()) {
+            QString message = QStringLiteral("Cannot read property '%1' of %2").arg(name->toQString()).arg(object.toQString());
+            ctx->throwTypeError(message);
+        }
+
         m = __qmljs_convert_to_object(ctx, object);
         res = m->get(ctx, name);
     }
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 14dc6d9..9362136 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -50,6 +50,7 @@
 #include <QtCore/qnumeric.h>
 #include <private/qqmlengine_p.h>
 #include <private/qqmlvmemetaobject_p.h>
+#include <private/qqmlcontextwrapper_p.h>
 #include "testtypes.h"
 #include "testhttpserver.h"
 #include "../../shared/util.h"
@@ -157,10 +158,8 @@ private slots:
     void singletonTypeCaching();
     void singletonTypeImportOrder();
     void singletonTypeResolution();
-#if 0
     void importScripts_data();
     void importScripts();
-#endif
     void scarceResources();
     void scarceResources_data();
     void scarceResources_other();
@@ -295,7 +294,7 @@ private slots:
 
 private:
 //    static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
-//    static void verifyContextLifetime(QQmlContextData *ctxt);
+    static void verifyContextLifetime(QQmlContextData *ctxt);
     QQmlEngine engine;
 };
 
@@ -3778,27 +3777,26 @@ void tst_qqmlecmascript::singletonTypeResolution()
     delete object;
 }
 
-#if 0
 void tst_qqmlecmascript::verifyContextLifetime(QQmlContextData *ctxt) {
     QQmlContextData *childCtxt = ctxt->childContexts;
 
     if (!ctxt->importedScripts.isEmpty()) {
         QV8Engine *engine = QV8Engine::get(ctxt->engine);
-        foreach (v8::Persistent<v8::Object> qmlglobal, ctxt->importedScripts) {
+        foreach (const QV4::PersistentValue& qmlglobal, ctxt->importedScripts) {
             QQmlContextData *scriptContext, *newContext;
 
-            if (qmlglobal.IsEmpty())
+            if (qmlglobal.isEmpty())
                 continue;
 
-            scriptContext = engine->contextWrapper()->context(qmlglobal);
+            scriptContext = QV4::QmlContextWrapper::getContext(qmlglobal);
 
             {
-                v8::Handle<v8::Object> temporaryScope = engine->qmlScope(scriptContext, NULL);
+                QV4::Value temporaryScope = QV4::QmlContextWrapper::qmlScope(engine, scriptContext, 0);
                 Q_UNUSED(temporaryScope)
             }
 
             engine->gc();
-            newContext = engine->contextWrapper()->context(qmlglobal);
+            newContext = QV4::QmlContextWrapper::getContext(qmlglobal);
             QVERIFY(scriptContext == newContext);
         }
     }
@@ -3809,9 +3807,7 @@ void tst_qqmlecmascript::verifyContextLifetime(QQmlContextData *ctxt) {
         childCtxt = childCtxt->nextChild;
     }
 }
-#endif
 
-#if 0
 void tst_qqmlecmascript::importScripts_data()
 {
     QTest::addColumn<QUrl>("testfile");
@@ -4054,7 +4050,6 @@ void tst_qqmlecmascript::importScripts()
 
     engine.setImportPathList(importPathList);
 }
-#endif
 
 void tst_qqmlecmascript::scarceResources_other()
 {
-- 
2.7.4