Add indexed deleter to sequence wrapper, implement length setter

Previously, elements could not be deleted from sequences directly
without reassignment.  This commit adds an indexed deleter which
allows elements to be deleted by specifying an index.  A deleted
element will be replaced with a default-constructed element in the
sequence (slight departure from ECMA262r3 which specifies that it
should be replaced with Undefined).

This commit also implements the length property setter according
to the requirements on Array [[Put]] by ECMA262r3 which allows
removal of elements from a sequence (required for proper behaviour
of Array.prototype methods such as splice() and pop()).

Task-number: QTBUG-22808
Change-Id: I62511b3edc2ec35f92d2a2bd719278e129c98547
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

6 files changed, 156 insertions, 10 deletions

diff --git a/doc/src/declarative/extending.qdoc b/doc/src/declarative/extending.qdoc
index c1c7547ec2..385469c29f 100644
--- a/doc/src/declarative/extending.qdoc
+++ b/doc/src/declarative/extending.qdoc
@@ -252,7 +252,7 @@ In particular, QML currently supports:
   \o \c {QList<int>}
   \o \c {QList<qreal>}
   \o \c {QList<bool>}
-  \o \c {QList<QString>}
+  \o \c {QList<QString>} and \c{QStringList}
   \o \c {QList<QUrl>}
 \endlist
 
@@ -274,6 +274,27 @@ modified directly.
 Other sequence types are not supported transparently, and instead an instance of any other
 sequence type will be passed between QML and C++ as an opaque QVariantList.
 
+\bold {Important Note:} There are some minor differences between the semantics of such
+sequence Array types and default JavaScript Array types which result from the use of a
+C++ storage type in the implementation.  In particular, deleting an element from an Array
+will result in a default-constructed value replacing that element, rather than an
+Undefined value.  Similarly, setting the length property of the Array to a value larger
+than its current value will result in the Array being padded out to the specified length
+with default-constructed elements rather than Undefined elements.
+
+The default-constructed values for each sequence type are as follows:
+\table
+\row \o QList<int> \o integer value 0
+\row \o QList<qreal> \o real value 0.0
+\row \o QList<bool> \o boolean value \c {false}
+\row \o QList<QString> and QStringList \o empty QString
+\row \o QList<QUrl> \o empty QUrl
+\endtable
+
+If you wish to remove elements from a sequence rather than simply replace them with default
+constructed values, do not use the indexed delete operator ("delete sequence[i]") but instead
+use the \c {splice} function ("sequence.splice(startIndex, deleteCount)").
+
 \section1 Inheritance and Coercion
 
 \snippet examples/declarative/cppextensions/referenceexamples/coercion/example.qml 0
diff --git a/src/declarative/qml/v8/qv8sequencewrapper.cpp b/src/declarative/qml/v8/qv8sequencewrapper.cpp
index f63b5da4ea..267e8ec4e6 100644
--- a/src/declarative/qml/v8/qv8sequencewrapper.cpp
+++ b/src/declarative/qml/v8/qv8sequencewrapper.cpp
@@ -66,10 +66,10 @@ void QV8SequenceWrapper::init(QV8Engine *engine)
     m_valueOf = qPersistentNew<v8::Function>(v8::FunctionTemplate::New(ValueOf)->GetFunction());
     v8::Local<v8::FunctionTemplate> ft = v8::FunctionTemplate::New();
     ft->InstanceTemplate()->SetFallbackPropertyHandler(Getter, Setter);
-    ft->InstanceTemplate()->SetIndexedPropertyHandler(IndexedGetter, IndexedSetter, 0, 0, IndexedEnumerator);
-    ft->InstanceTemplate()->SetAccessor(v8::String::New("length"), LengthGetter, 0,
+    ft->InstanceTemplate()->SetIndexedPropertyHandler(IndexedGetter, IndexedSetter, 0, IndexedDeleter, IndexedEnumerator);
+    ft->InstanceTemplate()->SetAccessor(v8::String::New("length"), LengthGetter, LengthSetter,
                                         v8::Handle<v8::Value>(), v8::DEFAULT,
-                                        v8::PropertyAttribute(v8::ReadOnly | v8::DontDelete | v8::DontEnum));
+                                        v8::PropertyAttribute(v8::DontDelete | v8::DontEnum));
     ft->InstanceTemplate()->SetAccessor(v8::String::New("toString"), ToStringGetter, 0,
                                         m_toString, v8::DEFAULT,
                                         v8::PropertyAttribute(v8::ReadOnly | v8::DontDelete | v8::DontEnum));
