Improve QJSValueIterator implementation.

The old implementation was a hack, it had some memory leak (in case of
deleted engine) and performance problems (for example all names were
copied to separate QList instance instead of reusing v8::Array).

Change-Id: Ic70ad511127a8c05df3c627e4496083004c6452a
Reviewed-on: http://codereview.qt.nokia.com/2512
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Kent Hansen <kent.hansen@nokia.com>

1 files changed, 1 insertions, 137 deletions

diff --git a/src/declarative/qml/v8/qjsvalueiterator.cpp b/src/declarative/qml/v8/qjsvalueiterator.cpp
index ca9123f0c0..42bfd3408c 100644
--- a/src/declarative/qml/v8/qjsvalueiterator.cpp
+++ b/src/declarative/qml/v8/qjsvalueiterator.cpp
@@ -22,6 +22,7 @@
 ****************************************************************************/
 
 #include "qjsvalueiterator.h"
+#include "qjsvalueiterator_p.h"
 
 #include "qscriptisolate_p.h"
 #include "qjsvalue_p.h"
@@ -69,143 +70,6 @@ QT_BEGIN_NAMESPACE
     \sa QJSValue::property()
 */
 
-using v8::Persistent;
-using v8::Local;
-using v8::Array;
-using v8::String;
-using v8::Handle;
-using v8::Object;
-using v8::Value;
-
-// FIXME (Qt5) This class should be refactored. It should use the common Iterator interface.
-// FIXME it could be faster!
-class QJSValueIteratorPrivate {
-public:
-    inline QJSValueIteratorPrivate(const QJSValuePrivate* value);
-    inline ~QJSValueIteratorPrivate();
-
-    inline bool hasNext() const;
-    inline bool next();
-
-    inline QString name() const;
-
-    inline QScriptPassPointer<QJSValuePrivate> value() const;
-
-    inline bool isValid() const;
-    inline QV8Engine* engine() const;
-private:
-    Q_DISABLE_COPY(QJSValueIteratorPrivate)
-    //void dump(QString) const;
-
-    QScriptSharedDataPointer<QJSValuePrivate> m_object;
-    QList<v8::Persistent<v8::String> > m_names;
-    QMutableListIterator<v8::Persistent<v8::String> > m_iterator;
-};
-
-inline QJSValueIteratorPrivate::QJSValueIteratorPrivate(const QJSValuePrivate* value)
-    : m_object(const_cast<QJSValuePrivate*>(value))
-    , m_iterator(m_names)
-{
-    Q_ASSERT(value);
-    QV8Engine *engine = m_object->engine();
-    QScriptIsolate api(engine);
-    if (!m_object->isObject())
-        m_object = 0;
-    else {
-        v8::HandleScope scope;
-        Handle<Value> tmp = *value;
-        Handle<Object> obj = Handle<Object>::Cast(tmp);
-        Local<Array> names;
-
-        // FIXME we need newer V8!
-        //names = obj->GetOwnPropertyNames();
-        names = engine->getOwnPropertyNames(obj);
-
-        // it is suboptimal, it would be better to write iterator instead
-        uint32_t count = names->Length();
-        Local<String> name;
-        m_names.reserve(count); // The count is the maximal count of values.
-        for (uint32_t i = count - 1; i < count; --i) {
-            name = names->Get(i)->ToString();
-            m_names.append(v8::Persistent<v8::String>::New(name));
-        }
-
-        // Reinitialize the iterator.
-        m_iterator = m_names;
-    }
-}
-
-inline QJSValueIteratorPrivate::~QJSValueIteratorPrivate()
-{
-    QMutableListIterator<v8::Persistent<v8::String> > it = m_names;
-    //FIXME: we need register this QJSVAlueIterator
-    if (engine()) {
-        while (it.hasNext()) {
-            it.next().Dispose();
-        }
-    } else {
-        // FIXME leak ?
-    }
-}
-
-inline bool QJSValueIteratorPrivate::hasNext() const
-{
-    //dump("hasNext()");
-    return isValid()
-            ? m_iterator.hasNext() : false;
-}
-
-inline bool QJSValueIteratorPrivate::next()
-{
-    //dump("next();");
-    if (m_iterator.hasNext()) {
-        m_iterator.next();
-        return true;
-    }
-    return false;
-}
-
-inline QString QJSValueIteratorPrivate::name() const
-{
-    //dump("name");
-    if (!isValid())
-        return QString();
-
-    return QJSConverter::toString(m_iterator.value());
-}
-
-inline QScriptPassPointer<QJSValuePrivate> QJSValueIteratorPrivate::value() const
-{
-    //dump("value()");
-    if (!isValid())
-        return InvalidValue();
-
-    return m_object->property(m_iterator.value());
-}
-
-inline bool QJSValueIteratorPrivate::isValid() const
-{
-    bool result = m_object ? m_object->isValid() : false;
-    // We know that if this object is still valid then it is an object
-    // if this assumption is not correct then some other logic in this class
-    // have to be changed too.
-    Q_ASSERT(!result || m_object->isObject());
-    return result;
-}
-
-inline QV8Engine* QJSValueIteratorPrivate::engine() const
-{
-    return m_object ? m_object->engine() : 0;
-}
-
-//void QJSValueIteratorPrivate::dump(QString fname) const
-//{
-//    qDebug() << "    *** " << fname << " ***";
-//    foreach (Persistent<String> name, m_names) {
-//        qDebug() << "        - " << QJSConverter::toString(name);
-//    }
-//}
-
 /*!
     Constructs an iterator for traversing \a object. The iterator is
     set to be at the front of the sequence of properties (before the