From: Michael Brasser Date: Wed, 30 May 2012 00:24:14 +0000 (+1000) Subject: Delay conversion of v8 exceptions to QQmlErrors. X-Git-Tag: 071012131707~218 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=c734706c69ed5b38cc97aea5be3f0553c194aa0a;p=profile%2Fivi%2Fqtdeclarative.git 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 --- 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(); }