From 43c516be86fb0b825b27144a68d837b87c3e1a23 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Fri, 4 Nov 2011 14:13:43 +1000 Subject: [PATCH] Remove unused codepaths from QV8SequenceWrapper The object equality comparison callback does not allow an object with a sequence resource to be equal to an object with a variant resource. As such, the SequenceType::isEqual(QVariant) codepaths are not needed. Also, QVariantList conversion is handled by toBasicVariant() in the QV8Engine, and thus we don't need conversion code for this type in the sequence wrapper. Change-Id: I2ec599c5ad6cfdb715cd4e0aae3f0cc3bb36cfdf Reviewed-by: Aaron Kennedy --- src/declarative/qml/v8/qv8sequencewrapper.cpp | 26 +--------------------- src/declarative/qml/v8/qv8sequencewrapper_p_p.h | 11 --------- .../data/sequenceConversion.threads.qml | 5 +++++ .../declarative/qdeclarativeecmascript/testtypes.h | 12 +++++++--- .../tst_qdeclarativeecmascript.cpp | 4 ++++ 5 files changed, 19 insertions(+), 39 deletions(-) diff --git a/src/declarative/qml/v8/qv8sequencewrapper.cpp b/src/declarative/qml/v8/qv8sequencewrapper.cpp index 61fb993..f63b5da 100644 --- a/src/declarative/qml/v8/qv8sequencewrapper.cpp +++ b/src/declarative/qml/v8/qv8sequencewrapper.cpp @@ -89,19 +89,11 @@ void QV8SequenceWrapper::destroy() qPersistentDispose(m_constructor); } -bool QV8SequenceWrapper::isEqual(QV8ObjectResource *lhs, const QVariant &rhs) -{ - Q_ASSERT(lhs->resourceType() == QV8ObjectResource::SequenceType); - QV8SequenceResource *r = static_cast(lhs); - Q_ASSERT(r); - return r->isEqual(rhs); -} - bool QV8SequenceWrapper::isEqual(QV8ObjectResource *lhs, QV8ObjectResource *rhs) { + Q_ASSERT(lhs && rhs && lhs->resourceType() == QV8ObjectResource::SequenceType && rhs->resourceType() == QV8ObjectResource::SequenceType); QV8SequenceResource *lr = static_cast(lhs); QV8SequenceResource *rr = static_cast(rhs); - Q_ASSERT(lr && rr); return lr->isEqual(rr); } @@ -173,23 +165,7 @@ QVariant QV8SequenceWrapper::toVariant(QV8ObjectResource *r) QVariant QV8SequenceWrapper::toVariant(v8::Handle array, int typeHint, bool *succeeded) { *succeeded = true; - QV8SequenceResource *sr = static_cast(array->GetExternalResource()); - if (sr) - return sr->toVariant(); - - // if no typehint is given it's just a sequence of arbitrary variants uint32_t length = array->Length(); - if (typeHint == -1) { - QVariantList list; - for (uint32_t ii = 0; ii < length; ++ii) { - v8::Local arrayItem = array->Get(ii); - list.append(m_engine->toVariant(arrayItem, -1)); - } - - return list; - } - - // otherwise, try to build a sequence of the correct type FOREACH_QML_SEQUENCE_TYPE(SEQUENCE_TO_VARIANT) { /* else */ *succeeded = false; return QVariant(); } } #undef SEQUENCE_TO_VARIANT diff --git a/src/declarative/qml/v8/qv8sequencewrapper_p_p.h b/src/declarative/qml/v8/qv8sequencewrapper_p_p.h index f609aff..04814e9 100644 --- a/src/declarative/qml/v8/qv8sequencewrapper_p_p.h +++ b/src/declarative/qml/v8/qv8sequencewrapper_p_p.h @@ -79,7 +79,6 @@ public: enum ObjectType { Reference, Copy }; virtual QVariant toVariant() = 0; - virtual bool isEqual(const QVariant &v) = 0; virtual bool isEqual(const QV8SequenceResource *v) = 0; virtual quint32 lengthGetter() = 0; @@ -256,16 +255,6 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v) } \ return QVariant::fromValue(c); \ } \ - bool isEqual(const QVariant &v) \ - { \ - if (objectType == QV8SequenceResource::Reference) { \ - if (!object) \ - return false; \ - void *a[] = { &c, 0 }; \ - QMetaObject::metacall(object, QMetaObject::ReadProperty, propertyIndex, a); \ - } \ - return (c == v.value()); \ - } \ bool isEqual(const QV8SequenceResource *v) \ { \ /* Note: two different sequences can never be equal (even if they */ \ diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.threads.qml b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.threads.qml index 5c4afe0..aefad89 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.threads.qml +++ b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.threads.qml @@ -43,6 +43,11 @@ Item { worker.sendSequence(msco.urlListProperty); } + function testVariantSequence() { + msco.variantListProperty = [ "one", true, 3, "four" ]; + worker.sendSequence(msco.variantListProperty); + } + WorkerScript { id: worker source: "threadScript.js" diff --git a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h index 12563d6..06cc561 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/testtypes.h +++ b/tests/auto/declarative/qdeclarativeecmascript/testtypes.h @@ -1160,9 +1160,10 @@ class MySequenceConversionObject : public QObject Q_PROPERTY (QList boolListProperty READ boolListProperty WRITE setBoolListProperty NOTIFY boolListPropertyChanged) Q_PROPERTY (QList stringListProperty READ stringListProperty WRITE setStringListProperty NOTIFY stringListPropertyChanged) Q_PROPERTY (QList urlListProperty READ urlListProperty WRITE setUrlListProperty NOTIFY urlListPropertyChanged) - Q_PROPERTY (QStringList qstringListProperty READ qstringListProperty WRITE setQStringListProperty NOTIFY qstringListPropertyChanged) + Q_PROPERTY (QList pointListProperty READ pointListProperty WRITE setPointListProperty NOTIFY pointListPropertyChanged) + Q_PROPERTY (QList variantListProperty READ variantListProperty WRITE setVariantListProperty NOTIFY variantListPropertyChanged) public: MySequenceConversionObject() @@ -1173,9 +1174,10 @@ public: m_boolList << true << false << true << false; m_stringList << QLatin1String("first") << QLatin1String("second") << QLatin1String("third") << QLatin1String("fourth"); m_urlList << QUrl("http://www.example1.com") << QUrl("http://www.example2.com") << QUrl("http://www.example3.com"); - m_qstringList << QLatin1String("first") << QLatin1String("second") << QLatin1String("third") << QLatin1String("fourth"); + m_pointList << QPoint(1, 2) << QPoint(3, 4) << QPoint(5, 6); + m_variantList << QVariant(QLatin1String("one")) << QVariant(true) << QVariant(3); } ~MySequenceConversionObject() {} @@ -1196,6 +1198,8 @@ public: void setQStringListProperty(const QStringList &list) { m_qstringList = list; emit qstringListPropertyChanged(); } QList pointListProperty() const { return m_pointList; } void setPointListProperty(const QList &list) { m_pointList = list; emit pointListPropertyChanged(); } + QList variantListProperty() const { return m_variantList; } + void setVariantListProperty(const QList &list) { m_variantList = list; emit variantListPropertyChanged(); } // now for "copy resource" sequences: Q_INVOKABLE QList generateIntSequence() const { QList retn; retn << 1 << 2 << 3; return retn; } @@ -1218,6 +1222,7 @@ signals: void urlListPropertyChanged(); void qstringListPropertyChanged(); void pointListPropertyChanged(); + void variantListPropertyChanged(); private: QList m_intList; @@ -1226,9 +1231,10 @@ private: QList m_boolList; QList m_stringList; QList m_urlList; + QStringList m_qstringList; - QStringList m_qstringList; // not a supported sequence type, but QStringList support is hardcoded. QList m_pointList; // not a supported sequence type + QList m_variantList; // not a supported sequence type, but QVariantList support is hardcoded. }; class MyDeleteObject : public QObject diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index 88e0b2a..cccbdb0 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -4239,6 +4239,10 @@ void tst_qdeclarativeecmascript::sequenceConversionThreads() QTRY_VERIFY(object->property("finished").toBool()); QVERIFY(object->property("success").toBool()); + QMetaObject::invokeMethod(object, "testVariantSequence"); + QTRY_VERIFY(object->property("finished").toBool()); + QVERIFY(object->property("success").toBool()); + delete object; } -- 2.7.4