Fix compilation of script strings

The right hand side of script string properties can be evaluated in entirely
dynamic scopes, due to QQmlExpressions' public API of allowing construction
from a QQmlScriptString and a variable scope/context. Nevertheless we should
compile these bindings at type compile time, as long as we make sure that the
compiled code doesn't try to do any compile time determined property lookups
and type resolution. This is implemented using a separate compilation pass
that ensures the disableAcceleratedLookups flag is set.

A few minor cleanups come with this patch:

 * Ensure that the property caches array is always symmetric to the list of
   compiled QML objects, as that allows the use of at() instead of value().
 * The code for creating a QML callable function object for a given run-time
   function is now centralized in a static function QmlBindingWrapper, used
   for script strings and bindings from custom parsers.

The provided unit test verifies the successful execution of the same script
string with two different scope objects.

Change-Id: Ica2cea46dd9e47263b4d494d922d3cc9664b08ae
Reviewed-by: Michael Brasser <michael.brasser@live.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

8 files changed, 117 insertions, 33 deletions

diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp
index ad1321b1e6..d79143fdc3 100644
--- a/src/qml/compiler/qqmltypecompiler.cpp
+++ b/src/qml/compiler/qqmltypecompiler.cpp
@@ -203,6 +203,13 @@ bool QQmlTypeCompiler::compile()
 
     // Compile JS binding expressions and signal handlers
     if (!document->javaScriptCompilationUnit) {
+        {
+            // We can compile script strings ahead of time, but they must be compiled
+            // without type optimizations as their scope is always entirely dynamic.
+            QQmlScriptStringScanner sss(this);
+            sss.scan();
+        }
+
         QmlIR::JSCodeGen v4CodeGenerator(typeData->finalUrlString(), document->code, &document->jsModule, &document->jsParserEngine, document->program, compiledData->importCache, &document->jsGenerator.stringTable);
         QQmlJSCodeGenerator jsCodeGen(this, &v4CodeGenerator);
         if (!jsCodeGen.generateCodeForComponents())
@@ -278,6 +285,8 @@ bool QQmlTypeCompiler::compile()
     compiledData->totalParserStatusCount = parserStatusCount;
     compiledData->totalObjectCount = objectCount;
 
+    Q_ASSERT(compiledData->propertyCaches.count() == static_cast<int>(compiledData->qmlUnit->nObjects));
+
     return errors.isEmpty();
 }
 
@@ -330,9 +339,10 @@ int QQmlTypeCompiler::rootObjectIndex() const
 
 void QQmlTypeCompiler::setPropertyCaches(const QVector<QQmlPropertyCache *> &caches)
 {
-    Q_ASSERT(compiledData->propertyCaches.isEmpty());
     compiledData->propertyCaches = caches;
     Q_ASSERT(caches.count() >= document->indexOfRootObject);
+    if (compiledData->rootPropertyCache)
+        compiledData->rootPropertyCache->release();
     compiledData->rootPropertyCache = caches.at(document->indexOfRootObject);
     compiledData->rootPropertyCache->addref();
 }
@@ -1255,6 +1265,42 @@ void QQmlAliasAnnotator::annotateBindingsToAliases()
     }
 }
 
+QQmlScriptStringScanner::QQmlScriptStringScanner(QQmlTypeCompiler *typeCompiler)
+    : QQmlCompilePass(typeCompiler)
+    , qmlObjects(*typeCompiler->qmlObjects())
+    , propertyCaches(typeCompiler->propertyCaches())
+{
+
+}
+
+void QQmlScriptStringScanner::scan()
+{
+    const int scriptStringMetaType = qMetaTypeId<QQmlScriptString>();
+    for (int i = 0; i < qmlObjects.count(); ++i) {
+        QQmlPropertyCache *propertyCache = propertyCaches.at(i);
+        if (!propertyCache)
+            continue;
+
+        const QmlIR::Object *obj = qmlObjects.at(i);
+
+        QmlIR::PropertyResolver resolver(propertyCache);
+        QQmlPropertyData *defaultProperty = obj->indexOfDefaultProperty != -1 ? propertyCache->parent()->defaultProperty() : propertyCache->defaultProperty();
+
+        for (QmlIR::Binding *binding = obj->firstBinding(); binding; binding = binding->next) {
+            if (binding->type != QV4::CompiledData::Binding::Type_Script)
+                continue;
+            bool notInRevision = false;
+            QQmlPropertyData *pd = binding->propertyNameIndex != 0 ? resolver.property(stringAt(binding->propertyNameIndex), &notInRevision) : defaultProperty;
+            if (!pd || pd->propType != scriptStringMetaType)
+                continue;
+
+            QmlIR::CompiledFunctionOrExpression *foe = obj->functionsAndExpressions->slowAt(binding->value.compiledScriptIndex);
+            if (foe)
+                foe->disableAcceleratedLookups = true;
+        }
+    }
+}
+
 QQmlComponentAndAliasResolver::QQmlComponentAndAliasResolver(QQmlTypeCompiler *typeCompiler)
     : QQmlCompilePass(typeCompiler)
     , enginePrivate(typeCompiler->enginePrivate())
