From c734706c69ed5b38cc97aea5be3f0553c194aa0a Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Wed, 30 May 2012 10:24:14 +1000 Subject: [PATCH] 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 --- src/qml/qml/qqmlbinding.cpp | 15 ++++------ src/qml/qml/qqmlboundsignal.cpp | 6 ++-- src/qml/qml/qqmlcomponent.cpp | 2 +- src/qml/qml/qqmlengine.cpp | 14 ++++++++++ src/qml/qml/qqmlengine_p.h | 2 ++ src/qml/qml/qqmlexpression.cpp | 2 +- src/qml/qml/qqmlincubator.cpp | 4 +-- src/qml/qml/qqmljavascriptexpression.cpp | 48 ++++++++++++++++++++++++++++---- src/qml/qml/qqmljavascriptexpression_p.h | 21 ++++++++++---- src/qml/qml/qqmlproperty.cpp | 18 ++++++------ src/qml/qml/v4/qv4bindings.cpp | 13 ++++----- src/qml/qml/v8/qv8bindings.cpp | 13 ++++----- 12 files changed, 106 insertions(+), 52 deletions(-) diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp index 81826d7..38d305a 100644 --- 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 --- 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 --- 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 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1463,6 +1463,12 @@ void QQmlEnginePrivate::warning(const QList &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 &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 --- 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 &); + void warning(QQmlDelayedError *); static void warning(QQmlEngine *, const QQmlError &); static void warning(QQmlEngine *, const QList &); + static void warning(QQmlEngine *, QQmlDelayedError *); static void warning(QQmlEnginePrivate *, const QQmlError &); static void warning(QQmlEnginePrivate *, const QList &); diff --git a/src/qml/qml/qqmlexpression.cpp b/src/qml/qml/qqmlexpression.cpp index d2842f7..8c24c4a 100644 --- 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 --- 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 --- 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 message) +{ + qPersistentDispose(m_message); + m_message = qPersistentNew(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 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 --- 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 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 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 --- 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()) { 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 --- 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 --- 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(); } -- 2.7.4