Delay conversion of v8 exceptions to QQmlErrors.

This conversion in relatively expensive, and not required for transient
exceptions that occur during startup due to the order of binding
evaluation.

Change-Id: I29a16c075890c8966c0bad0a77412ad232c791bb
Reviewed-by: Matthew Vogt <matthew.vogt@nokia.com>

diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index 81826d7..38d305a 100644 (file)
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/src/qml/qml/qqmlbinding.cpp
@@ -238,23 +238,18 @@ void QQmlBinding::update(QQmlPropertyPrivate::WriteFlags flags)
 
             trace.event("writing binding result");
 
-            bool needsErrorData = false;
+            bool needsErrorLocationData = false;
             if (!watcher.wasDeleted() && !hasError())
-                needsErrorData = !QQmlPropertyPrivate::writeBinding(*m_coreObject, m_core, context(),
+                needsErrorLocationData = !QQmlPropertyPrivate::writeBinding(*m_coreObject, m_core, context(),
                                                                     this, result, isUndefined, flags);
 
             if (!watcher.wasDeleted()) {
                
-                if (needsErrorData) {
-                    QUrl url = QUrl(m_url);
-
-                    delayedError()->error.setUrl(url);
-                    delayedError()->error.setLine(m_lineNumber);
-                    delayedError()->error.setColumn(m_columnNumber);
-                }
+                if (needsErrorLocationData)
+                    delayedError()->setErrorLocation(QUrl(m_url), m_lineNumber, m_columnNumber);
 
                 if (hasError()) {
-                    if (!delayedError()->addError(ep)) ep->warning(this->error());
+                    if (!delayedError()->addError(ep)) ep->warning(this->error(context()->engine));
                 } else {
                     clearError();
                 }

diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp
index bc8ce07..7ed7230 100644 (file)
--- a/src/qml/qml/qqmlboundsignal.cpp
+++ b/src/qml/qml/qqmlboundsignal.cpp
@@ -344,8 +344,10 @@ void QQmlBoundSignal_callback(QQmlNotifierEndpoint *e, void **a)
     if (s->m_params) s->m_params->setValues(a);
     if (s->m_expression && s->m_expression->engine()) {
         s->m_expression->evaluate(s->m_params);
-        if (s->m_expression && s->m_expression->hasError())
-            QQmlEnginePrivate::warning(s->m_expression->engine(), s->m_expression->error());
+        if (s->m_expression && s->m_expression->hasError()) {
+            QQmlEngine *engine = s->m_expression->engine();
+            QQmlEnginePrivate::warning(engine, s->m_expression->error(engine));
+        }
     }
     if (s->m_params) s->m_params->clearValues();
 

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index d31b4d4..ecadc00 100644 (file)
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -884,7 +884,7 @@ void QQmlComponentPrivate::complete(QQmlEnginePrivate *enginePriv, ConstructionS
 
         if (0 == enginePriv->inProgressCreations) {
             while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings->error);
+                enginePriv->warning(enginePriv->erroredBindings);
                 enginePriv->erroredBindings->removeError();
             }
         }

diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 6b1c869..7b4b827 100644 (file)
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1463,6 +1463,12 @@ void QQmlEnginePrivate::warning(const QList<QQmlError> &errors)
         dumpwarning(errors);
 }
 
