From ec519529087cc3005d55242569dcbca3dcee91bf Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Wed, 18 Jul 2012 13:20:00 +1000 Subject: Support JS Array.sort() function for sequence wrappers. The V8 natve sort implementation calls some functions that are incompatible with the way sequence wrappers work. In particular, it calls an internal length() function which does not pass through the length accessor provided by sequence wrappers, so the sort function always thinks the array is zero length. Instead, clone the array prototype and override the sort function with one that is specific to sequence wrappers. Task-number: QTBUG-25269 Change-Id: Ic83b9ee0bd3a0707e512f28057f0f99b432fded4 Reviewed-by: Matthew Vogt --- src/qml/qml/v8/qv8sequencewrapper.cpp | 44 +++++++++- src/qml/qml/v8/qv8sequencewrapper_p.h | 5 ++ src/qml/qml/v8/qv8sequencewrapper_p_p.h | 20 +++++ .../qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp | 2 +- .../auto/qml/qqmlecmascript/data/sequenceSort.qml | 95 ++++++++++++++++++++++ tests/auto/qml/qqmlecmascript/testtypes.cpp | 42 ++++++++++ tests/auto/qml/qqmlecmascript/testtypes.h | 2 +- .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 44 ++++++++++ 8 files changed, 250 insertions(+), 4 deletions(-) create mode 100644 tests/auto/qml/qqmlecmascript/data/sequenceSort.qml diff --git a/src/qml/qml/v8/qv8sequencewrapper.cpp b/src/qml/qml/v8/qv8sequencewrapper.cpp index 883ed1b60c..6bd72387b5 100644 --- a/src/qml/qml/v8/qv8sequencewrapper.cpp +++ b/src/qml/qml/v8/qv8sequencewrapper.cpp @@ -64,6 +64,22 @@ void QV8SequenceWrapper::init(QV8Engine *engine) m_engine = engine; m_toString = qPersistentNew(v8::FunctionTemplate::New(ToString)->GetFunction()); m_valueOf = qPersistentNew(v8::FunctionTemplate::New(ValueOf)->GetFunction()); + + QString defaultSortString = QLatin1String( + "(function compare(x,y) {" + " if (x === y) return 0;" + " x = x.toString();" + " y = y.toString();" + " if (x == y) return 0;" + " else return x < y ? -1 : 1;" + "})"); + + m_sort = qPersistentNew(v8::FunctionTemplate::New(Sort)->GetFunction()); + m_arrayPrototype = qPersistentNew(v8::Array::New(1)->GetPrototype()->ToObject()->Clone()); + m_arrayPrototype->Set(v8::String::New("sort"), m_sort); + v8::Local defaultSortCompareScript = v8::Script::Compile(engine->toString(defaultSortString)); + m_defaultSortComparer = qPersistentNew(v8::Handle(v8::Function::Cast(*defaultSortCompareScript->Run()))); + v8::Local ft = v8::FunctionTemplate::New(); ft->InstanceTemplate()->SetFallbackPropertyHandler(Getter, Setter); ft->InstanceTemplate()->SetIndexedPropertyHandler(IndexedGetter, IndexedSetter, 0, IndexedDeleter, IndexedEnumerator); @@ -84,6 +100,9 @@ void QV8SequenceWrapper::init(QV8Engine *engine) void QV8SequenceWrapper::destroy() { + qPersistentDispose(m_defaultSortComparer); + qPersistentDispose(m_sort); + qPersistentDispose(m_arrayPrototype); qPersistentDispose(m_toString); qPersistentDispose(m_valueOf); qPersistentDispose(m_constructor); @@ -122,7 +141,7 @@ v8::Local QV8SequenceWrapper::newSequence(int sequenceType, QObject v8::Local rv = m_constructor->NewInstance(); rv->SetExternalResource(r); - rv->SetPrototype(v8::Array::New(1)->GetPrototype()); + rv->SetPrototype(m_arrayPrototype); return rv; } #undef NEW_REFERENCE_SEQUENCE @@ -145,7 +164,7 @@ v8::Local QV8SequenceWrapper::fromVariant(const QVariant& v, bool *s v8::Local rv = m_constructor->NewInstance(); rv->SetExternalResource(r); - rv->SetPrototype(v8::Array::New(1)->GetPrototype()); + rv->SetPrototype(m_arrayPrototype); return rv; } #undef NEW_COPY_SEQUENCE @@ -227,6 +246,27 @@ v8::Handle QV8SequenceWrapper::ValueOfGetter(v8::Local pr return info.Data(); } +v8::Handle QV8SequenceWrapper::Sort(const v8::Arguments &args) +{ + int argCount = args.Length(); + + if (argCount < 2) { + QV8SequenceResource *sr = v8_resource_cast(args.This()); + Q_ASSERT(sr); + + qint32 length = sr->lengthGetter(); + if (length > 1) { + v8::Handle jsCompareFn = sr->engine->sequenceWrapper()->m_defaultSortComparer; + if (argCount == 1 && args[0]->IsFunction()) + jsCompareFn = v8::Handle(v8::Function::Cast(*args[0])); + + sr->sort(jsCompareFn); + } + } + + return args.This(); +} + v8::Handle QV8SequenceWrapper::ToString(const v8::Arguments &args) { QV8SequenceResource *sr = v8_resource_cast(args.This()); diff --git a/src/qml/qml/v8/qv8sequencewrapper_p.h b/src/qml/qml/v8/qv8sequencewrapper_p.h index 104135ff76..08bc6146f7 100644 --- a/src/qml/qml/v8/qv8sequencewrapper_p.h +++ b/src/qml/qml/v8/qv8sequencewrapper_p.h @@ -61,6 +61,7 @@ QT_BEGIN_NAMESPACE class QV8Engine; class QV8ObjectResource; + class QV8SequenceWrapper { public: @@ -85,6 +86,9 @@ private: v8::Persistent m_constructor; v8::Persistent m_toString; v8::Persistent m_valueOf; + v8::Persistent m_sort; + v8::Persistent m_arrayPrototype; + v8::Persistent m_defaultSortComparer; static v8::Handle IndexedGetter(quint32 index, const v8::AccessorInfo &info); static v8::Handle IndexedSetter(quint32 index, v8::Local value, const v8::AccessorInfo &info); @@ -98,6 +102,7 @@ private: static v8::Handle ValueOf(const v8::Arguments &args); static v8::Handle Getter(v8::Local property, const v8::AccessorInfo &info); static v8::Handle Setter(v8::Local property, v8::Local value, const v8::AccessorInfo &info); + static v8::Handle Sort(const v8::Arguments &args); }; diff --git a/src/qml/qml/v8/qv8sequencewrapper_p_p.h b/src/qml/qml/v8/qv8sequencewrapper_p_p.h index cf20aa39fd..e74a5849cc 100644 --- a/src/qml/qml/v8/qv8sequencewrapper_p_p.h +++ b/src/qml/qml/v8/qv8sequencewrapper_p_p.h @@ -88,6 +88,7 @@ public: virtual v8::Handle indexedDeleter(quint32 index) = 0; virtual v8::Handle indexedEnumerator() = 0; virtual v8::Handle toString() = 0; + virtual void sort(v8::Handle comparer) = 0; ObjectType objectType; QByteArray typeName; @@ -474,6 +475,25 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v) void *a[] = { &c, 0, &status, &flags }; \ QMetaObject::metacall(object, QMetaObject::WriteProperty, propertyIndex, a); \ } \ + class CompareFunctor \ + { \ + public: \ + CompareFunctor(QV8Engine *engine, v8::Handle f) : jsFn(f), eng(engine) {} \ + bool operator()(SequenceElementType e0, SequenceElementType e1) \ + { \ + v8::Handle argv[2] = { eng->fromVariant(e0), eng->fromVariant(e1) }; \ + v8::Handle compareValue = jsFn->Call(eng->global(), 2, argv); \ + return compareValue->NumberValue() < 0; \ + } \ + private: \ + v8::Handle jsFn; \ + QV8Engine *eng; \ + }; \ + void sort(v8::Handle jsCompareFunction) \ + { \ + CompareFunctor cf(engine, jsCompareFunction); \ + qSort(c.begin(), c.end(), cf); \ + } \ private: \ QQmlGuard object; \ int propertyIndex; \ diff --git a/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp b/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp index 5f74f865ec..803c2d8720 100644 --- a/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp +++ b/tests/auto/qml/debugger/qqmldebugjs/tst_qqmldebugjs.cpp @@ -1772,7 +1772,7 @@ void tst_QQmlDebugJS::getScripts() QList scripts = value.value("body").toList(); - QCOMPARE(scripts.count(), 2); + QCOMPARE(scripts.count(), 3); } void tst_QQmlDebugJS::getSource() diff --git a/tests/auto/qml/qqmlecmascript/data/sequenceSort.qml b/tests/auto/qml/qqmlecmascript/data/sequenceSort.qml new file mode 100644 index 0000000000..5e2892a31d --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/sequenceSort.qml @@ -0,0 +1,95 @@ +import QtQuick 2.0 +import Qt.test 1.0 + +Item { + + MyStringClass { + id: msc + } + + function compare(a0, a1) { + var compareOk = a0.length === a1.length; + + if (compareOk === true) { + for (var i=0 ; i < a0.length ; ++i) { + if (a0[i] != a1[i]) { + compareOk = false; + break; + } + } + } + + return compareOk; + } + + function compareStrings(a, b) { + return (a < b) ? 1 : -1; + } + + function compareNumbers(a, b) { + return a - b; + } + + function createExpected(list, sortFn) { + var expected = [] + for (var i=0 ; i < list.length ; ++i) + expected.push(list[i]); + if (sortFn === null) + expected.sort(); + else + expected.sort(sortFn); + return expected; + } + + function checkResults(expected, actual, sortFn) { + if (sortFn === null) + actual.sort(); + else + actual.sort(sortFn); + return compare(expected, actual); + } + + function doStringTest(stringList, fn) { + var expected = createExpected(stringList, fn); + var actual = msc.strings(stringList); + return checkResults(expected, actual, fn); + } + function doIntTest(intList, fn) { + var expected = createExpected(intList, fn); + var actual = msc.integers(intList); + return checkResults(expected, actual, fn); + } + function doRealTest(realList, fn) { + var expected = createExpected(realList, fn); + var actual = msc.reals(realList); + return checkResults(expected, actual, fn); + } + + function test_qtbug_25269(useCustomCompare) { + return doStringTest( [ "one", "two", "three" ], null ); + } + function test_alphabet_insertionSort(useCustomCompare) { + var fn = useCustomCompare ? compareStrings : null; + return doStringTest( [ "z", "y", "M", "a", "c", "B" ], fn ); + } + function test_alphabet_quickSort(useCustomCompare) { + var fn = useCustomCompare ? compareStrings : null; + return doStringTest( [ "z", "y", "m", "a", "c", "B", "gG", "u", "bh", "lk", "GW", "Z", "n", "nm", "oi", "njk", "f", "dd", "ooo", "3des", "num123", "ojh", "lkj", "a6^^", "bl!!", "!o" ], fn ); + } + function test_numbers_insertionSort(useCustomCompare) { + var fn = useCustomCompare ? compareNumbers : null; + return doIntTest( [ 7, 3, 9, 1, 0, -1, 20, -11 ], fn ); + } + function test_numbers_quickSort(useCustomCompare) { + var fn = useCustomCompare ? compareNumbers : null; + return doIntTest( [ 7, 3, 37, 9, 1, 0, -1, 20, -11, -300, -87, 1, 3, -2, 100, 108, 96, 9, 99999, 12, 11, 11, 12, 11, 13, -13, 10, 10, 10, 8, 12 ], fn ); + } + function test_reals_insertionSort(useCustomCompare) { + var fn = useCustomCompare ? compareNumbers : null; + return doRealTest( [ -3.4, 1, 10, 4.23, -30.1, 4.24, 4.21, -1, -1 ], fn ); + } + function test_reals_quickSort(useCustomCompare) { + var fn = useCustomCompare ? compareNumbers : null; + return doRealTest( [ -3.4, 1, 10, 4.23, -30.1, 4.24, 4.21, -1, -1, 12, -100, 87.4, 101.3, -8.88888, 7.76, 10.10, 1.1, -1.1, -0, 11, 12.8, 0.001, -11, -0.75, 99999.99, 11.12, 32.3, 3.333333, 9.876 ], fn ); + } +} diff --git a/tests/auto/qml/qqmlecmascript/testtypes.cpp b/tests/auto/qml/qqmlecmascript/testtypes.cpp index b02e9963ab..77670f74da 100644 --- a/tests/auto/qml/qqmlecmascript/testtypes.cpp +++ b/tests/auto/qml/qqmlecmascript/testtypes.cpp @@ -190,6 +190,46 @@ void MyWorkerObject::doIt() new MyWorkerObjectThread(this); } +class MyStringClass : public QObject +{ + Q_OBJECT +public: + Q_INVOKABLE QStringList strings(QStringList stringList) const + { + return stringList; + } + Q_INVOKABLE QList integers(QVariant v) const + { + QList intList; + QList vList = v.toList(); + for (int i=0 ; i < vList.size() ; ++i) { + int iv = vList[i].toInt(); + intList.append(iv); + } + return intList; + } + Q_INVOKABLE QList reals(QVariant v) const + { + QList realList; + QList vList = v.toList(); + for (int i=0 ; i < vList.size() ; ++i) { + qreal fv = vList[i].toReal(); + realList.append(fv); + } + return realList; + } + Q_INVOKABLE QList bools(QVariant v) const + { + QList boolList; + QList vList = v.toList(); + for (int i=0 ; i < vList.size() ; ++i) { + bool bv = vList[i].toBool(); + boolList.append(bv); + } + return boolList; + } +}; + void registerTypes() { qmlRegisterType("Qt.test", 1,0, "MyQmlObjectAlias"); @@ -253,6 +293,8 @@ void registerTypes() qmlRegisterType("Qt.test.fallbackBindingsObject", 1, 0, "FallbackBindingsType"); qmlRegisterType("Qt.test.fallbackBindingsDerived", 1, 0, "FallbackBindingsType"); + + qmlRegisterType("Qt.test", 1, 0, "MyStringClass"); } #include "testtypes.moc" diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h index d857b64d80..eaecf71ba7 100644 --- a/tests/auto/qml/qqmlecmascript/testtypes.h +++ b/tests/auto/qml/qqmlecmascript/testtypes.h @@ -755,7 +755,7 @@ public: Q_INVOKABLE void method_overload(const QJsonArray &a) { invoke(26); m_actuals << QVariant::fromValue(a); } Q_INVOKABLE void method_overload(const QJsonValue &a) { invoke(27); m_actuals << QVariant::fromValue(a); } - Q_INVOKABLE void method_unknown(MyInvokableObject *o) { invoke(28); } + Q_INVOKABLE void method_unknown(MyInvokableObject *) { invoke(28); } private: friend class MyInvokableBaseObject; diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 4e1638f516..f6ec475d40 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -186,6 +186,8 @@ private slots: void sequenceConversionBindings(); void sequenceConversionCopy(); void assignSequenceTypes(); + void sequenceSort_data(); + void sequenceSort(); void qtbug_22464(); void qtbug_21580(); void singleV8BindingDestroyedDuringEvaluation(); @@ -7121,6 +7123,48 @@ void tst_qqmlecmascript::fallbackBindings() QCOMPARE(object->property("success").toBool(), true); } +void tst_qqmlecmascript::sequenceSort_data() +{ + QTest::addColumn("function"); + QTest::addColumn("useComparer"); + + QTest::newRow("qtbug_25269") << "test_qtbug_25269" << false; + + const char *types[] = { "alphabet", "numbers", "reals" }; + const char *sort[] = { "insertionSort", "quickSort" }; + + for (size_t t=0 ; t < sizeof(types)/sizeof(types[0]) ; ++t) { + for (size_t s=0 ; s < sizeof(sort)/sizeof(sort[0]) ; ++s) { + for (int c=0 ; c < 2 ; ++c) { + QString testName = QLatin1String(types[t]) + QLatin1String("_") + QLatin1String(sort[s]); + QString fnName = QLatin1String("test_") + testName; + bool useComparer = c != 0; + testName += useComparer ? QLatin1String("[custom]") : QLatin1String("[default]"); + QTest::newRow(testName.toAscii().constData()) << fnName << useComparer; + } + } + } +} + +void tst_qqmlecmascript::sequenceSort() +{ + QFETCH(QString, function); + QFETCH(bool, useComparer); + + QQmlComponent component(&engine, testFileUrl("sequenceSort.qml")); + + QObject *object = component.create(); + if (object == 0) + qDebug() << component.errorString(); + QVERIFY(object != 0); + + QVariant q; + QMetaObject::invokeMethod(object, function.toAscii().constData(), Q_RETURN_ARG(QVariant, q), Q_ARG(QVariant, useComparer)); + QVERIFY(q.toBool() == true); + + delete object; +} + QTEST_MAIN(tst_qqmlecmascript) #include "tst_qqmlecmascript.moc" -- cgit v1.2.3