Complete the implementation of QScriptValueIterator with JSCore

This new implementation of QScriptValueIterator passes all the tests.

QScriptValueIterator uses an linked list instead of
JSC::PropertyNameArray so the list can be modified by ::remove() and
to be able to add internal properties for the strings and arrays.

Structure::getPropertyNames() has been modified to not show the
property from the prototype.

Reviewed-by: Kent Hansen

6 files changed, 137 insertions, 148 deletions

diff --git a/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.cpp b/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.cpp
index 8e5c398..10efd59 100644
--- a/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.cpp
+++ b/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.cpp
@@ -447,9 +447,9 @@ bool JSObject::getPropertySpecificValue(ExecState*, const Identifier& propertyNa
     return false;
 }
 
-void JSObject::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames, bool includeNonEnumerable)
+void JSObject::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames,  unsigned listedAttributes)
 {
-    m_structure->getPropertyNames(exec, propertyNames, this, includeNonEnumerable);
+    m_structure->getPropertyNames(exec, propertyNames, this, listedAttributes);
 }
 
 bool JSObject::toBoolean(ExecState*) const
diff --git a/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.h b/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.h
index 9838bb1..f390f4a 100644
--- a/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.h
+++ b/src/3rdparty/webkit/JavaScriptCore/runtime/JSObject.h
@@ -117,7 +117,7 @@ namespace JSC {
 
         virtual bool hasInstance(ExecState*, JSValue, JSValue prototypeProperty);
 
-        virtual void getPropertyNames(ExecState*, PropertyNameArray&, bool includeNonEnumerable = false);
+        virtual void getPropertyNames(ExecState*, PropertyNameArray&, unsigned listedAttributes = Structure::Prototype);
 
         virtual JSValue toPrimitive(ExecState*, PreferredPrimitiveType = NoPreference) const;
         virtual bool getPrimitiveNumber(ExecState*, double& number, JSValue& value);
diff --git a/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.cpp b/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.cpp
index 1e4c429..9f6b0c3 100644
--- a/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.cpp
+++ b/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.cpp
@@ -285,9 +285,9 @@ void Structure::materializePropertyMap()
     }
 }
 