+void QQmlEnginePrivate::warning(QQmlDelayedError *error)
+{
+    Q_Q(QQmlEngine);
+    warning(error->error(q));
+}
+
 void QQmlEnginePrivate::warning(QQmlEngine *engine, const QQmlError &error)
 {
     if (engine)
@@ -1479,6 +1485,14 @@ void QQmlEnginePrivate::warning(QQmlEngine *engine, const QList<QQmlError> &erro
         dumpwarning(error);
 }
 
+void QQmlEnginePrivate::warning(QQmlEngine *engine, QQmlDelayedError *error)
+{
+    if (engine)
+        QQmlEnginePrivate::get(engine)->warning(error);
+    else
+        dumpwarning(error->error(0));
+}
+
 void QQmlEnginePrivate::warning(QQmlEnginePrivate *engine, const QQmlError &error)
 {
     if (engine)

diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index 4518ba8..f74ded8 100644 (file)
--- a/src/qml/qml/qqmlengine_p.h
+++ b/src/qml/qml/qqmlengine_p.h
@@ -252,8 +252,10 @@ public:
     void sendQuit();
     void warning(const QQmlError &);
     void warning(const QList<QQmlError> &);
+    void warning(QQmlDelayedError *);
     static void warning(QQmlEngine *, const QQmlError &);
     static void warning(QQmlEngine *, const QList<QQmlError> &);
+    static void warning(QQmlEngine *, QQmlDelayedError *);
     static void warning(QQmlEnginePrivate *, const QQmlError &);
     static void warning(QQmlEnginePrivate *, const QList<QQmlError> &);
 

diff --git a/src/qml/qml/qqmlexpression.cpp b/src/qml/qml/qqmlexpression.cpp
index d2842f7..8c24c4a 100644 (file)
--- a/src/qml/qml/qqmlexpression.cpp
+++ b/src/qml/qml/qqmlexpression.cpp
@@ -521,7 +521,7 @@ void QQmlExpression::clearError()
 QQmlError QQmlExpression::error() const
 {
     Q_D(const QQmlExpression);
-    return d->error();
+    return d->error(engine());
 }
 
 /*!

diff --git a/src/qml/qml/qqmlincubator.cpp b/src/qml/qml/qqmlincubator.cpp
index 12a48a2..81adf9e 100644 (file)
--- a/src/qml/qml/qqmlincubator.cpp
+++ b/src/qml/qml/qqmlincubator.cpp
@@ -354,7 +354,7 @@ finishIncubate:
 
         if (0 == enginePriv->inProgressCreations) {
             while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings->error);
+                enginePriv->warning(enginePriv->erroredBindings);
                 enginePriv->erroredBindings->removeError();
             }
         }
@@ -569,7 +569,7 @@ void QQmlIncubator::clear()
         enginePriv->inProgressCreations--;
         if (0 == enginePriv->inProgressCreations) {
             while (enginePriv->erroredBindings) {
-                enginePriv->warning(enginePriv->erroredBindings->error);
+                enginePriv->warning(enginePriv->erroredBindings);
                 enginePriv->erroredBindings->removeError();
             }
         }

diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp
index d021882..9b61756 100644 (file)
--- a/src/qml/qml/qqmljavascriptexpression.cpp
+++ b/src/qml/qml/qqmljavascriptexpression.cpp
@@ -61,6 +61,42 @@ bool QQmlDelayedError::addError(QQmlEnginePrivate *e)
     return true;
 }
 
+void QQmlDelayedError::setMessage(v8::Handle<v8::Message> message)
+{
+    qPersistentDispose(m_message);
+    m_message = qPersistentNew<v8::Message>(message);
+}
+
+void QQmlDelayedError::setErrorLocation(const QUrl &url, int line, int column)
+{
+    m_error.setUrl(url);
+    m_error.setLine(line);
+    m_error.setColumn(column);
+}
+
+void QQmlDelayedError::setErrorDescription(const QString &description)
+{
+    m_error.setDescription(description);
+}
+
+/*
+    Converting from a message to an error is relatively expensive.
+
+    We don't want to do this work for transient exceptions (exceptions
+    that occur during startup because of the order of binding
+    execution, but have gone away by the time startup has finished), so we
+    delay conversion until it is required for displaying the error.
+*/
+void QQmlDelayedError::convertMessageToError(QQmlEngine *engine) const
+{
+    if (!m_message.IsEmpty() && engine) {
+        v8::HandleScope handle_scope;
+        v8::Context::Scope context_scope(QQmlEnginePrivate::getV8Engine(engine)->context());
+        QQmlExpressionPrivate::exceptionToError(m_message, m_error);
+        qPersistentDispose(m_message);
+    }
+}
+
 QQmlJavaScriptExpression::QQmlJavaScriptExpression(VTable *v)
 : m_vtable(v)
 {
@@ -142,12 +178,12 @@ QQmlJavaScriptExpression::evaluate(QQmlContextData *context,
             v8::Context::Scope scope(ep->v8engine()->context());
             v8::Local<v8::Message> message = try_catch.Message();
             if (!message.IsEmpty()) {
-                QQmlExpressionPrivate::exceptionToError(message, delayedError()->error);
+                delayedError()->setMessage(message);
             } else {
-                if (hasDelayedError()) delayedError()->error = QQmlError();
+                if (hasDelayedError()) delayedError()->clearError();
             }
         } else {
-            if (hasDelayedError()) delayedError()->error = QQmlError();
+            if (hasDelayedError()) delayedError()->clearError();
         }
     }
 
@@ -237,14 +273,14 @@ void QQmlJavaScriptExpression::GuardCapture::captureProperty(QObject *o, int c,
 void QQmlJavaScriptExpression::clearError()
 {
     if (m_vtable.hasValue()) {
-        m_vtable.value().error = QQmlError();
+        m_vtable.value().clearError();
         m_vtable.value().removeError();
     }
 }
 
-QQmlError QQmlJavaScriptExpression::error() const
+QQmlError QQmlJavaScriptExpression::error(QQmlEngine *engine) const
 {
-    if (m_vtable.hasValue()) return m_vtable.constValue()->error;
+    if (m_vtable.hasValue()) return m_vtable.constValue()->error(engine);
     else return QQmlError();
 }
 

diff --git a/src/qml/qml/qqmljavascriptexpression_p.h b/src/qml/qml/qqmljavascriptexpression_p.h
index 4208584..4c89a08 100644 (file)
--- a/src/qml/qml/qqmljavascriptexpression_p.h
+++ b/src/qml/qml/qqmljavascriptexpression_p.h
@@ -65,9 +65,7 @@ class QQmlDelayedError
 {
 public:
     inline QQmlDelayedError() : nextError(0), prevError(0) {}
-    inline ~QQmlDelayedError() { removeError(); }
-
-    QQmlError error;
+    inline ~QQmlDelayedError() { qPersistentDispose(m_message); removeError(); }
 
     bool addError(QQmlEnginePrivate *);
 
@@ -79,7 +77,20 @@ public:
         prevError = 0;
     }
 
+    inline bool isValid() const { return !m_message.IsEmpty() || m_error.isValid(); }
+    inline const QQmlError &error(QQmlEngine *engine) const { convertMessageToError(engine); return m_error; }
+    inline void clearError() { qPersistentDispose(m_message); m_error = QQmlError(); }
+
+    void setMessage(v8::Handle<v8::Message> message);
+    void setErrorLocation(const QUrl &url, int line, int column);
+    void setErrorDescription(const QString &description);
+
 private:
+    void convertMessageToError(QQmlEngine *engine) const;
+
+    mutable QQmlError m_error;
+    mutable v8::Persistent<v8::Message> m_message;
+
     QQmlDelayedError  *nextError;
     QQmlDelayedError **prevError;
 };
@@ -128,7 +139,7 @@ public:
 
     inline bool hasError() const;
     inline bool hasDelayedError() const;
-    QQmlError error() const;
+    QQmlError error(QQmlEngine *) const;
     void clearError();
     QQmlDelayedError *delayedError();
 
@@ -242,7 +253,7 @@ void QQmlJavaScriptExpression::setScopeObject(QObject *v)
 
 bool QQmlJavaScriptExpression::hasError() const
 {
-    return m_vtable.hasValue() && m_vtable.constValue()->error.isValid();
+    return m_vtable.hasValue() && m_vtable.constValue()->isValid();
 }
 
 bool QQmlJavaScriptExpression::hasDelayedError() const

diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp
index 5450fda..2a8aa32 100644 (file)
--- a/src/qml/qml/qqmlproperty.cpp
+++ b/src/qml/qml/qqmlproperty.cpp
@@ -1545,7 +1545,7 @@ bool QQmlPropertyPrivate::writeBinding(QObject *object,
                 && !result->ToObject()->GetHiddenValue(v8engine->bindingFlagKey()).IsEmpty()) {
             // we explicitly disallow this case to avoid confusion.  Users can still store one
             // in an array in a var property if they need to, but the common case is user error.
-            expression->delayedError()->error.setDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
+            expression->delayedError()->setErrorDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
             return false;
         }
 
@@ -1561,7 +1561,7 @@ bool QQmlPropertyPrivate::writeBinding(QObject *object,
     } else if (type == qMetaTypeId<QJSValue>()) {
         if (!result.IsEmpty() && result->IsFunction()
                 && !result->ToObject()->GetHiddenValue(v8engine->bindingFlagKey()).IsEmpty()) {
-            expression->delayedError()->error.setDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
+            expression->delayedError()->setErrorDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
             return false;
         }
         writeValueProperty(object, engine, core, QVariant::fromValue(v8engine->scriptValueFromInternal(result)), context, flags);
@@ -1571,13 +1571,13 @@ bool QQmlPropertyPrivate::writeBinding(QObject *object,
             errorStr += QLatin1String("[unknown property type]");
         else
             errorStr += QLatin1String(QMetaType::typeName(type));
-        expression->delayedError()->error.setDescription(errorStr);
+        expression->delayedError()->setErrorDescription(errorStr);
         return false;
     } else if (result->IsFunction()) {
         if (!result->ToObject()->GetHiddenValue(v8engine->bindingFlagKey()).IsEmpty())
-            expression->delayedError()->error.setDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
+            expression->delayedError()->setErrorDescription(QLatin1String("Invalid use of Qt.binding() in a binding declaration."));
         else
-            expression->delayedError()->error.setDescription(QLatin1String("Unable to assign a function to a property of any type other than var."));
+            expression->delayedError()->setErrorDescription(QLatin1String("Unable to assign a function to a property of any type other than var."));
         return false;
     } else if (!writeValueProperty(object, engine, core, value, context, flags)) {
 
@@ -1606,10 +1606,10 @@ bool QQmlPropertyPrivate::writeBinding(QObject *object,
         if (!propertyType)
             propertyType = "[unknown property type]";
 
-        expression->delayedError()->error.setDescription(QLatin1String("Unable to assign ") +
-                                                         QLatin1String(valueType) +
-                                                         QLatin1String(" to ") +
-                                                         QLatin1String(propertyType));
+        expression->delayedError()->setErrorDescription(QLatin1String("Unable to assign ") +
+                                                        QLatin1String(valueType) +
+                                                        QLatin1String(" to ") +
+                                                        QLatin1String(propertyType));
         return false;
     }
 

diff --git a/src/qml/qml/v4/qv4bindings.cpp b/src/qml/qml/v4/qv4bindings.cpp
index ba90654..ff16f3b 100644 (file)
--- a/src/qml/qml/v4/qv4bindings.cpp
+++ b/src/qml/qml/v4/qv4bindings.cpp
@@ -667,21 +667,18 @@ static void throwException(int id, QQmlDelayedError *error,
                            QV4Program *program, QQmlContextData *context,
                            const QString &description = QString())
 {
-    error->error.setUrl(context->url);
     if (description.isEmpty())
-        error->error.setDescription(QLatin1String("TypeError: Result of expression is not an object"));
+        error->setErrorDescription(QLatin1String("TypeError: Result of expression is not an object"));
     else
-        error->error.setDescription(description);
+        error->setErrorDescription(description);
     if (id != 0xFF) {
         quint64 e = *((quint64 *)(program->data() + program->exceptionDataOffset) + id); 
-        error->error.setLine((e >> 32) & 0xFFFFFFFF);
-        error->error.setColumn(e & 0xFFFFFFFF); 
+        error->setErrorLocation(context->url, (e >> 32) & 0xFFFFFFFF, e & 0xFFFFFFFF);
     } else {
-        error->error.setLine(-1);
-        error->error.setColumn(-1);
+        error->setErrorLocation(context->url, -1, -1);
     }
     if (!context->engine || !error->addError(QQmlEnginePrivate::get(context->engine)))
-        QQmlEnginePrivate::warning(context->engine, error->error);
+        QQmlEnginePrivate::warning(context->engine, error);
 }
 
 const double QV4Bindings::D32 = 4294967296.0;

diff --git a/src/qml/qml/v8/qv8bindings.cpp b/src/qml/qml/v8/qv8bindings.cpp
index d8624c7..00134c5 100644 (file)
--- a/src/qml/qml/v8/qv8bindings.cpp
+++ b/src/qml/qml/v8/qv8bindings.cpp
@@ -169,23 +169,20 @@ void QV8Bindings::Binding::update(QQmlPropertyPrivate::WriteFlags flags)
                      &isUndefined);
 
         trace.event("writing V8 result");
-        bool needsErrorData = false;
+        bool needsErrorLocationData = false;
         if (!watcher.wasDeleted() && !destroyedFlag() && !hasError()) {
             typedef QQmlPropertyPrivate PP;
-            needsErrorData = !PP::writeBinding(*target, instruction->property, context, this, result,
+            needsErrorLocationData = !PP::writeBinding(*target, instruction->property, context, this, result,
                                                isUndefined, flags);
         }
 
         if (!watcher.wasDeleted() && !destroyedFlag()) {
 
-            if (needsErrorData) {
-                delayedError()->error.setUrl(parent->url());
-                delayedError()->error.setLine(instruction->line);
-                delayedError()->error.setColumn(-1);
-            }
+            if (needsErrorLocationData)
+                delayedError()->setErrorLocation(parent->url(), instruction->line, -1);
 
             if (hasError()) {
-                if (!delayedError()->addError(ep)) ep->warning(delayedError()->error);
+                if (!delayedError()->addError(ep)) ep->warning(this->error(context->engine));
             } else {
                 clearError();
             }