Loosen shared binding tests

Previously we blocked all function calls, but only because we were trying
to prevent "eval" calls from appearing in shared bindings.  Instead this
test allows function calls as long as the identifier "eval" doesn't
appear.

This also exposed a crash with v8 bindings that is also fixed.

Change-Id: I22eefd290c7b82d9c01b2cd2907a46e560ae4db9
Reviewed-by: Chris Adams <christopher.adams@nokia.com>
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

6 files changed, 42 insertions, 25 deletions

diff --git a/src/qml/qml/qqmlexpression.cpp b/src/qml/qml/qqmlexpression.cpp
index 6e20047cf0..940b3c8354 100644
--- a/src/qml/qml/qqmlexpression.cpp
+++ b/src/qml/qml/qqmlexpression.cpp
@@ -60,7 +60,7 @@ static QQmlJavaScriptExpression::VTable QQmlExpressionPrivate_jsvtable = {
 QQmlExpressionPrivate::QQmlExpressionPrivate()
 : QQmlJavaScriptExpression(&QQmlExpressionPrivate_jsvtable),
   expressionFunctionValid(true), expressionFunctionRewritten(false),
-  extractExpressionFromFunction(false), line(-1), dataRef(0)
+  line(-1)
 {
 }
 
@@ -68,8 +68,6 @@ QQmlExpressionPrivate::~QQmlExpressionPrivate()
 {
     qPersistentDispose(v8qmlscope);
     qPersistentDispose(v8function);
-    if (dataRef) dataRef->release();
-    dataRef = 0;
 }
 
 void QQmlExpressionPrivate::init(QQmlContextData *ctxt, const QString &expr, QObject *me)
@@ -321,13 +319,7 @@ QQmlContext *QQmlExpression::context() const
 QString QQmlExpression::expression() const
 {
     Q_D(const QQmlExpression);
-    if (d->extractExpressionFromFunction && context()->engine()) {
-        QV8Engine *v8engine = QQmlEnginePrivate::getV8Engine(context()->engine());
-        v8::HandleScope handle_scope;
-        v8::Context::Scope scope(v8engine->context());
-
-        return v8engine->toString(v8::Handle<v8::Value>(d->v8function));
-    } else if (!d->expressionUtf8.isEmpty()) {
+    if (!d->expressionUtf8.isEmpty()) {
         return QString::fromUtf8(d->expressionUtf8);
     } else {
         return d->expression;
diff --git a/src/qml/qml/qqmlexpression_p.h b/src/qml/qml/qqmlexpression_p.h
index d3d27f259d..d32e2d314c 100644
--- a/src/qml/qml/qqmlexpression_p.h
+++ b/src/qml/qml/qqmlexpression_p.h
@@ -96,7 +96,6 @@ public:
 
     bool expressionFunctionValid:1;
     bool expressionFunctionRewritten:1;
-    bool extractExpressionFromFunction:1;
 
     // "Inherited" from QQmlJavaScriptExpression
     static QString expressionIdentifier(QQmlJavaScriptExpression *);
@@ -113,8 +112,6 @@ public:
     int line;
     int column;
     QString name; //function name, hint for the debugger
-
-    QQmlRefCount *dataRef;
 };
 
 QQmlExpressionPrivate *QQmlExpressionPrivate::get(QQmlExpression *expr)
diff --git a/src/qml/qml/qqmlrewrite.cpp b/src/qml/qml/qqmlrewrite.cpp
index 0bd8597ec4..bbb17b631d 100644
--- a/src/qml/qml/qqmlrewrite.cpp
+++ b/src/qml/qml/qqmlrewrite.cpp
@@ -51,6 +51,8 @@ DEFINE_BOOL_CONFIG_OPTION(rewriteDump, QML_REWRITE_DUMP);
 
 namespace QQmlRewrite {
 
+QString SharedBindingTester::evalString("eval");
+
 static void rewriteStringLiteral(AST::StringLiteral *ast, const QString *code, int startPosition, TextWriter *writer)
 {
     const unsigned position = ast->firstSourceLocation().begin() - startPosition + 1;
diff --git a/src/qml/qml/qqmlrewrite_p.h b/src/qml/qml/qqmlrewrite_p.h
index 73ff50a040..efa10ce316 100644
--- a/src/qml/qml/qqmlrewrite_p.h
+++ b/src/qml/qml/qqmlrewrite_p.h
@@ -70,9 +70,11 @@ public:
     bool isSharable(const QString &code);
     bool isSharable(AST::Node *Node);
     
-    virtual bool visit(AST::FunctionDeclaration *) { _sharable = false; return false; }
-    virtual bool visit(AST::FunctionExpression *) { _sharable = false; return false; }
-    virtual bool visit(AST::CallExpression *) { _sharable = false; return false; }
+    inline virtual bool visit(AST::FunctionDeclaration *);
+    inline virtual bool visit(AST::FunctionExpression *);
+    inline virtual bool visit(AST::IdentifierExpression *);
+
+    static QString evalString;
 };
 
 class RewriteBinding: protected AST::Visitor
@@ -143,6 +145,26 @@ protected:
     virtual bool visit(AST::StringLiteral *ast);
 };
 
+bool SharedBindingTester::visit(AST::FunctionDeclaration *)
+{
+    _sharable = false;
+    return false;
+}
+
+bool SharedBindingTester::visit(AST::FunctionExpression *)
+{
+    _sharable = false;
+    return false;
+}
+
+bool SharedBindingTester::visit(AST::IdentifierExpression *e)
+{
+    if (e->name == evalString)
+        _sharable = false;
+
+    return false; // IdentifierExpression is a leaf node anyway
+}
+
 } // namespace QQmlRewrite
 
 QT_END_NAMESPACE
diff --git a/src/qml/qml/v8/qv8bindings.cpp b/src/qml/qml/v8/qv8bindings.cpp
index 4b96679cf3..487e1005e7 100644
--- a/src/qml/qml/v8/qv8bindings.cpp
+++ b/src/qml/qml/v8/qv8bindings.cpp
@@ -58,7 +58,7 @@ static QQmlJavaScriptExpression::VTable QV8Bindings_Binding_jsvtable = {
 };
 
 QV8Bindings::Binding::Binding()
-: QQmlJavaScriptExpression(&QV8Bindings_Binding_jsvtable), target(0), parent(0)
+: QQmlJavaScriptExpression(&QV8Bindings_Binding_jsvtable), parent(0)
 {
 }
 
@@ -90,7 +90,7 @@ int QV8Bindings::Binding::propertyIndex() const
 
 QObject *QV8Bindings::Binding::object() const
 {
-    return target;
+    return *target;
 }
 
 void QV8Bindings::Binding::update(QQmlPropertyPrivate::WriteFlags flags)
@@ -127,13 +127,13 @@ void QV8Bindings::Binding::update(QQmlPropertyPrivate::WriteFlags flags)
 
         trace.event("writing V8 result");
         bool needsErrorData = false;
-        if (!watcher.wasDeleted() && !hasError()) {
+        if (!watcher.wasDeleted() && !destroyedFlag() && !hasError()) {
             typedef QQmlPropertyPrivate PP;
-            needsErrorData = !PP::writeBinding(target, instruction->property, context, this, result,
+            needsErrorData = !PP::writeBinding(*target, instruction->property, context, this, result,
                                                isUndefined, flags);
         }
 
-        if (!watcher.wasDeleted()) {
+        if (!watcher.wasDeleted() && !destroyedFlag()) {
 
             if (needsErrorData) {
                 QUrl url = parent->url();
@@ -156,7 +156,7 @@ void QV8Bindings::Binding::update(QQmlPropertyPrivate::WriteFlags flags)
         ep->dereferenceScarceResources(); 
 
     } else {
-        QQmlProperty p = QQmlPropertyPrivate::restore(target, instruction->property, context);
+        QQmlProperty p = QQmlPropertyPrivate::restore(*target, instruction->property, context);
         QQmlAbstractBinding::printBindingLoopError(p);
     }
 }
@@ -177,6 +177,7 @@ void QV8Bindings::Binding::expressionChanged(QQmlJavaScriptExpression *e)
 void QV8Bindings::Binding::destroy()
 {
     setEnabledFlag(false);
+    setDestroyedFlag(true);
     removeFromObject();
     clear();
     clearError();
diff --git a/src/qml/qml/v8/qv8bindings_p.h b/src/qml/qml/v8/qv8bindings_p.h
index ad5b2cb8b0..8d27207a56 100644
--- a/src/qml/qml/v8/qv8bindings_p.h
+++ b/src/qml/qml/v8/qv8bindings_p.h
@@ -99,14 +99,17 @@ public:
         virtual int propertyIndex() const;
         virtual QObject *object() const;
 
-        QObject *target;
         QV8Bindings *parent;
 
         // To save memory, we store flags inside the instruction pointer.
-        //    flag1: enabled
-        //    flag2: updating
+        //    target.flag1: destroyed
+        //    instruction.flag1: enabled
+        //    instruction.flag2: updating
+        QFlagPointer<QObject> target;
         QFlagPointer<const QQmlInstruction::instr_assignBinding> instruction;
 
+        inline bool destroyedFlag() const { return target.flag(); }
+        inline void setDestroyedFlag(bool v) { return target.setFlagValue(v); }
         inline bool enabledFlag() const { return instruction.flag(); }
         inline void setEnabledFlag(bool v) { instruction.setFlagValue(v); }
         inline bool updatingFlag() const { return instruction.flag2(); }