From 462edd23f953af6ce37d1c42400e2cb4443f5a24 Mon Sep 17 00:00:00 2001 From: Michael Brasser Date: Thu, 26 Apr 2012 16:12:21 +1000 Subject: [PATCH] More robust tracking of signal handler expression ownership. Reference count the expressions, and improve testing. Change-Id: I810509eae1c7608b367e9ff5f7891a294667a692 Reviewed-by: Chris Adams --- src/qml/debugger/qqmlenginedebugservice.cpp | 2 +- src/qml/qml/qml.pri | 1 + src/qml/qml/qqmlboundsignal.cpp | 70 ++++++++++++++++-- src/qml/qml/qqmlboundsignal_p.h | 15 ++-- src/qml/qml/qqmlboundsignalexpressionpointer_p.h | 83 ++++++++++++++++++++++ src/qml/qml/qqmlproperty.cpp | 35 ++++++--- src/qml/qml/qqmlproperty_p.h | 8 ++- src/qml/qml/qqmlvme.cpp | 2 +- src/quick/util/qquickconnections.cpp | 2 +- src/quick/util/qquickpropertychanges.cpp | 48 +++---------- tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp | 34 ++++----- .../qquickstates/data/signalOverrideCrash4.qml | 22 ++++++ tests/auto/quick/qquickstates/tst_qquickstates.cpp | 23 ++++++ 13 files changed, 263 insertions(+), 82 deletions(-) create mode 100644 src/qml/qml/qqmlboundsignalexpressionpointer_p.h create mode 100644 tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml diff --git a/src/qml/debugger/qqmlenginedebugservice.cpp b/src/qml/debugger/qqmlenginedebugservice.cpp index f837da5..b095e8f 100644 --- a/src/qml/debugger/qqmlenginedebugservice.cpp +++ b/src/qml/debugger/qqmlenginedebugservice.cpp @@ -580,7 +580,7 @@ bool QQmlEngineDebugService::setBinding(int objectId, } else if (hasValidSignal(object, propertyName)) { QQmlBoundSignalExpression *qmlExpression = new QQmlBoundSignalExpression(QQmlContextData::get(context), object, expression.toString(), false, filename, line, column); - QQmlPropertyPrivate::setSignalExpression(property, qmlExpression); + QQmlPropertyPrivate::takeSignalExpression(property, qmlExpression); } else if (property.isProperty()) { QQmlBinding *binding = new QQmlBinding(expression.toString(), false, object, QQmlContextData::get(context), filename, line, column);; binding->setTarget(property); diff --git a/src/qml/qml/qml.pri b/src/qml/qml/qml.pri index fadfc9d..1237740 100644 --- a/src/qml/qml/qml.pri +++ b/src/qml/qml/qml.pri @@ -72,6 +72,7 @@ HEADERS += \ $$PWD/qqmlpropertyvaluesource.h \ $$PWD/qqmlpropertyvalueinterceptor_p.h \ $$PWD/qqmlboundsignal_p.h \ + $$PWD/qqmlboundsignalexpressionpointer_p.h \ $$PWD/qqmlparserstatus.h \ $$PWD/qqmlproxymetaobject_p.h \ $$PWD/qqmlvme_p.h \ diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp index c0475b1..b8b74ac 100644 --- a/src/qml/qml/qqmlboundsignal.cpp +++ b/src/qml/qml/qqmlboundsignal.cpp @@ -233,7 +233,6 @@ QQmlBoundSignal::QQmlBoundSignal(QObject *scope, const QMetaMethod &signal, QQmlBoundSignal::~QQmlBoundSignal() { - delete m_expression; m_expression = 0; delete m_params; } @@ -255,17 +254,32 @@ QQmlBoundSignalExpression *QQmlBoundSignal::expression() const Sets the signal expression to \a e. Returns the current signal expression, or null if there is no signal expression. - The QQmlBoundSignal instance takes ownership of \a e. The caller is - assumes ownership of the returned QQmlExpression. + The QQmlBoundSignal instance adds a reference to \a e. The caller + assumes ownership of the returned QQmlBoundSignalExpression reference. */ -QQmlBoundSignalExpression *QQmlBoundSignal::setExpression(QQmlBoundSignalExpression *e) +QQmlBoundSignalExpressionPointer QQmlBoundSignal::setExpression(QQmlBoundSignalExpression *e) { - QQmlBoundSignalExpression *rv = m_expression; + QQmlBoundSignalExpressionPointer rv = m_expression; m_expression = e; if (m_expression) m_expression->setNotifyOnValueChanged(false); return rv; } +/*! + Sets the signal expression to \a e. Returns the current signal expression, + or null if there is no signal expression. + + The QQmlBoundSignal instance takes ownership of \a e (and does not add a reference). The caller + assumes ownership of the returned QQmlBoundSignalExpression reference. +*/ +QQmlBoundSignalExpressionPointer QQmlBoundSignal::takeExpression(QQmlBoundSignalExpression *e) +{ + QQmlBoundSignalExpressionPointer rv = m_expression; + m_expression.take(e); + if (m_expression) m_expression->setNotifyOnValueChanged(false); + return rv; +} + void QQmlBoundSignal::subscriptionCallback(QQmlNotifierEndpoint *e, void **a) { QQmlBoundSignal *s = static_cast(e); @@ -401,6 +415,52 @@ int QQmlBoundSignalParameters::metaCall(QMetaObject::Call c, int id, void **a) } } +//////////////////////////////////////////////////////////////////////// + +QQmlBoundSignalExpressionPointer::QQmlBoundSignalExpressionPointer(QQmlBoundSignalExpression *o) +: o(o) +{ + if (o) o->addref(); +} + +QQmlBoundSignalExpressionPointer::QQmlBoundSignalExpressionPointer(const QQmlBoundSignalExpressionPointer &other) +: o(other.o) +{ + if (o) o->addref(); +} + +QQmlBoundSignalExpressionPointer::~QQmlBoundSignalExpressionPointer() +{ + if (o) o->release(); +} + +QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::operator=(const QQmlBoundSignalExpressionPointer &other) +{ + if (other.o) other.o->addref(); + if (o) o->release(); + o = other.o; + return *this; +} + +QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::operator=(QQmlBoundSignalExpression *other) +{ + if (other) other->addref(); + if (o) o->release(); + o = other; + return *this; +} + +/*! +Takes ownership of \a other. take() does *not* add a reference, as it assumes ownership +of the callers reference of other. +*/ +QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::take(QQmlBoundSignalExpression *other) +{ + if (o) o->release(); + o = other; + return *this; +} + QT_END_NAMESPACE #include diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h index 7ce45aa..3fb20a1 100644 --- a/src/qml/qml/qqmlboundsignal_p.h +++ b/src/qml/qml/qqmlboundsignal_p.h @@ -57,20 +57,21 @@ #include #include +#include #include #include +#include #include QT_BEGIN_NAMESPACE -class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression : public QQmlAbstractExpression, public QQmlJavaScriptExpression +class Q_QML_PRIVATE_EXPORT QQmlBoundSignalExpression : public QQmlAbstractExpression, public QQmlJavaScriptExpression, public QQmlRefCount { public: QQmlBoundSignalExpression(QQmlContextData *ctxt, QObject *scope, const QByteArray &expression, bool isRewritten, const QString &fileName, int line, int column); QQmlBoundSignalExpression(QQmlContextData *ctxt, QObject *scope, const QString &expression, bool isRewritten, const QString &fileName, int line, int column); - ~QQmlBoundSignalExpression(); // "inherited" from QQmlJavaScriptExpression. static QString expressionIdentifier(QQmlJavaScriptExpression *); @@ -87,6 +88,8 @@ public: QQmlEngine *engine() const { return context() ? context()->engine : 0; } private: + ~QQmlBoundSignalExpression(); + v8::Persistent m_v8qmlscope; v8::Persistent m_v8function; @@ -108,7 +111,8 @@ public: virtual int index() const = 0; virtual QQmlBoundSignalExpression *expression() const = 0; - virtual QQmlBoundSignalExpression *setExpression(QQmlBoundSignalExpression *) = 0; + virtual QQmlBoundSignalExpressionPointer setExpression(QQmlBoundSignalExpression *) = 0; + virtual QQmlBoundSignalExpressionPointer takeExpression(QQmlBoundSignalExpression *) = 0; virtual QObject *scope() = 0; void removeFromObject(); @@ -134,7 +138,8 @@ public: int index() const; QQmlBoundSignalExpression *expression() const; - QQmlBoundSignalExpression *setExpression(QQmlBoundSignalExpression *); + QQmlBoundSignalExpressionPointer setExpression(QQmlBoundSignalExpression *); + QQmlBoundSignalExpressionPointer takeExpression(QQmlBoundSignalExpression *); QObject *scope() { return *m_scope; } static void subscriptionCallback(QQmlNotifierEndpoint *e, void **); @@ -142,7 +147,7 @@ public: bool isEvaluating() const { return m_scope.flag(); } private: - QQmlBoundSignalExpression *m_expression; + QQmlBoundSignalExpressionPointer m_expression; QQmlBoundSignalParameters *m_params; // We store some flag bits in the following flag pointer. // m_scope:flag1 - m_isEvaluating diff --git a/src/qml/qml/qqmlboundsignalexpressionpointer_p.h b/src/qml/qml/qqmlboundsignalexpressionpointer_p.h new file mode 100644 index 0000000..cc2106f --- /dev/null +++ b/src/qml/qml/qqmlboundsignalexpressionpointer_p.h @@ -0,0 +1,83 @@ +/**************************************************************************** +** +** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies). +** Contact: http://www.qt-project.org/ +** +** This file is part of the QtQml module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** GNU Lesser General Public License Usage +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this +** file. Please review the following information to ensure the GNU Lesser +** General Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU General +** Public License version 3.0 as published by the Free Software Foundation +** and appearing in the file LICENSE.GPL included in the packaging of this +** file. Please review the following information to ensure the GNU General +** Public License version 3.0 requirements will be met: +** http://www.gnu.org/copyleft/gpl.html. +** +** Other Usage +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** +** +** +** +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H +#define QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// +#include + +QT_BEGIN_NAMESPACE + +class QQmlBoundSignalExpression; +class Q_QML_EXPORT QQmlBoundSignalExpressionPointer +{ +public: + inline QQmlBoundSignalExpressionPointer() : o(0) {} + QQmlBoundSignalExpressionPointer(QQmlBoundSignalExpression *); + QQmlBoundSignalExpressionPointer(const QQmlBoundSignalExpressionPointer &); + ~QQmlBoundSignalExpressionPointer(); + + QQmlBoundSignalExpressionPointer &operator=(const QQmlBoundSignalExpressionPointer &o); + QQmlBoundSignalExpressionPointer &operator=(QQmlBoundSignalExpression *); + + inline QQmlBoundSignalExpression* operator->() const { return o; } + inline QQmlBoundSignalExpression& operator*() const { return *o; } + inline operator QQmlBoundSignalExpression*() const { return o; } + + QQmlBoundSignalExpressionPointer &take(QQmlBoundSignalExpression *); + +private: + QQmlBoundSignalExpression *o; +}; + +QT_END_NAMESPACE + +#endif // QQMLBOUNDSIGNALEXPRESSIONPOINTER_P_H diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp index 0029861..d791d73 100644 --- a/src/qml/qml/qqmlproperty.cpp +++ b/src/qml/qml/qqmlproperty.cpp @@ -943,17 +943,34 @@ QQmlPropertyPrivate::signalExpression(const QQmlProperty &that) /*! Set the signal expression associated with this signal property to \a expr. - Returns the existing signal expression (if any), otherwise 0. + Returns the existing signal expression (if any), otherwise null. - Ownership of \a expr transfers to QML. Ownership of the return value is - assumed by the caller. + A reference to \a expr will be added by QML. Ownership of the return value + reference is assumed by the caller. */ -QQmlBoundSignalExpression * +QQmlBoundSignalExpressionPointer QQmlPropertyPrivate::setSignalExpression(const QQmlProperty &that, QQmlBoundSignalExpression *expr) { + if (expr) + expr->addref(); + return QQmlPropertyPrivate::takeSignalExpression(that, expr); +} + +/*! + Set the signal expression associated with this signal property to \a expr. + Returns the existing signal expression (if any), otherwise null. + + Ownership of \a expr transfers to QML. Ownership of the return value + reference is assumed by the caller. +*/ +QQmlBoundSignalExpressionPointer +QQmlPropertyPrivate::takeSignalExpression(const QQmlProperty &that, + QQmlBoundSignalExpression *expr) +{ if (!(that.type() & QQmlProperty::SignalProperty)) { - delete expr; + if (expr) + expr->release(); return 0; } @@ -967,15 +984,13 @@ QQmlPropertyPrivate::setSignalExpression(const QQmlProperty &that, signalHandler = signalHandler->m_nextSignal; if (signalHandler) - return signalHandler->setExpression(expr); + return signalHandler->takeExpression(expr); if (expr) { QQmlBoundSignal *signal = new QQmlBoundSignal(that.d->object, that.method(), that.d->object); - QQmlBoundSignalExpression *oldExpr = signal->setExpression(expr); - return oldExpr; - } else { - return 0; + signal->takeExpression(expr); } + return 0; } /*! diff --git a/src/qml/qml/qqmlproperty_p.h b/src/qml/qml/qqmlproperty_p.h index e33c95a..cba7849 100644 --- a/src/qml/qml/qqmlproperty_p.h +++ b/src/qml/qml/qqmlproperty_p.h @@ -60,11 +60,11 @@ #include #include #include +#include QT_BEGIN_NAMESPACE class QQmlContext; -class QQmlBoundSignalExpression; class QQmlEnginePrivate; class QQmlJavaScriptExpression; class Q_QML_PRIVATE_EXPORT QQmlPropertyPrivate : public QQmlRefCount @@ -141,8 +141,10 @@ public: QQmlAbstractBinding *, WriteFlags flags = DontRemoveBinding); static QQmlBoundSignalExpression *signalExpression(const QQmlProperty &that); - static QQmlBoundSignalExpression *setSignalExpression(const QQmlProperty &that, - QQmlBoundSignalExpression *) ; + static QQmlBoundSignalExpressionPointer setSignalExpression(const QQmlProperty &that, + QQmlBoundSignalExpression *); + static QQmlBoundSignalExpressionPointer takeSignalExpression(const QQmlProperty &that, + QQmlBoundSignalExpression *); static bool write(const QQmlProperty &that, const QVariant &, WriteFlags); static bool writeBinding(QObject *, const QQmlPropertyData &, QQmlContextData *context, diff --git a/src/qml/qml/qqmlvme.cpp b/src/qml/qml/qqmlvme.cpp index 47ea690..6047688 100644 --- a/src/qml/qml/qqmlvme.cpp +++ b/src/qml/qml/qqmlvme.cpp @@ -739,7 +739,7 @@ QObject *QQmlVME::run(QList *errors, QQmlBoundSignal *bs = new QQmlBoundSignal(target, signal, target); QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(CTXT, context, DATAS.at(instr.value), true, COMP->name, instr.line, instr.column); - bs->setExpression(expr); + bs->takeExpression(expr); QML_END_INSTR(StoreSignal) QML_BEGIN_INSTR(StoreImportedScript) diff --git a/src/quick/util/qquickconnections.cpp b/src/quick/util/qquickconnections.cpp index 8116ac3..1740224 100644 --- a/src/quick/util/qquickconnections.cpp +++ b/src/quick/util/qquickconnections.cpp @@ -292,7 +292,7 @@ void QQuickConnections::connectSignals() QQmlBoundSignalExpression *expression = ctxtdata ? new QQmlBoundSignalExpression(ctxtdata, 0, script, true, location, line, column) : 0; - signal->setExpression(expression); + signal->takeExpression(expression); d->boundsignals += signal; } else { if (!d->ignoreUnknownSignals) diff --git a/src/quick/util/qquickpropertychanges.cpp b/src/quick/util/qquickpropertychanges.cpp index 49df009..e2486d3 100644 --- a/src/quick/util/qquickpropertychanges.cpp +++ b/src/quick/util/qquickpropertychanges.cpp @@ -139,44 +139,23 @@ QT_BEGIN_NAMESPACE class QQuickReplaceSignalHandler : public QQuickActionEvent { public: - QQuickReplaceSignalHandler() : expression(0), reverseExpression(0), - rewindExpression(0), ownedExpression(0), ownedExpressionWatcher(0) {} - ~QQuickReplaceSignalHandler() { - delete ownedExpression; - } + QQuickReplaceSignalHandler() {} + ~QQuickReplaceSignalHandler() {} virtual EventType type() const { return SignalHandler; } QQmlProperty property; - QQmlBoundSignalExpression *expression; - QQmlBoundSignalExpression *reverseExpression; - QQmlBoundSignalExpression *rewindExpression; - QQmlBoundSignalExpression *ownedExpression; - QQmlAbstractExpression::DeleteWatcher *ownedExpressionWatcher; // TODO: refactor the ownership impl. + QQmlBoundSignalExpressionPointer expression; + QQmlBoundSignalExpressionPointer reverseExpression; + QQmlBoundSignalExpressionPointer rewindExpression; virtual void execute(Reason) { - ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, expression); - if (ownedExpression == expression) { - delete ownedExpressionWatcher; - ownedExpressionWatcher = 0; - ownedExpression = 0; - } else if (ownedExpression) { - delete ownedExpressionWatcher; - ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression); - } + QQmlPropertyPrivate::setSignalExpression(property, expression); } virtual bool isReversable() { return true; } virtual void reverse(Reason) { - ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, reverseExpression); - if (ownedExpression == reverseExpression) { - delete ownedExpressionWatcher; - ownedExpressionWatcher = 0; - ownedExpression = 0; - } else if (ownedExpression) { - delete ownedExpressionWatcher; - ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression); - } + QQmlPropertyPrivate::setSignalExpression(property, reverseExpression); } virtual void saveOriginals() { @@ -192,18 +171,10 @@ public: if (rsh == this) return; reverseExpression = rsh->reverseExpression; - if (rsh->ownedExpression == reverseExpression) { - ownedExpression = rsh->ownedExpression; - rsh->ownedExpression = 0; - delete ownedExpressionWatcher; - ownedExpressionWatcher = new QQmlAbstractExpression::DeleteWatcher(ownedExpression); - } } virtual void rewind() { - ownedExpression = QQmlPropertyPrivate::setSignalExpression(property, rewindExpression); - if (ownedExpression == rewindExpression) - ownedExpression = 0; + QQmlPropertyPrivate::setSignalExpression(property, rewindExpression); } virtual void saveCurrentValues() { rewindExpression = QQmlPropertyPrivate::signalExpression(property); @@ -370,7 +341,7 @@ void QQuickPropertyChangesPrivate::decode() QQuickReplaceSignalHandler *handler = new QQuickReplaceSignalHandler; handler->property = prop; - handler->expression = new QQmlBoundSignalExpression(QQmlContextData::get(qmlContext(q)), object, expression, false, url.toString(), line, column); + handler->expression.take(new QQmlBoundSignalExpression(QQmlContextData::get(qmlContext(q)), object, expression, false, url.toString(), line, column)); signalReplacements << handler; } else if (isScript) { // binding QString expression = data.toString(); @@ -390,7 +361,6 @@ void QQuickPropertyChangesPrivate::decode() properties << qMakePair(name, data); } } - decoded = true; data.clear(); } diff --git a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp index d99ed30..a65693b 100644 --- a/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp +++ b/tests/auto/qml/qqmlproperty/tst_qqmlproperty.cpp @@ -181,7 +181,7 @@ void tst_qqmlproperty::qmlmetaproperty() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), -1); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -285,7 +285,7 @@ void tst_qqmlproperty::qmlmetaproperty_object() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), -1); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -335,7 +335,7 @@ void tst_qqmlproperty::qmlmetaproperty_object() QVERIFY(binding != 0); QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty")); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -388,7 +388,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), -1); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -438,7 +438,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string() QVERIFY(binding != 0); QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty")); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -486,7 +486,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(!sigExprWatcher.wasDeleted()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr); QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("clicked()")); @@ -535,7 +535,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(!sigExprWatcher.wasDeleted()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr); QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("oddlyNamedNotifySignal()")); @@ -589,7 +589,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_context() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), -1); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -639,7 +639,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_context() QVERIFY(binding != 0); QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty")); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -692,7 +692,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), -1); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -742,7 +742,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context() QVERIFY(binding != 0); QVERIFY(QQmlPropertyPrivate::binding(prop) == binding.data()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(sigExprWatcher.wasDeleted()); QCOMPARE(prop.index(), dobject.metaObject()->indexOfProperty("defaultProperty")); QCOMPARE(QQmlPropertyPrivate::valueTypeCoreIndex(prop), -1); @@ -790,7 +790,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(!sigExprWatcher.wasDeleted()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr); QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("clicked()")); @@ -839,7 +839,7 @@ void tst_qqmlproperty::qmlmetaproperty_object_string_context() QVERIFY(QQmlPropertyPrivate::setBinding(prop, binding.data()) == 0); QVERIFY(binding == 0); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == 0); - QVERIFY(QQmlPropertyPrivate::setSignalExpression(prop, sigExpr) == 0); + QVERIFY(QQmlPropertyPrivate::takeSignalExpression(prop, sigExpr) == 0); QVERIFY(!sigExprWatcher.wasDeleted()); QVERIFY(QQmlPropertyPrivate::signalExpression(prop) == sigExpr); QCOMPARE(prop.index(), dobject.metaObject()->indexOfMethod("oddlyNamedNotifySignal()")); @@ -987,7 +987,7 @@ void tst_qqmlproperty::read() QQmlProperty p(&o, "onClicked"); QCOMPARE(p.read(), QVariant()); - QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); + QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p)); QCOMPARE(p.read(), QVariant()); @@ -999,7 +999,7 @@ void tst_qqmlproperty::read() QQmlProperty p(&o, "onPropertyWithNotifyChanged"); QCOMPARE(p.read(), QVariant()); - QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); + QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p)); QCOMPARE(p.read(), QVariant()); @@ -1155,7 +1155,7 @@ void tst_qqmlproperty::write() QQmlProperty p(&o, "onClicked"); QCOMPARE(p.write(QVariant("console.log(1921)")), false); - QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); + QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p)); QCOMPARE(p.write(QVariant("console.log(1921)")), false); @@ -1169,7 +1169,7 @@ void tst_qqmlproperty::write() QQmlProperty p(&o, "onPropertyWithNotifyChanged"); QCOMPARE(p.write(QVariant("console.log(1921)")), false); - QVERIFY(0 == QQmlPropertyPrivate::setSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); + QVERIFY(0 == QQmlPropertyPrivate::takeSignalExpression(p, new QQmlBoundSignalExpression(QQmlContextData::get(engine.rootContext()), 0, QLatin1String("null"), false, QString(), -1, -1))); QVERIFY(0 != QQmlPropertyPrivate::signalExpression(p)); QCOMPARE(p.write(QVariant("console.log(1921)")), false); diff --git a/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml b/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml new file mode 100644 index 0000000..705ad07 --- /dev/null +++ b/tests/auto/quick/qquickstates/data/signalOverrideCrash4.qml @@ -0,0 +1,22 @@ +import QtQuick 2.0 + +Rectangle { + id: root + width: 400; height: 400 + property int someProp + + states: [ + State { + name: "state1" + PropertyChanges { target: root; onSomePropChanged: h1() } + }, + State { + name: "state2" + PropertyChanges { target: root; onSomePropChanged: h2() } + } + ] + + // non-default handlers + function h1() {} + function h2() {} +} diff --git a/tests/auto/quick/qquickstates/tst_qquickstates.cpp b/tests/auto/quick/qquickstates/tst_qquickstates.cpp index 7fd8fc4..1aca63a 100644 --- a/tests/auto/quick/qquickstates/tst_qquickstates.cpp +++ b/tests/auto/quick/qquickstates/tst_qquickstates.cpp @@ -112,6 +112,7 @@ private slots: void signalOverrideCrash(); void signalOverrideCrash2(); void signalOverrideCrash3(); + void signalOverrideCrash4(); void parentChange(); void parentChangeErrors(); void anchorChanges(); @@ -475,6 +476,8 @@ void tst_qquickstates::signalOverride() QQuickItemPrivate::get(rect)->setState("green"); rect->doSomething(); QCOMPARE(rect->color(),QColor("green")); + + delete rect; } { @@ -492,6 +495,8 @@ void tst_qquickstates::signalOverride() QCOMPARE(rect->color(),QColor("blue")); QCOMPARE(innerRect->color(),QColor("green")); QCOMPARE(innerRect->property("extendedColor").value(),QColor("green")); + + delete rect; } } @@ -538,6 +543,24 @@ void tst_qquickstates::signalOverrideCrash3() delete rect; } +void tst_qquickstates::signalOverrideCrash4() +{ + QQmlEngine engine; + QQmlComponent c(&engine, testFileUrl("signalOverrideCrash4.qml")); + QQuickRectangle *rect = qobject_cast(c.create()); + QVERIFY(rect != 0); + + QQuickItemPrivate *rectPrivate = QQuickItemPrivate::get(rect); + + rectPrivate->setState("state1"); + rectPrivate->setState("state2"); + rectPrivate->setState("state1"); + rectPrivate->setState("state2"); + rectPrivate->setState(""); + + delete rect; +} + void tst_qquickstates::parentChange() { QQmlEngine engine; -- 2.7.4