@@ -184,6 +184,13 @@ v8::Handle<v8::Value> QV8SequenceWrapper::IndexedGetter(quint32 index, const v8:
     return sr->indexedGetter(index);
 }
 
+v8::Handle<v8::Boolean> QV8SequenceWrapper::IndexedDeleter(quint32 index, const v8::AccessorInfo &info)
+{
+    QV8SequenceResource *sr = v8_resource_cast<QV8SequenceResource>(info.This());
+    Q_ASSERT(sr);
+    return sr->indexedDeleter(index);
+}
+
 v8::Handle<v8::Array> QV8SequenceWrapper::IndexedEnumerator(const v8::AccessorInfo &info)
 {
     QV8SequenceResource *sr = v8_resource_cast<QV8SequenceResource>(info.This());
@@ -199,6 +206,14 @@ v8::Handle<v8::Value> QV8SequenceWrapper::LengthGetter(v8::Local<v8::String> pro
     return v8::Integer::NewFromUnsigned(sr->lengthGetter());
 }
 
+void QV8SequenceWrapper::LengthSetter(v8::Local<v8::String> property, v8::Local<v8::Value> value, const v8::AccessorInfo &info)
+{
+    Q_UNUSED(property);
+    QV8SequenceResource *sr = v8_resource_cast<QV8SequenceResource>(info.This());
+    Q_ASSERT(sr);
+    sr->lengthSetter(value);
+}
+
 v8::Handle<v8::Value> QV8SequenceWrapper::ToStringGetter(v8::Local<v8::String> property, const v8::AccessorInfo &info)
 {
     Q_UNUSED(property);
diff --git a/src/declarative/qml/v8/qv8sequencewrapper_p.h b/src/declarative/qml/v8/qv8sequencewrapper_p.h
index da0f7eacca..0ddeed9eda 100644
--- a/src/declarative/qml/v8/qv8sequencewrapper_p.h
+++ b/src/declarative/qml/v8/qv8sequencewrapper_p.h
@@ -88,8 +88,10 @@ private:
 
     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);
+    static v8::Handle<v8::Boolean> IndexedDeleter(quint32 index, const v8::AccessorInfo &info);
     static v8::Handle<v8::Array> IndexedEnumerator(const v8::AccessorInfo &info);
     static v8::Handle<v8::Value> LengthGetter(v8::Local<v8::String> property, const v8::AccessorInfo &info);
+    static void LengthSetter(v8::Local<v8::String> property, v8::Local<v8::Value> value, const v8::AccessorInfo &info);
     static v8::Handle<v8::Value> ToStringGetter(v8::Local<v8::String> property, const v8::AccessorInfo &info);
     static v8::Handle<v8::Value> ToString(const v8::Arguments &args);
     static v8::Handle<v8::Value> ValueOfGetter(v8::Local<v8::String> property, const v8::AccessorInfo &info);
diff --git a/src/declarative/qml/v8/qv8sequencewrapper_p_p.h b/src/declarative/qml/v8/qv8sequencewrapper_p_p.h
index eea595ed6b..a947d52013 100644
--- a/src/declarative/qml/v8/qv8sequencewrapper_p_p.h
+++ b/src/declarative/qml/v8/qv8sequencewrapper_p_p.h
@@ -82,8 +82,10 @@ public:
     virtual bool isEqual(const QV8SequenceResource *v) = 0;
 
     virtual quint32 lengthGetter() = 0;
+    virtual void lengthSetter(v8::Handle<v8::Value> value) = 0;
     virtual v8::Handle<v8::Value> indexedSetter(quint32 index, v8::Handle<v8::Value> value) = 0;
     virtual v8::Handle<v8::Value> indexedGetter(quint32 index) = 0;
+    virtual v8::Handle<v8::Boolean> indexedDeleter(quint32 index) = 0;
     virtual v8::Handle<v8::Array> indexedEnumerator() = 0;
     virtual v8::Handle<v8::Value> toString() = 0;
 
@@ -286,6 +288,48 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v)
                 } \
                 return c.count(); \
             } \