@@ -1331,6 +1377,10 @@ void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(const QmlI
 
         qmlObjects->append(syntheticComponent);
         const int componentIndex = qmlObjects->count() - 1;
+        // Keep property caches symmetric
+        QQmlPropertyCache *componentCache = enginePrivate->cache(&QQmlComponent::staticMetaObject);
+        componentCache->addref();
+        propertyCaches.append(componentCache);
 
         QmlIR::Binding *syntheticBinding = pool->New<QmlIR::Binding>();
         *syntheticBinding = *binding;
@@ -1355,7 +1405,7 @@ bool QQmlComponentAndAliasResolver::resolve()
     const int objCountWithoutSynthesizedComponents = qmlObjects->count();
     for (int i = 0; i < objCountWithoutSynthesizedComponents; ++i) {
         const QmlIR::Object *obj = qmlObjects->at(i);
-        QQmlPropertyCache *cache = propertyCaches.value(i);
+        QQmlPropertyCache *cache = propertyCaches.at(i);
         if (obj->inheritedTypeNameIndex == 0 && !cache)
             continue;
 
@@ -1427,6 +1477,10 @@ bool QQmlComponentAndAliasResolver::resolve()
 
     resolveAliases();
 
+    // Implicit component insertion may have added objects and thus we also need
+    // to extend the symmetric propertyCaches.
+    compiler->setPropertyCaches(propertyCaches);
+
     return true;
 }
 
@@ -1472,7 +1526,7 @@ bool QQmlComponentAndAliasResolver::resolveAliases()
     foreach (int objectIndex, _objectsWithAliases) {
         const QmlIR::Object *obj = qmlObjects->at(objectIndex);
 
-        QQmlPropertyCache *propertyCache = propertyCaches.value(objectIndex);
+        QQmlPropertyCache *propertyCache = propertyCaches.at(objectIndex);
         Q_ASSERT(propertyCache);
 
         int effectiveSignalIndex = propertyCache->signalHandlerIndexCacheStart + propertyCache->propertyIndexCache.count();
@@ -1528,7 +1582,7 @@ bool QQmlComponentAndAliasResolver::resolveAliases()
                 flags |= QML_ALIAS_FLAG_PTR;
                 propertyFlags |= QQmlPropertyData::IsQObjectDerived;
             } else {
-                QQmlPropertyCache *targetCache = propertyCaches.value(targetObjectIndex);
+                QQmlPropertyCache *targetCache = propertyCaches.at(targetObjectIndex);
                 Q_ASSERT(targetCache);
                 QmlIR::PropertyResolver resolver(targetCache);
 
@@ -2249,7 +2303,7 @@ bool QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData *property, co
     } else if (property->isQList()) {
         const int listType = enginePrivate->listType(property->propType);
         if (!QQmlMetaType::isInterface(listType)) {
-            QQmlPropertyCache *source = propertyCaches.value(binding->value.objectIndex);
+            QQmlPropertyCache *source = propertyCaches.at(binding->value.objectIndex);
             if (!canCoerce(listType, source)) {
                 recordError(binding->valueLocation, tr("Cannot assign object to list"));
                 return false;
@@ -2274,7 +2328,7 @@ bool QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData *property, co
         bool isAssignable = false;
         // Determine isAssignable value
         if (propertyMetaObject) {
-            QQmlPropertyCache *c = propertyCaches.value(binding->value.objectIndex);
+            QQmlPropertyCache *c = propertyCaches.at(binding->value.objectIndex);
             while (c && !isAssignable) {
                 isAssignable |= c == propertyMetaObject;
                 c = c->parent();
diff --git a/src/qml/compiler/qqmltypecompiler_p.h b/src/qml/compiler/qqmltypecompiler_p.h
index d54bc1b37b..3b449bf9bd 100644
--- a/src/qml/compiler/qqmltypecompiler_p.h
+++ b/src/qml/compiler/qqmltypecompiler_p.h
@@ -207,6 +207,18 @@ private:
     const QVector<QQmlPropertyCache *> propertyCaches;
 };
 
+class QQmlScriptStringScanner : public QQmlCompilePass
+{
+public:
+    QQmlScriptStringScanner(QQmlTypeCompiler *typeCompiler);
+
+    void scan();
+
+private:
+    const QList<QmlIR::Object*> &qmlObjects;
+    const QVector<QQmlPropertyCache *> propertyCaches;
+};
+
 class QQmlComponentAndAliasResolver : public QQmlCompilePass
 {
     Q_DECLARE_TR_FUNCTIONS(QQmlAnonymousComponentResolver)
@@ -238,7 +250,7 @@ protected:
     QList<int> _objectsWithAliases;
 
     QHash<int, QQmlCompiledData::TypeReference*> *resolvedTypes;
-    const QVector<QQmlPropertyCache *> propertyCaches;
+    QVector<QQmlPropertyCache *> propertyCaches;
     QVector<QByteArray> *vmeMetaObjectData;
     QHash<int, int> *objectIndexToIdForRoot;
     QHash<int, QHash<int, int> > *objectIndexToIdPerComponent;
diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp
index 15298017c7..e489b99ec5 100644
--- a/src/qml/jsruntime/qv4script.cpp
+++ b/src/qml/jsruntime/qv4script.cpp
@@ -54,6 +54,7 @@
 #include <private/qqmlengine_p.h>
 #include <qv4jsir_p.h>
 #include <qv4codegen_p.h>
+#include <qml/qqmlcontextwrapper_p.h>
 
 #include <QtCore/QDebug>
 #include <QtCore/QString>
@@ -69,8 +70,9 @@ QmlBindingWrapper::QmlBindingWrapper(ExecutionContext *scope, Function *f, Objec
 
     setVTable(staticVTable());
     function = f;
-    function->compilationUnit->ref();
-    needsActivation = function->needsActivation();
+    if (function)
+        function->compilationUnit->ref();
+    needsActivation = function ? function->needsActivation() : false;
 
     Scope s(scope);
     ScopedValue protectThis(s, this);
@@ -108,7 +110,8 @@ ReturnedValue QmlBindingWrapper::call(Managed *that, CallData *)
 
     Scope scope(engine);
     QmlBindingWrapper *This = static_cast<QmlBindingWrapper *>(that);
-    Q_ASSERT(This->function);
+    if (!This->function)
+        return QV4::Encode::undefined();
 
     CallContext *ctx = This->qmlContext;
     std::fill(ctx->locals, ctx->locals + ctx->function->varCount(), Primitive::undefinedValue());
@@ -129,6 +132,15 @@ void QmlBindingWrapper::markObjects(Managed *m, ExecutionEngine *e)
         wrapper->qmlContext->mark(e);
 }
 
+Returned<FunctionObject> *QmlBindingWrapper::createQmlCallableForFunction(ExecutionEngine *engine, QQmlContextData *qmlContext, QObject *scopeObject, Function *runtimeFunction)
+{
+    QV4::Scope valueScope(engine);
+    QV4::ScopedObject qmlScopeObject(valueScope, QV4::QmlContextWrapper::qmlScope(engine->v8Engine, qmlContext, scopeObject));
+    QV4::Scoped<QV4::QmlBindingWrapper> wrapper(valueScope, new (engine->memoryManager) QV4::QmlBindingWrapper(engine->rootContext, qmlScopeObject));
+    QV4::ScopedFunctionObject function(valueScope, QV4::FunctionObject::createScriptFunction(wrapper->context(), runtimeFunction));
+    return function->asReturned<FunctionObject>();
+}
+
 DEFINE_OBJECT_VTABLE(QmlBindingWrapper);
 
 struct CompilationUnitHolder : public QV4::Object
diff --git a/src/qml/jsruntime/qv4script_p.h b/src/qml/jsruntime/qv4script_p.h
index ec06565ddc..0165778f4b 100644
--- a/src/qml/jsruntime/qv4script_p.h
+++ b/src/qml/jsruntime/qv4script_p.h
@@ -49,6 +49,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QQmlContextData;
+
 namespace QV4 {
 
 struct ExecutionContext;
@@ -65,6 +67,8 @@ struct QmlBindingWrapper : FunctionObject {
 
     CallContext *context() const { return qmlContext; }
 
+    static Returned<FunctionObject> *createQmlCallableForFunction(QV4::ExecutionEngine *engine, QQmlContextData *qmlContext, QObject *scopeObject, QV4::Function *runtimeFunction);
+
 private:
     Object *qml;
     CallContext *qmlContext;
diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index 71c0f0b709..e9c48baa86 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -88,11 +88,8 @@ QQmlBinding::createBinding(Identifier id, QObject *obj, QQmlContext *ctxt)
         if (QQmlCompiledData *cdata = typeData->compiledData()) {
             QV4::ExecutionEngine *v4 = engine->v4engine();
             QV4::Scope valueScope(v4);
-            QV4::ScopedObject scopeObject(valueScope, QV4::QmlContextWrapper::qmlScope(v4->v8Engine, ctxtdata, obj));
-            QV4::Scoped<QV4::QmlBindingWrapper> wrapper(valueScope, new (v4->memoryManager) QV4::QmlBindingWrapper(v4->rootContext, scopeObject));
-            QV4::ExecutionContext *qmlContext = wrapper->context();
             QV4::Function *runtimeFunction = cdata->compilationUnit->runtimeFunctions[cdata->customParserBindings[id]];
-            QV4::ScopedValue function(valueScope, QV4::FunctionObject::createScriptFunction(qmlContext, runtimeFunction));
+            QV4::ScopedValue function(valueScope, QV4::QmlBindingWrapper::createQmlCallableForFunction(v4, ctxtdata, obj, runtimeFunction));
             rv = new QQmlBinding(function, obj, ctxtdata);
         }
 
@@ -128,7 +125,7 @@ QQmlBinding::QQmlBinding(const QQmlScriptString &script, QObject *obj, QQmlConte
         return;
 
     QString url;
-    QString code;
+    QV4::Function *runtimeFunction = 0;
 
     QQmlContextData *ctxtdata = QQmlContextData::get(scriptPrivate->context);
     QQmlEnginePrivate *engine = QQmlEnginePrivate::get(scriptPrivate->context->engine());
@@ -138,17 +135,23 @@ QQmlBinding::QQmlBinding(const QQmlScriptString &script, QObject *obj, QQmlConte
 
         if (QQmlCompiledData *cdata = typeData->compiledData()) {
             url = cdata->name;
+            if (scriptPrivate->bindingId != QQmlBinding::Invalid)
+                runtimeFunction = cdata->compilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
         }
 
         typeData->release();
     }
-    code = scriptPrivate->script;
 
     setNotifyOnValueChanged(true);
     QQmlAbstractExpression::setContext(QQmlContextData::get(ctxt ? ctxt : scriptPrivate->context));
     setScopeObject(obj ? obj : scriptPrivate->scope);
 
-    v4function = qmlBinding(context(), scopeObject(), code, url, scriptPrivate->lineNumber);
+    if (runtimeFunction) {
+        v4function = QV4::QmlBindingWrapper::createQmlCallableForFunction(ctxtdata, scopeObject(), runtimeFunction);
+    } else {
+        QString code = scriptPrivate->script;
+        v4function = qmlBinding(context(), scopeObject(), code, url, scriptPrivate->lineNumber);
+    }
 }
 
 QQmlBinding::QQmlBinding(const QString &str, QObject *obj, QQmlContextData *ctxt)
diff --git a/src/qml/qml/qqmlexpression.cpp b/src/qml/qml/qqmlexpression.cpp
index 6034d3fdea..e3e5dff4d8 100644
--- a/src/qml/qml/qqmlexpression.cpp
+++ b/src/qml/qml/qqmlexpression.cpp
@@ -78,17 +78,10 @@ void QQmlExpressionPrivate::init(QQmlContextData *ctxt, const QString &expr, QOb
     expressionFunctionValid = false;
 }
 
-void QQmlExpressionPrivate::init(QQmlContextData *ctxt, const QString &expr,
-                                 QObject *me, const QString &srcUrl,
-                                 quint16 lineNumber, quint16 columnNumber)
+void QQmlExpressionPrivate::init(QQmlContextData *ctxt, QV4::Function *runtimeFunction, QObject *me)
 {
-    url = srcUrl;
-    line = lineNumber;
-    column = columnNumber;
-
-    expression = expr;
-
-    expressionFunctionValid = false;
+    expressionFunctionValid = true;
+    function = QV4::QmlBindingWrapper::createQmlCallableForFunction(QQmlEnginePrivate::getV4Engine(ctxt->engine), ctxt, me, runtimeFunction);
 
     QQmlAbstractExpression::setContext(ctxt);
     setScopeObject(me);
@@ -156,9 +149,9 @@ QQmlExpression::QQmlExpression(const QQmlScriptString &script, QQmlContext *ctxt
     if (!ctxt && (!scriptPrivate->context || !scriptPrivate->context->isValid()))
         return;
 
-    bool defaultConstruction = true;
     QQmlContextData *evalCtxtData = QQmlContextData::get(ctxt ? ctxt : scriptPrivate->context);
     QObject *scopeObject = scope ? scope : scriptPrivate->scope;
+    QV4::Function *runtimeFunction = 0;
 
     if (scriptPrivate->context) {
         QQmlContextData *ctxtdata = QQmlContextData::get(scriptPrivate->context);
@@ -171,13 +164,19 @@ QQmlExpression::QQmlExpression(const QQmlScriptString &script, QQmlContext *ctxt
                 d->url = cdata->name;
                 d->line = scriptPrivate->lineNumber;
                 d->column = scriptPrivate->columnNumber;
+
+                if (scriptPrivate->bindingId != QQmlBinding::Invalid)
+                    runtimeFunction = cdata->compilationUnit->runtimeFunctions.at(scriptPrivate->bindingId);
             }
 
             typeData->release();
         }
     }
 
-    if (defaultConstruction)
+    if (runtimeFunction) {
+        d->expression = scriptPrivate->script;
+        d->init(evalCtxtData, runtimeFunction, scopeObject);
+    } else
         d->init(evalCtxtData, scriptPrivate->script, scopeObject);
 }
 
diff --git a/src/qml/qml/qqmlexpression_p.h b/src/qml/qml/qqmlexpression_p.h
index e84d193837..5488459962 100644
--- a/src/qml/qml/qqmlexpression_p.h
+++ b/src/qml/qml/qqmlexpression_p.h
@@ -77,7 +77,7 @@ public:
     ~QQmlExpressionPrivate();
 
     void init(QQmlContextData *, const QString &, QObject *);
-    void init(QQmlContextData *, const QString &, QObject *, const QString &, quint16, quint16);
+    void init(QQmlContextData *, QV4::Function *runtimeFunction, QObject *);
 
     QVariant value(bool *isUndefined = 0);
 
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index f6d1e81f3f..ff32266588 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -744,7 +744,7 @@ bool QQmlObjectCreator::setPropertyBinding(QQmlPropertyData *property, const QV4
     // ### resolve this at compile time
     if (property && property->propType == qMetaTypeId<QQmlScriptString>()) {
         QQmlScriptString ss(binding->valueAsScriptString(&qmlUnit->header), context->asQQmlContext(), _scopeObject);
-        ss.d.data()->bindingId = QQmlBinding::Invalid;
+        ss.d.data()->bindingId = binding->value.compiledScriptIndex;
         ss.d.data()->lineNumber = binding->location.line;
         ss.d.data()->columnNumber = binding->location.column;
         ss.d.data()->isStringLiteral = binding->type == QV4::CompiledData::Binding::Type_String;
@@ -1125,7 +1125,7 @@ QObject *QQmlObjectCreator::createInstance(int index, QObject *parent, bool isCo
     if (isComponent)
         return instance;
 
-    QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.value(index);
+    QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.at(index);
     Q_ASSERT(!cache.isNull());
     if (installPropertyCache) {
         if (ddata->propertyCache)
@@ -1273,7 +1273,7 @@ bool QQmlObjectCreator::populateInstance(int index, QObject *instance, QObject *
     QV4::Scope valueScope(v4);
     QV4::ScopedValue scopeObjectProtector(valueScope);
 
-    QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.value(index);
+    QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.at(index);
 
     QQmlVMEMetaObject *vmeMetaObject = 0;
     const QByteArray data = vmeMetaObjectData.value(index);