-void Structure::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames, JSObject* baseObject, bool includeNonEnumerable)
+void Structure::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames, JSObject* baseObject, unsigned listedAttributes)
 {
-    bool shouldCache = propertyNames.shouldCache() && !(propertyNames.size() || m_isDictionary);
+    bool shouldCache = propertyNames.shouldCache() && !(propertyNames.size() || m_isDictionary) && (listedAttributes & Prototype);
 
     if (shouldCache && m_cachedPropertyNameArrayData) {
         if (m_cachedPropertyNameArrayData->cachedPrototypeChain() == prototypeChain(exec)) {
@@ -296,11 +296,13 @@ void Structure::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNam
         }
         clearEnumerationCache();
     }
-
+    bool includeNonEnumerable = false;
+    if (listedAttributes & NonEnumerable)
+        includeNonEnumerable = true;
     getNamesFromPropertyTable(propertyNames, includeNonEnumerable);
     getNamesFromClassInfoTable(exec, baseObject->classInfo(), propertyNames, includeNonEnumerable);
 
-    if (m_prototype.isObject()) {
+    if ((listedAttributes & Prototype) && m_prototype.isObject()) {
         propertyNames.setShouldCache(false); // No need for our prototypes to waste memory on caching, since they're not being enumerated directly.
         asObject(m_prototype)->getPropertyNames(exec, propertyNames);
     }
diff --git a/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.h b/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.h
index 6c5e82f..0de03a3 100644
--- a/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.h
+++ b/src/3rdparty/webkit/JavaScriptCore/runtime/Structure.h
@@ -50,6 +50,11 @@ namespace JSC {
 
     class Structure : public RefCounted<Structure> {
     public:
+        enum ListedAttribute {
+            NonEnumerable = 1 << 1,
+            Prototype = 1 << 2
+        };
+
         friend class JIT;
         static PassRefPtr<Structure> create(JSValue prototype, const TypeInfo& typeInfo)
         {
@@ -106,7 +111,7 @@ namespace JSC {
             return get(propertyName._ustring.rep(), attributes, specificValue);
         }
 
-        void getPropertyNames(ExecState*, PropertyNameArray&, JSObject*, bool includeNonEnumerable = false);
+        void getPropertyNames(ExecState*, PropertyNameArray&, JSObject*, unsigned listedAttributes = Prototype);
 
         bool hasGetterSetterProperties() const { return m_hasGetterSetterProperties; }
         void setHasGetterSetterProperties(bool hasGetterSetterProperties) { m_hasGetterSetterProperties = hasGetterSetterProperties; }
diff --git a/src/script/api/qscriptvalueiterator.cpp b/src/script/api/qscriptvalueiterator.cpp
index eaa84a4..0092fbd 100644
--- a/src/script/api/qscriptvalueiterator.cpp
+++ b/src/script/api/qscriptvalueiterator.cpp
@@ -47,9 +47,13 @@
 #include "qscriptengine.h"
 #include "qscriptengine_p.h"
 #include "qscriptvalue_p.h"
+#include "qlinkedlist.h"
+
 
 #include "JSObject.h"
 #include "PropertyNameArray.h"
+#include "JSArray.h"
+#include "JSFunction.h"
 
 QT_BEGIN_NAMESPACE
 
@@ -97,27 +101,42 @@ class QScriptValueIteratorPrivate
 {
 public:
     QScriptValueIteratorPrivate()
-        : propertyNames(0), it(0), current(0)
+        : initialized(false)
     {}
-    ~QScriptValueIteratorPrivate()
-    {
-        delete propertyNames;
-    }
     void ensureInitialized()
     {
-        if (propertyNames != 0)
+        if (initialized)
             return;
         QScriptEnginePrivate *eng_p = QScriptEnginePrivate::get(object.engine());
         JSC::ExecState *exec = eng_p->globalExec();
-        propertyNames = new JSC::PropertyNameArray(exec);
-        JSC::asObject(QScriptValuePrivate::get(object)->jscValue)->getPropertyNames(exec, *propertyNames, true);
-        it = propertyNames->begin();
+        JSC::PropertyNameArray propertyNamesArray(exec);
+        propertyNamesArray.setShouldCache(false);
+        JSC::asObject(QScriptValuePrivate::get(object)->jscValue)->getPropertyNames(exec, propertyNamesArray, JSC::Structure::NonEnumerable);
+
+        JSC::PropertyNameArray::const_iterator propertyNamesIt = propertyNamesArray.begin();
+        for(; propertyNamesIt != propertyNamesArray.end(); ++propertyNamesIt) {
+            propertyNames.append(propertyNamesIt->ustring());
+        }
+        if (object.isArray() || (QScriptValuePrivate::get(object)->jscValue.isObject(&JSC::JSArray::info))) {
+            const JSC::JSArray *array = JSC::asArray(QScriptValuePrivate::get(object)->jscValue);
+            const int length = array->length();
+            for (int i = 0; i < length; ++i)
+                propertyNames.append(JSC::UString::from(i));
+        } else if (object.isString() || (QScriptValuePrivate::get(object)->jscValue.isObject(&JSC::StringObject::info))) {
+            const JSC::UString string =  QScriptValuePrivate::get(object)->jscValue.toString(exec);
+            const int length = string.size();
+            for (int i = 0; i < length; ++i)
+                propertyNames.append(JSC::UString::from(i));
+        }
+        it = propertyNames.begin();
+        initialized = true;
     }
 
     QScriptValue object;
-    JSC::PropertyNameArray *propertyNames;
-    JSC::PropertyNameArray::const_iterator it;
-    const JSC::Identifier *current;
+    QLinkedList<JSC::UString> propertyNames;
+    QLinkedList<JSC::UString>::iterator it;
+    QLinkedList<JSC::UString>::iterator current;
+    bool initialized;
 };
 
 /*!
@@ -160,7 +179,7 @@ bool QScriptValueIterator::hasNext() const
         return false;
 
     const_cast<QScriptValueIteratorPrivate*>(d)->ensureInitialized();
-    return d->it != d->propertyNames->end();
+    return d->it != d->propertyNames.end();
 }
 
 /*!
@@ -196,7 +215,7 @@ bool QScriptValueIterator::hasPrevious() const
         return false;
 
     const_cast<QScriptValueIteratorPrivate*>(d)->ensureInitialized();
-    return d->it != d->propertyNames->begin();
+    return d->it != d->propertyNames.begin();
 }
 
 /*!
@@ -229,7 +248,7 @@ void QScriptValueIterator::toFront()
     if (!d)
         return;
     d->ensureInitialized();
-    d->it = d->propertyNames->begin();
+    d->it = d->propertyNames.begin();
 }
 
 /*!
@@ -244,7 +263,7 @@ void QScriptValueIterator::toBack()
     if (!d)
         return;
     d->ensureInitialized();
-    d->it = d->propertyNames->end();
+    d->it = d->propertyNames.end();
 }
 
 /*!
@@ -256,9 +275,9 @@ void QScriptValueIterator::toBack()
 QString QScriptValueIterator::name() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return QString();
-    return QScript::qtStringFromJSCUString(d->current->ustring());
+    return QScript::qtStringFromJSCUString(*d->current);
 }
 
 /*!
@@ -270,7 +289,7 @@ QString QScriptValueIterator::name() const
 QScriptString QScriptValueIterator::scriptName() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return QScriptString();
     return d->object.engine()->toStringHandle(name());
 }
@@ -284,7 +303,7 @@ QScriptString QScriptValueIterator::scriptName() const
 QScriptValue QScriptValueIterator::value() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return QScriptValue();
     return d->object.property(name());
 }
@@ -298,7 +317,7 @@ QScriptValue QScriptValueIterator::value() const
 void QScriptValueIterator::setValue(const QScriptValue &value)
 {
     Q_D(QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return;
     d->object.setProperty(name(), value);
 }
@@ -312,7 +331,7 @@ void QScriptValueIterator::setValue(const QScriptValue &value)
 QScriptValue::PropertyFlags QScriptValueIterator::flags() const
 {
     Q_D(const QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return 0;
     return d->object.propertyFlags(name());
 }
@@ -326,10 +345,10 @@ QScriptValue::PropertyFlags QScriptValueIterator::flags() const
 void QScriptValueIterator::remove()
 {
     Q_D(QScriptValueIterator);
-    if (!d || !d->it)
+    if (!d || !d->initialized)
         return;
     d->object.setProperty(name(), QScriptValue());
-    d->current = 0;
+    d->propertyNames.erase(d->current);
 }
 
 /*!
diff --git a/tests/auto/qscriptvalueiterator/tst_qscriptvalueiterator.cpp b/tests/auto/qscriptvalueiterator/tst_qscriptvalueiterator.cpp
index 673ac70..c0ab36b 100644
--- a/tests/auto/qscriptvalueiterator/tst_qscriptvalueiterator.cpp
+++ b/tests/auto/qscriptvalueiterator/tst_qscriptvalueiterator.cpp
@@ -48,6 +48,8 @@
 //TESTED_CLASS=
 //TESTED_FILES=
 
+Q_DECLARE_METATYPE(QScriptValue);
+
 class tst_QScriptValueIterator : public QObject
 {
     Q_OBJECT
@@ -61,15 +63,14 @@ private slots:
     void iterateForward();
     void iterateBackward_data();
     void iterateBackward();
+    void iterateArray_data();
     void iterateArray();
     void iterateBackAndForth();
     void setValue();
     void remove();
     void iterateString();
     void iterateGetterSetter();
-    void iterateArgumentsObject();
     void assignObjectToIterator();
-    void undefinedBehavior();
 };
 
 tst_QScriptValueIterator::tst_QScriptValueIterator()
@@ -207,49 +208,101 @@ void tst_QScriptValueIterator::iterateBackward()
     QCOMPARE(it.hasNext(), false);
 }
 
+void tst_QScriptValueIterator::iterateArray_data()
+{
+    QTest::addColumn<QStringList>("inputPropertyNames");
+    QTest::addColumn<QStringList>("inputPropertyValues");
+    QTest::addColumn<QStringList>("propertyNames");
+    QTest::addColumn<QStringList>("propertyValues");
+    QTest::newRow("no elements") << QStringList() << QStringList() << QStringList() << QStringList();
+
+
+    QTest::newRow("0=foo, 1=barr")
+        << (QStringList() << "0" << "1")
+        << (QStringList() << "foo" << "bar")
+        << (QStringList() << "0" << "1")
+        << (QStringList() << "foo" << "bar");
+
+
+    QTest::newRow("0=foo, 3=barr")
+        << (QStringList() << "0" << "3")
+        << (QStringList() << "foo" << "bar")
+        << (QStringList() << "0" << "1" << "2" << "3")
+        << (QStringList() << "foo" << "" << "" << "bar");
+}
+
 void tst_QScriptValueIterator::iterateArray()
 {
+    QFETCH(QStringList, inputPropertyNames);
+    QFETCH(QStringList, inputPropertyValues);
+    QFETCH(QStringList, propertyNames);
+    QFETCH(QStringList, propertyValues);
+
     QScriptEngine engine;
     QScriptValue array = engine.newArray();
-    array.setProperty("0", QScriptValue(&engine, 123));
-    array.setProperty("1", QScriptValue(&engine, 456));
-    array.setProperty("2", QScriptValue(&engine, 789));
+    for (int i = 0; i < inputPropertyNames.size(); ++i) {
+        array.setProperty(inputPropertyNames.at(i), inputPropertyValues.at(i));
+    }
+
     int length = array.property("length").toInt32();
-    QCOMPARE(length, 3);
+    QCOMPARE(length, propertyNames.size());
     QScriptValueIterator it(array);
     for (int i = 0; i < length; ++i) {
         QCOMPARE(it.hasNext(), true);
-        QString indexStr = QScriptValue(&engine, i).toString();
         it.next();
-        QCOMPARE(it.name(), indexStr);
-        QCOMPARE(it.flags(), array.propertyFlags(indexStr));
-        QCOMPARE(it.value().strictlyEquals(array.property(indexStr)), true);
+        QCOMPARE(it.name(), propertyNames.at(i));
+        QCOMPARE(it.flags(), array.propertyFlags(propertyNames.at(i)));
+        QVERIFY(it.value().strictlyEquals(array.property(propertyNames.at(i))));
+        QCOMPARE(it.value().toString(), propertyValues.at(i));
     }
     QCOMPARE(it.hasNext(), false);
 
-    QVERIFY(it.hasPrevious());
+    QCOMPARE(it.hasPrevious(), length > 0);
     for (int i = length - 1; i >= 0; --i) {
         it.previous();
-        QString indexStr = QScriptValue(&engine, i).toString();
-        QCOMPARE(it.name(), indexStr);
-        QCOMPARE(it.value().strictlyEquals(array.property(indexStr)), true);
+        QCOMPARE(it.name(), propertyNames.at(i));
+        QCOMPARE(it.flags(), array.propertyFlags(propertyNames.at(i)));
+        QVERIFY(it.value().strictlyEquals(array.property(propertyNames.at(i))));
+        QCOMPARE(it.value().toString(), propertyValues.at(i));
         QCOMPARE(it.hasPrevious(), i > 0);
     }
     QCOMPARE(it.hasPrevious(), false);
 
     // hasNext() and hasPrevious() cache their result; verify that the result is in sync
-    QVERIFY(it.hasNext());
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("0"));
-    QVERIFY(it.hasNext());
-    it.previous();
-    QCOMPARE(it.name(), QString::fromLatin1("0"));
-    QVERIFY(!it.hasPrevious());
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("0"));
-    QVERIFY(it.hasPrevious());
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("1"));
+    if (length > 1) {
+        QVERIFY(it.hasNext());
+        it.next();
+        QCOMPARE(it.name(), QString::fromLatin1("0"));
+        QVERIFY(it.hasNext());
+        it.previous();
+        QCOMPARE(it.name(), QString::fromLatin1("0"));
+        QVERIFY(!it.hasPrevious());
+        it.next();
+        QCOMPARE(it.name(), QString::fromLatin1("0"));
+        QVERIFY(it.hasPrevious());
+        it.next();
+        QCOMPARE(it.name(), QString::fromLatin1("1"));
+    }
+    {
+        // same test as object:
+        QScriptValue originalArray = engine.newArray();
+        for (int i = 0; i < inputPropertyNames.size(); ++i) {
+            originalArray.setProperty(inputPropertyNames.at(i), inputPropertyValues.at(i));
+        }
+        QScriptValue array = originalArray.toObject();
+        int length = array.property("length").toInt32();
+        QCOMPARE(length, propertyNames.size());
+        QScriptValueIterator it(array);
+        for (int i = 0; i < length; ++i) {
+            QCOMPARE(it.hasNext(), true);
+            it.next();
+            QCOMPARE(it.name(), propertyNames.at(i));
+            QCOMPARE(it.flags(), array.propertyFlags(propertyNames.at(i)));
+            QVERIFY(it.value().strictlyEquals(array.property(propertyNames.at(i))));
+            QCOMPARE(it.value().toString(), propertyValues.at(i));
+        }
+        QCOMPARE(it.hasNext(), false);
+    }
 }
 
 void tst_QScriptValueIterator::iterateBackAndForth()
@@ -484,96 +537,6 @@ void tst_QScriptValueIterator::iterateGetterSetter()
     }
 }
 
-static QScriptValue getArgumentsObject(QScriptContext *ctx, QScriptEngine *)
-{
-    return ctx->argumentsObject();
-}
-
-void tst_QScriptValueIterator::iterateArgumentsObject()
-{
-    QScriptEngine eng;
-    QScriptValue fun = eng.newFunction(getArgumentsObject);
-    QScriptValue ret = fun.call(QScriptValue(), QScriptValueList() << QScriptValue(&eng, 123) << QScriptValue(&eng, 456));
-    QCOMPARE(ret.property("length").toInt32(), 2);
-
-    QScriptValueIterator it(ret);
-    QVERIFY(it.hasNext());
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("callee"));
-    QVERIFY(it.value().isFunction());
-    QVERIFY(it.value().strictlyEquals(fun));
-    QVERIFY(it.hasNext());
-
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("length"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 2);
-    QVERIFY(it.hasNext());
-
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("0"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 123);
-    QVERIFY(it.hasNext());
-
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("1"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 456);
-    QVERIFY(!it.hasNext());
-
-    QVERIFY(it.hasPrevious());
-    it.previous();
-    QCOMPARE(it.name(), QString::fromLatin1("1"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 456);
-    QVERIFY(it.hasPrevious());
-
-    it.previous();
-    QCOMPARE(it.name(), QString::fromLatin1("0"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 123);
-    QVERIFY(it.hasPrevious());
-
-    it.previous();
-    QCOMPARE(it.name(), QString::fromLatin1("length"));
-    QVERIFY(it.value().isNumber());
-    QCOMPARE(it.value().toInt32(), 2);
-    QVERIFY(it.hasPrevious());
-
-    it.previous();
-    QCOMPARE(it.name(), QString::fromLatin1("callee"));
-    QVERIFY(it.value().isFunction());
-    QVERIFY(it.value().strictlyEquals(fun));
-    QVERIFY(!it.hasPrevious());
-}
-
-void tst_QScriptValueIterator::undefinedBehavior()
-{
-    QScriptEngine eng;
-    QScriptValue obj = eng.newObject();
-    obj.setProperty("foo", QScriptValue(&eng, 123));
-
-    QScriptValueIterator it(obj);
-    QVERIFY(it.hasNext());
-
-    // delete the property
-    obj.setProperty("foo", QScriptValue());
-
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("foo"));
-    QVERIFY(!it.value().isValid());
-
-    QVERIFY(!it.hasNext());
-    // add a property
-    obj.setProperty("bar", QScriptValue(&eng, 123));
-    QVERIFY(it.hasNext());
-
-    it.next();
-    QCOMPARE(it.name(), QString::fromLatin1("bar"));
-    QVERIFY(it.value().isNumber());
-}
-
 void tst_QScriptValueIterator::assignObjectToIterator()
 {
     QScriptEngine eng;