+            void lengthSetter(v8::Handle<v8::Value> value) \
+            { \
+                /* Get the new required length */ \
+                if (value.IsEmpty() || !value->IsUint32()) \
+                    return; \
+                quint32 newLength = value->Uint32Value(); \
+                /* Read the sequence from the QObject property if we're a reference */ \
+                if (objectType == QV8SequenceResource::Reference) { \
+                    if (!object) \
+                        return; \
+                    void *a[] = { &c, 0 }; \
+                    QMetaObject::metacall(object, QMetaObject::ReadProperty, propertyIndex, a); \
+                } \
+                /* Determine whether we need to modify the sequence */ \
+                quint32 count = c.count(); \
+                if (newLength == count) { \
+                    return; \
+                } else if (newLength > count) { \
+                    /* according to ECMA262r3 we need to insert */ \
+                    /* undefined values increasing length to newLength. */ \
+                    /* We cannot, so we insert default-values instead. */ \
+                    while (newLength > count++) { \
+                        c.append(DefaultValue); \
+                    } \
+                } else { \
+                    /* according to ECMA262r3 we need to remove */ \
+                    /* elements until the sequence is the required length. */ \
+                    while (newLength < count) { \
+                        count--; \
+                        c.removeAt(count); \
+                    } \
+                } \
+                /* write back if required. */ \
+                if (objectType == QV8SequenceResource::Reference) { \
+                    /* write back.  already checked that object is non-null, so skip that check here. */ \
+                    int status = -1; \
+                    QDeclarativePropertyPrivate::WriteFlags flags = QDeclarativePropertyPrivate::DontRemoveBinding; \
+                    void *a[] = { &c, 0, &status, &flags }; \
+                    QMetaObject::metacall(object, QMetaObject::WriteProperty, propertyIndex, a); \
+                } \
+                return; \
+            } \
             v8::Handle<v8::Value> indexedSetter(quint32 index, v8::Handle<v8::Value> value) \
             { \
                 if (objectType == QV8SequenceResource::Reference) { \
@@ -331,6 +375,29 @@ static QString convertUrlToString(QV8Engine *, const QUrl &v)
                     return ConversionToV8fn(engine, c.at(index)); \
                 return v8::Undefined(); \
             } \
+            v8::Handle<v8::Boolean> indexedDeleter(quint32 index) \
+            { \
+                if (objectType == QV8SequenceResource::Reference) { \
+                    if (!object) \
+                        return v8::Boolean::New(false); \
+                    void *a[] = { &c, 0 }; \
+                    QMetaObject::metacall(object, QMetaObject::ReadProperty, propertyIndex, a); \
+                } \
+                if (index < c.count()) { \
+                    /* according to ECMA262r3 it should be Undefined, */ \
+                    /* but we cannot, so we insert a default-value instead. */ \
+                    c.replace(index, DefaultValue); \
+                    if (objectType == QV8SequenceResource::Reference) { \
+                        /* write back.  already checked that object is non-null, so skip that check here. */ \
+                        int status = -1; \
+                        QDeclarativePropertyPrivate::WriteFlags flags = QDeclarativePropertyPrivate::DontRemoveBinding; \
+                        void *a[] = { &c, 0, &status, &flags }; \
+                        QMetaObject::metacall(object, QMetaObject::WriteProperty, propertyIndex, a); \
+                    } \
+                    return v8::Boolean::New(true); \
+                } \
+                return v8::Boolean::New(false); \
+            } \
             v8::Handle<v8::Array> indexedEnumerator() \
             { \
                 if (objectType == QV8SequenceResource::Reference) { \
diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.array.qml b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.array.qml
index 5eaa225708..52abda1e55 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.array.qml
+++ b/tests/auto/declarative/qdeclarativeecmascript/data/sequenceConversion.array.qml
@@ -79,26 +79,67 @@ Item {
         if (msco.intListProperty[199] != 200) success = false;
         if (msco.intListProperty.length != 200) success = false;
 
+        // test indexed deleter
+        msco.intListProperty = [ 1, 2, 3, 4, 5 ];
+        delete msco.intListProperty[-1];
+        expected = [ 1, 2, 3, 4, 5 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
+        delete msco.intListProperty[0];
+        expected = [ 0, 2, 3, 4, 5 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
+        delete msco.intListProperty[2];
+        expected = [ 0, 2, 0, 4, 5 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
+        delete msco.intListProperty[7];
+        expected = [ 0, 2, 0, 4, 5 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
+
         // other operations are defined on the array prototype; see if they work.
+
+        // splice
         msco.intListProperty = [ 0, 1, 2, 3, 4, 5, 6, 7 ];
         msco.intListProperty.splice(1,3, 33, 44, 55, 66);
         expected = [ 0, 33, 44, 55, 66, 4, 5, 6, 7 ];
         if (msco.intListProperty.toString() != expected.toString()) success = false;
+        msco.intListProperty = [ 0, 1, 2, 3, 4, 5, 6, 7 ];
+        msco.intListProperty.splice(1, 3);
+        expected = [ 0, 4, 5, 6, 7 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
 
         msco.qrealListProperty = [ 0.1, 1.1, 2.1, 3.1, 4.1, 5.1, 6.1, 7.1 ];
         msco.qrealListProperty.splice(1,3, 33.33, 44.44, 55.55, 66.66);
         expected = [ 0.1, 33.33, 44.44, 55.55, 66.66, 4.1, 5.1, 6.1, 7.1 ];
         if (msco.qrealListProperty.toString() != expected.toString()) success = false;
+        msco.qrealListProperty = [ 0.1, 1.1, 2.1, 3.1, 4.1, 5.1, 6.1, 7.1 ];
+        msco.qrealListProperty.splice(1,3);
+        expected = [ 0.1, 4.1, 5.1, 6.1, 7.1 ];
+        if (msco.qrealListProperty.toString() != expected.toString()) success = false;
 
         msco.boolListProperty = [ false, true, true, false, false, true, false, true ];
         msco.boolListProperty.splice(1,3, false, true, false, false);
         expected = [ false, false, true, false, false, false, true, false, true ];
         if (msco.boolListProperty.toString() != expected.toString()) success = false;
+        msco.boolListProperty = [ false, true, true, false, false, true, false, true ];
+        msco.boolListProperty.splice(1,3);
+        expected = [ false, false, true, false, true ];
+        if (msco.boolListProperty.toString() != expected.toString()) success = false;
 
         msco.stringListProperty = [ "one", "two", "three", "four", "five", "six", "seven", "eight" ];
         msco.stringListProperty.splice(1,3, "nine", "ten", "eleven", "twelve");
         expected = [ "one", "nine", "ten", "eleven", "twelve", "five", "six", "seven", "eight" ];
         if (msco.stringListProperty.toString() != expected.toString()) success = false;
+        msco.stringListProperty = [ "one", "two", "three", "four", "five", "six", "seven", "eight" ];
+        msco.stringListProperty.splice(0,3);
+        expected = [ "four", "five", "six", "seven", "eight" ];
+        if (msco.stringListProperty.toString() != expected.toString()) success = false;
+
+        // pop
+        msco.intListProperty = [ 0, 1, 2, 3, 4, 5, 6, 7 ];
+        var poppedVal = msco.intListProperty.pop();
+        expected = [ 0, 1, 2, 3, 4, 5, 6 ];
+        if (msco.intListProperty.toString() != expected.toString()) success = false;
+        expected = 7;
+        if (poppedVal != expected) success = false;
     }
 
     property variant variantList: [ 1, 2, 3, 4, 5 ];
diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
index 426f9663d2..a95d4380fa 100644
--- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
+++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp
@@ -4225,14 +4225,14 @@ void tst_qdeclarativeecmascript::sequenceConversionArray()
     QDeclarativeComponent component(&engine, qmlFile);
     QObject *object = component.create();
     QVERIFY(object != 0);
-    //QMetaObject::invokeMethod(object, "indexedAccess");
-    //QVERIFY(object->property("success").toBool());
-    //QMetaObject::invokeMethod(object, "arrayOperations");
-    //QVERIFY(object->property("success").toBool());
+    QMetaObject::invokeMethod(object, "indexedAccess");
+    QVERIFY(object->property("success").toBool());
+    QMetaObject::invokeMethod(object, "arrayOperations");
+    QVERIFY(object->property("success").toBool());
     QMetaObject::invokeMethod(object, "testEqualitySemantics");
     QVERIFY(object->property("success").toBool());
-    //QMetaObject::invokeMethod(object, "testReferenceDeletion");
-    //QCOMPARE(object->property("referenceDeletion").toBool(), true);
+    QMetaObject::invokeMethod(object, "testReferenceDeletion");
+    QCOMPARE(object->property("referenceDeletion").toBool(), true);
     delete object;
 }