From eaab2a271f0d73d5a413902169efe32741208c42 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Wed, 28 Oct 2009 13:59:33 +1000 Subject: Add a QmlExpression::error() method QmlExpression should not print errors itself. This is the responsibility of the caller. --- src/declarative/qml/qmlbinding.cpp | 8 +++-- src/declarative/qml/qmlboundsignal.cpp | 5 +++- src/declarative/qml/qmlcontext.cpp | 7 +++-- src/declarative/qml/qmlerror.cpp | 42 +++++++++++++++++++------- src/declarative/qml/qmlerror.h | 2 ++ src/declarative/qml/qmlexpression.cpp | 55 +++++++++++++++++++++++++++++++--- src/declarative/qml/qmlexpression.h | 5 ++++ src/declarative/qml/qmlexpression_p.h | 4 ++- 8 files changed, 107 insertions(+), 21 deletions(-) diff --git a/src/declarative/qml/qmlbinding.cpp b/src/declarative/qml/qmlbinding.cpp index 65ff789..ab4343c 100644 --- a/src/declarative/qml/qmlbinding.cpp +++ b/src/declarative/qml/qmlbinding.cpp @@ -128,10 +128,12 @@ void QmlBinding::update(QmlMetaProperty::WriteFlags flags) idx, a); } else { - bool undefined = false; - QVariant value = this->value(&undefined); + bool isUndefined = false; + QVariant value = this->value(&isUndefined); - if (!undefined && data->property.object() && !data->property.write(value, flags)) { + if (this->hasError()) { + qWarning().nospace() << qPrintable(this->error().toString()); + } else if (!isUndefined && data->property.object() && !data->property.write(value, flags)) { QString fileName = data->fileName; int line = data->line; if (fileName.isEmpty()) fileName = QLatin1String(""); diff --git a/src/declarative/qml/qmlboundsignal.cpp b/src/declarative/qml/qmlboundsignal.cpp index ce591e8..d715309 100644 --- a/src/declarative/qml/qmlboundsignal.cpp +++ b/src/declarative/qml/qmlboundsignal.cpp @@ -175,8 +175,11 @@ int QmlBoundSignal::qt_metacall(QMetaObject::Call c, int id, void **a) { if (c == QMetaObject::InvokeMetaMethod && id == evaluateIdx) { if (m_params) m_params->setValues(a); - if (m_expression) + if (m_expression) { QmlExpressionPrivate::get(m_expression)->value(m_params); + if (m_expression->hasError()) + qWarning().nospace() << qPrintable(m_expression->error().toString()); + } if (m_params) m_params->clearValues(); return -1; } else { diff --git a/src/declarative/qml/qmlcontext.cpp b/src/declarative/qml/qmlcontext.cpp index 2ebdf10..5032ff4 100644 --- a/src/declarative/qml/qmlcontext.cpp +++ b/src/declarative/qml/qmlcontext.cpp @@ -77,8 +77,11 @@ void QmlContextPrivate::addScript(const QString &script, QObject *scopeObject, QScriptValue val = scriptEngine->evaluate(script, fileName, lineNumber); - if (scriptEngine->hasUncaughtException()) - QmlExpressionPrivate::printException(scriptEngine); + if (scriptEngine->hasUncaughtException()) { + QmlError error; + QmlExpressionPrivate::exceptionToError(scriptEngine, error); + qWarning().nospace() << qPrintable(error.toString()); + } scriptEngine->popContext(); diff --git a/src/declarative/qml/qmlerror.cpp b/src/declarative/qml/qmlerror.cpp index 514fe44..f4c9580 100644 --- a/src/declarative/qml/qmlerror.cpp +++ b/src/declarative/qml/qmlerror.cpp @@ -70,7 +70,7 @@ QmlErrorPrivate::QmlErrorPrivate() Create an empty error object. */ QmlError::QmlError() -: d(new QmlErrorPrivate) +: d(0) { } @@ -78,7 +78,7 @@ QmlError::QmlError() Create a copy of \a other. */ QmlError::QmlError(const QmlError &other) -: d(new QmlErrorPrivate) +: d(0) { *this = other; } @@ -88,10 +88,16 @@ QmlError::QmlError(const QmlError &other) */ QmlError &QmlError::operator=(const QmlError &other) { - d->url = other.d->url; - d->description = other.d->description; - d->line = other.d->line; - d->column = other.d->column; + if (!other.d) { + delete d; + d = 0; + } else { + if (!d) d = new QmlErrorPrivate; + d->url = other.d->url; + d->description = other.d->description; + d->line = other.d->line; + d->column = other.d->column; + } return *this; } @@ -104,11 +110,20 @@ QmlError::~QmlError() } /*! + Return true if this error is valid, otherwise false. +*/ +bool QmlError::isValid() const +{ + return d != 0; +} + +/*! Return the url for the file that caused this error. */ QUrl QmlError::url() const { - return d->url; + if (d) return d->url; + else return QUrl(); } /*! @@ -116,6 +131,7 @@ QUrl QmlError::url() const */ void QmlError::setUrl(const QUrl &url) { + if (!d) d = new QmlErrorPrivate; d->url = url; } @@ -124,7 +140,8 @@ void QmlError::setUrl(const QUrl &url) */ QString QmlError::description() const { - return d->description; + if (d) return d->description; + else return QString(); } /*! @@ -132,6 +149,7 @@ QString QmlError::description() const */ void QmlError::setDescription(const QString &description) { + if (!d) d = new QmlErrorPrivate; d->description = description; } @@ -140,7 +158,8 @@ void QmlError::setDescription(const QString &description) */ int QmlError::line() const { - return d->line; + if (d) return d->line; + else return -1; } /*! @@ -148,6 +167,7 @@ int QmlError::line() const */ void QmlError::setLine(int line) { + if (!d) d = new QmlErrorPrivate; d->line = line; } @@ -156,7 +176,8 @@ void QmlError::setLine(int line) */ int QmlError::column() const { - return d->column; + if (d) return d->column; + else return -1; } /*! @@ -164,6 +185,7 @@ int QmlError::column() const */ void QmlError::setColumn(int column) { + if (!d) d = new QmlErrorPrivate; d->column = column; } diff --git a/src/declarative/qml/qmlerror.h b/src/declarative/qml/qmlerror.h index c1a8720..ee3d7b4 100644 --- a/src/declarative/qml/qmlerror.h +++ b/src/declarative/qml/qmlerror.h @@ -61,6 +61,8 @@ public: QmlError &operator=(const QmlError &); ~QmlError(); + bool isValid() const; + QUrl url() const; void setUrl(const QUrl &); QString description() const; diff --git a/src/declarative/qml/qmlexpression.cpp b/src/declarative/qml/qmlexpression.cpp index c62756b..8231bf8 100644 --- a/src/declarative/qml/qmlexpression.cpp +++ b/src/declarative/qml/qmlexpression.cpp @@ -260,7 +260,8 @@ QVariant QmlExpressionPrivate::evalSSE() return rv; } -void QmlExpressionPrivate::printException(QScriptEngine *scriptEngine) +void QmlExpressionPrivate::exceptionToError(QScriptEngine *scriptEngine, + QmlError &error) { if (scriptEngine->hasUncaughtException() && scriptEngine->uncaughtException().isError()) { @@ -277,8 +278,12 @@ void QmlExpressionPrivate::printException(QScriptEngine *scriptEngine) fileName = QLatin1String(""); } - qWarning().nospace() << qPrintable(fileName) << ":" << lineNumber << ": " - << qPrintable(exception.toString()); + error.setUrl(QUrl(fileName)); + error.setLine(lineNumber); + error.setColumn(-1); + error.setDescription(exception.toString()); + } else { + error = QmlError(); } } @@ -321,10 +326,13 @@ QVariant QmlExpressionPrivate::evalQtScript(QObject *secondaryScope, bool *isUnd if (isUndefined) *isUndefined = svalue.isUndefined() || scriptEngine->hasUncaughtException(); + // Handle exception if (scriptEngine->hasUncaughtException()) { - printException(scriptEngine); + exceptionToError(scriptEngine, data->error); scriptEngine->clearExceptions(); return QVariant(); + } else { + data->error = QmlError(); } if (secondaryScope) { @@ -418,6 +426,8 @@ QVariant QmlExpressionPrivate::value(QObject *secondaryScope, bool *isUndefined) \a isUndefined is set to true if the expression resulted in an undefined value. + + \sa hasError(), error() */ QVariant QmlExpression::value(bool *isUndefined) { @@ -509,6 +519,43 @@ QObject *QmlExpression::scopeObject() const return d->data->me; } +/*! + Returns true if the last call to value() resulted in an error, + otherwise false. + + \sa error(), clearError() +*/ +bool QmlExpression::hasError() const +{ + Q_D(const QmlExpression); + return d->data->error.isValid(); +} + +/*! + Clear any expression errors. Calls to hasError() following this will + return false. + + \sa hasError(), error() +*/ +void QmlExpression::clearError() +{ + Q_D(QmlExpression); + d->data->error = QmlError(); +} + +/*! + Return any error from the last call to value(). If there was no error, + this returns an invalid QmlError instance. + + \sa hasError(), clearError() +*/ + +QmlError QmlExpression::error() const +{ + Q_D(const QmlExpression); + return d->data->error; +} + /*! \internal */ void QmlExpression::__q_notify() { diff --git a/src/declarative/qml/qmlexpression.h b/src/declarative/qml/qmlexpression.h index 96694d6..127d3f3 100644 --- a/src/declarative/qml/qmlexpression.h +++ b/src/declarative/qml/qmlexpression.h @@ -44,6 +44,7 @@ #include #include +#include QT_BEGIN_HEADER @@ -82,6 +83,10 @@ public: QObject *scopeObject() const; + bool hasError() const; + void clearError(); + QmlError error() const; + public Q_SLOTS: QVariant value(bool *isUndefined = 0); diff --git a/src/declarative/qml/qmlexpression_p.h b/src/declarative/qml/qmlexpression_p.h index 3ec8d1c..b453a96 100644 --- a/src/declarative/qml/qmlexpression_p.h +++ b/src/declarative/qml/qmlexpression_p.h @@ -92,6 +92,8 @@ public: bool expressionRewritten:1; QScriptValue expressionFunction; + QmlError error; + QmlBasicScript sse; QObject *me; bool trackChange; @@ -151,7 +153,7 @@ public: return static_cast(QObjectPrivate::get(expr)); } - static void printException(QScriptEngine *); + static void exceptionToError(QScriptEngine *, QmlError &); }; QT_END_NAMESPACE -- cgit v0.12 From d42846123ec17641d1e8a8082fab0d04ce93e8a1 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Wed, 28 Oct 2009 14:57:16 +1000 Subject: Do not display transient binding errors During QML startup, it is common to have "errors" in bindings as the apps state stabilizes. These are not real errors, but just a consequence of implementing a declarative UI in an imperative world. Now during startup, the display of errors is delayed until the startup completes, and then only bindings that are still in an error state are displayed. QT-2373 --- src/declarative/qml/qmlbinding.cpp | 62 +++++++++++++++++++++++++++++++---- src/declarative/qml/qmlbinding_p.h | 6 ++++ src/declarative/qml/qmlcomponent.cpp | 17 ++++++++++ src/declarative/qml/qmlengine.cpp | 5 +-- src/declarative/qml/qmlengine_p.h | 5 +++ src/declarative/qml/qmlexpression.cpp | 1 + src/declarative/qml/qmlexpression_p.h | 2 +- 7 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/declarative/qml/qmlbinding.cpp b/src/declarative/qml/qmlbinding.cpp index ab4343c..317a4b3 100644 --- a/src/declarative/qml/qmlbinding.cpp +++ b/src/declarative/qml/qmlbinding.cpp @@ -58,10 +58,46 @@ QT_BEGIN_NAMESPACE QML_DEFINE_NOCREATE_TYPE(QmlBinding); QmlBindingData::QmlBindingData() -: updating(false), enabled(false) +: updating(false), enabled(false), nextError(0), prevError(0) { } +QmlBindingData::~QmlBindingData() +{ + removeError(); +} + +void QmlBindingData::removeError() +{ + if (!prevError) return; + + if (nextError) nextError->prevError = prevError; + *prevError = nextError; + nextError = 0; + prevError = 0; +} + +bool QmlBindingData::addError() +{ + if (prevError) return false; + + QmlContext *c = context(); + if (!c) return false; + QmlEngine *e = c->engine(); + if (!e) return false; + + QmlEnginePrivate *p = QmlEnginePrivate::get(e); + + if (p->inProgressCreations == 0) return false; // Not in construction + + prevError = &p->erroredBindings; + nextError = p->erroredBindings; + p->erroredBindings = this; + if (nextError) nextError->prevError = &nextError; + + return true; +} + QmlBindingPrivate::QmlBindingPrivate() : QmlExpressionPrivate(new QmlBindingData) { @@ -131,9 +167,9 @@ void QmlBinding::update(QmlMetaProperty::WriteFlags flags) bool isUndefined = false; QVariant value = this->value(&isUndefined); - if (this->hasError()) { - qWarning().nospace() << qPrintable(this->error().toString()); - } else if (!isUndefined && data->property.object() && !data->property.write(value, flags)) { + if (!isUndefined && data->property.object() && + !data->property.write(value, flags)) { + QString fileName = data->fileName; int line = data->line; if (fileName.isEmpty()) fileName = QLatin1String(""); @@ -141,9 +177,21 @@ void QmlBinding::update(QmlMetaProperty::WriteFlags flags) const char *valueType = 0; if (value.userType() == QVariant::Invalid) valueType = "null"; else valueType = QMetaType::typeName(value.userType()); - qWarning().nospace() << qPrintable(fileName) << ":" << line - << " Unable to assign " << valueType << " to " - << QMetaType::typeName(data->property.propertyType()); + + data->error.setUrl(fileName); + data->error.setLine(line); + data->error.setColumn(-1); + data->error.setDescription(QLatin1String("Unable to assign ") + + QLatin1String(valueType) + + QLatin1String(" to ") + + QLatin1String(QMetaType::typeName(data->property.propertyType()))); + } + + if (data->error.isValid()) { + if (!data->addError()) + qWarning().nospace() << qPrintable(this->error().toString()); + } else { + data->removeError(); } } diff --git a/src/declarative/qml/qmlbinding_p.h b/src/declarative/qml/qmlbinding_p.h index 2c0c6b9..c9378cb 100644 --- a/src/declarative/qml/qmlbinding_p.h +++ b/src/declarative/qml/qmlbinding_p.h @@ -63,11 +63,17 @@ class QmlBindingData : public QmlExpressionData { public: QmlBindingData(); + virtual ~QmlBindingData(); bool updating:1; bool enabled:1; QmlMetaProperty property; + + void removeError(); + bool addError(); + QmlBindingData *nextError; + QmlBindingData **prevError; }; class QmlBindingPrivate : public QmlExpressionPrivate diff --git a/src/declarative/qml/qmlcomponent.cpp b/src/declarative/qml/qmlcomponent.cpp index 0894758..3767695 100644 --- a/src/declarative/qml/qmlcomponent.cpp +++ b/src/declarative/qml/qmlcomponent.cpp @@ -55,6 +55,7 @@ #include "qmlbinding.h" #include #include +#include #include "qmlscriptparser_p.h" @@ -197,6 +198,12 @@ QmlComponent::QmlComponent(QObject *parent) QmlComponent::~QmlComponent() { Q_D(QmlComponent); + + if (d->completePending) { + qWarning("QmlComponent: Component destroyed while completion pending"); + d->completeCreate(); + } + if (d->typeData) { d->typeData->remWaiter(d); d->typeData->release(); @@ -574,8 +581,10 @@ QmlComponentPrivate::beginCreate(QmlContext *context, const QBitField &bindings) ep->bindValues.clear(); ep->parserStatus.clear(); completePending = true; + QmlEnginePrivate::get(engine)->inProgressCreations++; } + if (rv) { QFx_setParent_noEvent(ctxt, rv); } else { @@ -643,6 +652,14 @@ void QmlComponentPrivate::completeCreate() bindValues.clear(); parserStatus.clear(); completePending = false; + QmlEnginePrivate *p = QmlEnginePrivate::get(engine); + p->inProgressCreations--; + if (0 == p->inProgressCreations) { + while (p->erroredBindings) { + qWarning().nospace() << qPrintable(p->erroredBindings->error.toString()); + p->erroredBindings->removeError(); + } + } } } diff --git a/src/declarative/qml/qmlengine.cpp b/src/declarative/qml/qmlengine.cpp index 0e239ce..3da8aa9 100644 --- a/src/declarative/qml/qmlengine.cpp +++ b/src/declarative/qml/qmlengine.cpp @@ -126,8 +126,9 @@ static QString userLocalDataPath(const QString& app) QmlEnginePrivate::QmlEnginePrivate(QmlEngine *e) : rootContext(0), currentExpression(0), isDebugging(false), contextClass(0), objectClass(0), valueTypeClass(0), globalClass(0), - nodeListClass(0), namedNodeMapClass(0), sqlQueryClass(0), cleanup(0), scriptEngine(this), - componentAttacheds(0), rootComponent(0), networkAccessManager(0), typeManager(e), uniqueId(1) + nodeListClass(0), namedNodeMapClass(0), sqlQueryClass(0), cleanup(0), erroredBindings(0), + inProgressCreations(0), scriptEngine(this), componentAttacheds(0), rootComponent(0), + networkAccessManager(0), typeManager(e), uniqueId(1) { QScriptValue qtObject = scriptEngine.newQMetaObject(StaticQtMetaObject::get()); diff --git a/src/declarative/qml/qmlengine_p.h b/src/declarative/qml/qmlengine_p.h index 69b121e..efd26bf 100644 --- a/src/declarative/qml/qmlengine_p.h +++ b/src/declarative/qml/qmlengine_p.h @@ -98,6 +98,7 @@ class QmlTypeNameCache; class QmlComponentAttached; class QmlListScriptClass; class QmlCleanup; +class QmlBindingData; class QmlEnginePrivate : public QObjectPrivate { @@ -143,6 +144,10 @@ public: // Registered cleanup handlers QmlCleanup *cleanup; + // Bindings that have had errors during startup + QmlBindingData *erroredBindings; + int inProgressCreations; + struct QmlScriptEngine : public QScriptEngine { QmlScriptEngine(QmlEnginePrivate *priv) diff --git a/src/declarative/qml/qmlexpression.cpp b/src/declarative/qml/qmlexpression.cpp index 8231bf8..e5e5cf9 100644 --- a/src/declarative/qml/qmlexpression.cpp +++ b/src/declarative/qml/qmlexpression.cpp @@ -293,6 +293,7 @@ QVariant QmlExpressionPrivate::evalQtScript(QObject *secondaryScope, bool *isUnd QFxPerfTimer perfqt; #endif + QmlExpressionData *data = this->data; QmlContextPrivate *ctxtPriv = data->context()->d_func(); QmlEngine *engine = data->context()->engine(); QmlEnginePrivate *ep = QmlEnginePrivate::get(engine); diff --git a/src/declarative/qml/qmlexpression_p.h b/src/declarative/qml/qmlexpression_p.h index b453a96..4e13de3 100644 --- a/src/declarative/qml/qmlexpression_p.h +++ b/src/declarative/qml/qmlexpression_p.h @@ -83,7 +83,7 @@ class QmlExpressionData : public QmlAbstractExpression, public QmlRefCount { public: QmlExpressionData(); - ~QmlExpressionData(); + virtual ~QmlExpressionData(); QmlExpressionPrivate *q; -- cgit v0.12 From 57d2291b2e0533d23bd52e9814faf6a5727a4e96 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Wed, 28 Oct 2009 15:21:36 +1000 Subject: Test for QT-2373 Ensure that transient binding errors are not displayed. --- .../data/NestedTypeTransientErrors.qml | 11 +++++++++++ .../qmlecmascript/data/transientErrors.qml | 10 ++++++++++ .../qmlecmascript/tst_qmlecmascript.cpp | 23 ++++++++++++++++++++++ 3 files changed, 44 insertions(+) create mode 100644 tests/auto/declarative/qmlecmascript/data/NestedTypeTransientErrors.qml create mode 100644 tests/auto/declarative/qmlecmascript/data/transientErrors.qml diff --git a/tests/auto/declarative/qmlecmascript/data/NestedTypeTransientErrors.qml b/tests/auto/declarative/qmlecmascript/data/NestedTypeTransientErrors.qml new file mode 100644 index 0000000..e19bf24 --- /dev/null +++ b/tests/auto/declarative/qmlecmascript/data/NestedTypeTransientErrors.qml @@ -0,0 +1,11 @@ +import Qt 4.6 + +Object { + property int b: obj.prop.a + + property var prop; + prop: Object { + property int a: 10 + } +} + diff --git a/tests/auto/declarative/qmlecmascript/data/transientErrors.qml b/tests/auto/declarative/qmlecmascript/data/transientErrors.qml new file mode 100644 index 0000000..4e123c6 --- /dev/null +++ b/tests/auto/declarative/qmlecmascript/data/transientErrors.qml @@ -0,0 +1,10 @@ +import Qt 4.6 + +Object { + property var obj: nested + + property var obj2 + obj2: NestedTypeTransientErrors { + id: nested + } +} diff --git a/tests/auto/declarative/qmlecmascript/tst_qmlecmascript.cpp b/tests/auto/declarative/qmlecmascript/tst_qmlecmascript.cpp index 67a98b1..48c2249 100644 --- a/tests/auto/declarative/qmlecmascript/tst_qmlecmascript.cpp +++ b/tests/auto/declarative/qmlecmascript/tst_qmlecmascript.cpp @@ -66,6 +66,7 @@ private slots: void signalTriggeredBindings(); void listProperties(); void exceptionClearsOnReeval(); + void transientErrors(); private: QmlEngine engine; @@ -847,6 +848,28 @@ void tst_qmlecmascript::exceptionClearsOnReeval() QCOMPARE(object->property("test").toBool(), true); } +static int transientErrorsMsgCount = 0; +static void transientErrorsMsgHandler(QtMsgType, const char *) +{ + ++transientErrorsMsgCount; +} + +// Check that transient binding errors are not displayed +void tst_qmlecmascript::transientErrors() +{ + QmlComponent component(&engine, TEST_FILE("transientErrors.qml")); + + transientErrorsMsgCount = 0; + QtMsgHandler old = qInstallMsgHandler(transientErrorsMsgHandler); + + QObject *object = component.create(); + QVERIFY(object != 0); + + qInstallMsgHandler(old); + + QCOMPARE(transientErrorsMsgCount, 0); +} + QTEST_MAIN(tst_qmlecmascript) #include "tst_qmlecmascript.moc" -- cgit v0.12