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 <matthew.vogt@nokia.com>
This commit is contained in:
Glenn Watson 2012-07-18 13:20:00 +10:00 committed by Qt by Nokia
parent 8bb5677f45
commit ec51952908
8 changed files with 250 additions and 4 deletions

View File

@ -64,6 +64,22 @@ void QV8SequenceWrapper::init(QV8Engine *engine)
m_engine = engine;
m_toString = qPersistentNew<v8::Function>(v8::FunctionTemplate::New(ToString)->GetFunction());
m_valueOf = qPersistentNew<v8::Function>(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::Function>(v8::FunctionTemplate::New(Sort)->GetFunction());
m_arrayPrototype = qPersistentNew<v8::Object>(v8::Array::New(1)->GetPrototype()->ToObject()->Clone());
m_arrayPrototype->Set(v8::String::New("sort"), m_sort);
v8::Local<v8::Script> defaultSortCompareScript = v8::Script::Compile(engine->toString(defaultSortString));
m_defaultSortComparer = qPersistentNew<v8::Function>(v8::Handle<v8::Function>(v8::Function::Cast(*defaultSortCompareScript->Run())));
v8::Local<v8::FunctionTemplate> 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<v8::Object> QV8SequenceWrapper::newSequence(int sequenceType, QObject
v8::Local<v8::Object> 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<v8::Object> QV8SequenceWrapper::fromVariant(const QVariant& v, bool *s
v8::Local<v8::Object> 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<v8::Value> QV8SequenceWrapper::ValueOfGetter(v8::Local<v8::String> pr
return info.Data();
}
v8::Handle<v8::Value> QV8SequenceWrapper::Sort(const v8::Arguments &args)
{
int argCount = args.Length();
if (argCount < 2) {
QV8SequenceResource *sr = v8_resource_cast<QV8SequenceResource>(args.This());
Q_ASSERT(sr);
qint32 length = sr->lengthGetter();
if (length > 1) {
v8::Handle<v8::Function> jsCompareFn = sr->engine->sequenceWrapper()->m_defaultSortComparer;
if (argCount == 1 && args[0]->IsFunction())
jsCompareFn = v8::Handle<v8::Function>(v8::Function::Cast(*args[0]));
sr->sort(jsCompareFn);
}
}
return args.This();
}
v8::Handle<v8::Value> QV8SequenceWrapper::ToString(const v8::Arguments &args)
{
QV8SequenceResource *sr = v8_resource_cast<QV8SequenceResource>(args.This());

View File

@ -61,6 +61,7 @@ QT_BEGIN_NAMESPACE
class QV8Engine;
class QV8ObjectResource;
class QV8SequenceWrapper
{
public:
@ -85,6 +86,9 @@ private:
v8::Persistent<v8::Function> m_constructor;
v8::Persistent<v8::Function> m_toString;
v8::Persistent<v8::Function> m_valueOf;
v8::Persistent<v8::Function> m_sort;
v8::Persistent<v8::Object> m_arrayPrototype;
v8::Persistent<v8::Function> m_defaultSortComparer;
static v8::Handle<v8::Value> IndexedGetter(quint32 index, const v8::AccessorInfo &info);
static v8::Handle<v8::Value> IndexedSetter(quint32 index, v8::Local<v8::Value> value, const v8::AccessorInfo &info);
@ -98,6 +102,7 @@ private:
static v8::Handle<v8::Value> ValueOf(const v8::Arguments &args);
static v8::Handle<v8::Value> Getter(v8::Local<v8::String> property, const v8::AccessorInfo &info);
static v8::Handle<v8::Value> Setter(v8::Local<v8::String> property, v8::Local<v8::Value> value, const v8::AccessorInfo &info);
static v8::Handle<v8::Value> Sort(const v8::Arguments &args);
};

View File

@ -88,6 +88,7 @@ public:
virtual v8::Handle<v8::Boolean> indexedDeleter(quint32 index) = 0;
virtual v8::Handle<v8::Array> indexedEnumerator() = 0;
virtual v8::Handle<v8::Value> toString() = 0;
virtual void sort(v8::Handle<v8::Function> 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<v8::Function> f) : jsFn(f), eng(engine) {} \
bool operator()(SequenceElementType e0, SequenceElementType e1) \
{ \
v8::Handle<v8::Value> argv[2] = { eng->fromVariant(e0), eng->fromVariant(e1) }; \
v8::Handle<v8::Value> compareValue = jsFn->Call(eng->global(), 2, argv); \
return compareValue->NumberValue() < 0; \
} \
private: \
v8::Handle<v8::Function> jsFn; \
QV8Engine *eng; \
}; \
void sort(v8::Handle<v8::Function> jsCompareFunction) \
{ \
CompareFunctor cf(engine, jsCompareFunction); \
qSort(c.begin(), c.end(), cf); \
} \
private: \
QQmlGuard<QObject> object; \
int propertyIndex; \

View File

@ -1772,7 +1772,7 @@ void tst_QQmlDebugJS::getScripts()
QList<QVariant> scripts = value.value("body").toList();
QCOMPARE(scripts.count(), 2);
QCOMPARE(scripts.count(), 3);
}
void tst_QQmlDebugJS::getSource()

View File

@ -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 );
}
}

View File

@ -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<int> integers(QVariant v) const
{
QList<int> intList;
QList<QVariant> vList = v.toList();
for (int i=0 ; i < vList.size() ; ++i) {
int iv = vList[i].toInt();
intList.append(iv);
}
return intList;
}
Q_INVOKABLE QList<qreal> reals(QVariant v) const
{
QList<qreal> realList;
QList<QVariant> vList = v.toList();
for (int i=0 ; i < vList.size() ; ++i) {
qreal fv = vList[i].toReal();
realList.append(fv);
}
return realList;
}
Q_INVOKABLE QList<bool> bools(QVariant v) const
{
QList<bool> boolList;
QList<QVariant> vList = v.toList();
for (int i=0 ; i < vList.size() ; ++i) {
bool bv = vList[i].toBool();
boolList.append(bv);
}
return boolList;
}
};
void registerTypes()
{
qmlRegisterType<MyQmlObject>("Qt.test", 1,0, "MyQmlObjectAlias");
@ -253,6 +293,8 @@ void registerTypes()
qmlRegisterType<FallbackBindingsTypeObject>("Qt.test.fallbackBindingsObject", 1, 0, "FallbackBindingsType");
qmlRegisterType<FallbackBindingsTypeDerived>("Qt.test.fallbackBindingsDerived", 1, 0, "FallbackBindingsType");
qmlRegisterType<MyStringClass>("Qt.test", 1, 0, "MyStringClass");
}
#include "testtypes.moc"

View File

@ -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;

View File

@ -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<QString>("function");
QTest::addColumn<bool>("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"