Remove more v8::Persistent usages

Change-Id: Ifa1706e2e609ded86a8bc1a840ca2ed36b869098
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>

diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp
index 0c020b7..52777b0 100644 (file)
--- a/src/qml/qml/qqmlcomponent.cpp
+++ b/src/qml/qml/qqmlcomponent.cpp
@@ -99,8 +99,8 @@ public:
     QQmlComponentExtension(QV8Engine *);
     virtual ~QQmlComponentExtension();
 
-    v8::Persistent<v8::Function> incubationConstructor;
-    v8::Persistent<v8::Function> forceCompletion;
+    QV4::PersistentValue incubationConstructor;
+    QV4::PersistentValue forceCompletion;
 };
 V8_DEFINE_EXTENSION(QQmlComponentExtension, componentExtension);
 
@@ -1101,10 +1101,10 @@ public:
 
     void dispose();
 
-    v8::Persistent<v8::Object> me;
+    QV4::PersistentValue me;
     QQmlGuard<QObject> parent;
-    v8::Persistent<v8::Value> valuemap;
-    v8::Persistent<v8::Object> qmlGlobal;
+    QV4::PersistentValue valuemap;
+    QV4::PersistentValue qmlGlobal;
 protected:
     virtual void statusChanged(Status);
     virtual void setInitialState(QObject *);
@@ -1333,15 +1333,15 @@ void QQmlComponent::incubateObject(QQmlV8Function *args)
     QQmlComponentExtension *e = componentExtension(args->engine());
     
     QV8IncubatorResource *r = new QV8IncubatorResource(args->engine(), mode);
-    v8::Handle<v8::Object> o = e->incubationConstructor->NewInstance();
+    v8::Handle<v8::Object> o = e->incubationConstructor.value().asFunctionObject()->newInstance();
     o->SetExternalResource(r);
 
     if (!valuemap.IsEmpty()) {
-        r->valuemap = qPersistentNew(valuemap);
-        r->qmlGlobal = qPersistentNew(args->qmlGlobal());
+        r->valuemap = valuemap->v4Value();
+        r->qmlGlobal = args->qmlGlobal()->v4Value();
     }
     r->parent = parent;
-    r->me = qPersistentNew(o);
+    r->me = o->v4Value();
 
     create(*r, creationContext());
 
@@ -1375,7 +1375,7 @@ void QQmlComponentPrivate::initializeObjectWithInitialProperties(v8::Handle<v8::
 
 QQmlComponentExtension::QQmlComponentExtension(QV8Engine *engine)
 {
-    forceCompletion = qPersistentNew(V8FUNCTION(QV8IncubatorResource::ForceCompletion, engine));
+    forceCompletion = (V8FUNCTION(QV8IncubatorResource::ForceCompletion, engine))->v4Value();
 
     {
     v8::Handle<v8::FunctionTemplate> ft = v8::FunctionTemplate::New();
@@ -1390,7 +1390,7 @@ QQmlComponentExtension::QQmlComponentExtension(QV8Engine *engine)
                                         QV8IncubatorResource::ObjectGetter); 
     ft->InstanceTemplate()->SetAccessor(v8::String::New("forceCompletion"), 
                                         QV8IncubatorResource::ForceCompletionGetter); 
-    incubationConstructor = qPersistentNew(ft->GetFunction());
+    incubationConstructor = ft->GetFunction()->v4Value();
     }
 }
 
@@ -1405,7 +1405,7 @@ v8::Handle<v8::Value> QV8IncubatorResource::ForceCompletionGetter(v8::Handle<v8:
                                                                   const v8::AccessorInfo& info)
 {
     QV8IncubatorResource *r = v8_resource_check<QV8IncubatorResource>(info.This());
-    return componentExtension(r->engine)->forceCompletion;
+    return componentExtension(r->engine)->forceCompletion.value();
 }
 
 QV4::Value QV8IncubatorResource::ForceCompletion(const v8::Arguments &args)
@@ -1440,8 +1440,6 @@ void QV8IncubatorResource::StatusChangedSetter(v8::Handle<v8::String>, v8::Handl
 
 QQmlComponentExtension::~QQmlComponentExtension()
 {
-    qPersistentDispose(incubationConstructor);
-    qPersistentDispose(forceCompletion);
 }
 
 QV8IncubatorResource::QV8IncubatorResource(QV8Engine *engine, IncubationMode m)
@@ -1453,23 +1451,19 @@ void QV8IncubatorResource::setInitialState(QObject *o)
 {
     QQmlComponent_setQmlParent(o, parent);
 
-    if (!valuemap.IsEmpty()) {
+    if (!valuemap.isEmpty()) {
         QQmlComponentExtension *e = componentExtension(engine);
 
         QV4::ExecutionEngine *v4engine = QV8Engine::getV4(engine);
 
-        QV4::Value f = engine->evaluateScript(QString::fromLatin1(INITIALPROPERTIES_SOURCE), qmlGlobal->v4Value().asObject());
-        QV4::Value args[] = { engine->newQObject(o), valuemap->v4Value() };
+        QV4::Value f = engine->evaluateScript(QString::fromLatin1(INITIALPROPERTIES_SOURCE), qmlGlobal.value().asObject());
+        QV4::Value args[] = { engine->newQObject(o), valuemap };
         f.asFunctionObject()->call(v4engine->current, QV4::Value::fromObject(v4engine->globalObject), args, 2);
     }
 }
     
 void QV8IncubatorResource::dispose()
 {
-    qPersistentDispose(valuemap);
-    qPersistentDispose(qmlGlobal);
-    // No further status changes are forthcoming, so we no long need a self reference
-    qPersistentDispose(me);
 }
 
 void QV8IncubatorResource::statusChanged(Status s)
@@ -1480,8 +1474,8 @@ void QV8IncubatorResource::statusChanged(Status s)
         QQmlData::get(object())->indestructible = false;
     }
 
-    if (!me.IsEmpty()) { // Will be false in synchronous mode
-        v8::Handle<v8::Value> callback = me->GetInternalField(0);
+    if (!me.isEmpty()) { // Will be false in synchronous mode
+        v8::Handle<v8::Value> callback = v8::Handle<v8::Object>(me)->GetInternalField(0);
 
         if (!callback.IsEmpty() && !callback->IsUndefined()) {
 
@@ -1489,7 +1483,7 @@ void QV8IncubatorResource::statusChanged(Status s)
                 v8::Handle<v8::Function> f = v8::Handle<v8::Function>::Cast(callback);
                 v8::Handle<v8::Value> args[] = { v8::Integer::NewFromUnsigned(s) };
                 v8::TryCatch tc;
-                f->Call(me, 1, args);
+                f->Call(me.value(), 1, args);
                 if (tc.HasCaught()) {
                     QQmlError error;
                     QQmlJavaScriptExpression::exceptionToError(tc.Message(), error);

diff --git a/src/qml/qml/qqmlcontext.cpp b/src/qml/qml/qqmlcontext.cpp
index b865ff0..9b196a6 100644 (file)
--- a/src/qml/qml/qqmlcontext.cpp
+++ b/src/qml/qml/qqmlcontext.cpp
@@ -654,10 +654,6 @@ void QQmlContextData::destroy()
     if (v8bindings)
         v8bindings->release();
 
-    for (int ii = 0; ii < importedScripts.count(); ++ii) {
-        qPersistentDispose(importedScripts[ii]);
-    }
-
     delete [] idValues;
 
     if (isInternal)

diff --git a/src/qml/qml/qqmlcontext_p.h b/src/qml/qml/qqmlcontext_p.h
index 10105bd..ef91c57 100644 (file)
--- a/src/qml/qml/qqmlcontext_p.h
+++ b/src/qml/qml/qqmlcontext_p.h
@@ -164,7 +164,7 @@ public:
     QObject *contextObject;
 
     // Any script blocks that exist on this context
-    QList<v8::Persistent<v8::Object> > importedScripts;
+    QList<QV4::PersistentValue> importedScripts;
 
     // Context base url
     QUrl url;

diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 14f1fef..096e358 100644 (file)
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -57,6 +57,8 @@
 #include <private/qobject_p.h>
 #include <private/qv8_p.h>
 
+#include <private/qv4value_p.h>
+
 QT_BEGIN_NAMESPACE
 
 template <class Key, class T> class QHash;
@@ -177,7 +179,7 @@ public:
     unsigned int deferredIdx;
 
     quint32 v8objectid;
-    v8::Persistent<v8::Object> v8object;
+    QV4::PersistentValue v8object;
 
     QQmlPropertyCache *propertyCache;
 

diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 9a7b218..34114ea 100644 (file)
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -1531,11 +1531,7 @@ void QQmlData::destroyed(QObject *object)
         delete extendedData;
 
     // Dispose the handle.
-    // We don't simply clear it (and wait for next gc cycle to dispose
-    // via the weak qobject reference callback) as this affects the
-    // outcomes of v8's gc statistical analysis heuristics, which can
-    // cause unnecessary growth of the old pointer space js heap area.
-    qPersistentDispose(v8object);
+    v8object = QV4::Value::undefinedValue();
 
     if (ownMemory)
         delete this;

diff --git a/src/qml/qml/qqmlvme.cpp b/src/qml/qml/qqmlvme.cpp
index 8b82376..d97058e 100644 (file)
--- a/src/qml/qml/qqmlvme.cpp
+++ b/src/qml/qml/qqmlvme.cpp
@@ -473,8 +473,6 @@ QObject *QQmlVME::run(QList<QQmlError> *errors,
                 // double dispose.  It is possible we could do this more efficiently using some form of
                 // referencing instead.
                 CTXT->importedScripts = creationContext->importedScripts;
-                for (int ii = 0; ii < CTXT->importedScripts.count(); ++ii)
-                    CTXT->importedScripts[ii] = qPersistentNew<v8::Object>(CTXT->importedScripts[ii]);
             }
         QML_END_INSTR(Init)
 
@@ -1181,12 +1179,12 @@ void QQmlScriptData::initialize(QQmlEngine *engine)
     addref();
 }
 
-v8::Persistent<v8::Object> QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptData *script)
+QV4::PersistentValue QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptData *script)
 {
     if (script->m_loaded)
-        return qPersistentNew<v8::Object>(script->m_value);
+        return script->m_value->v4Value();
 
-    v8::Persistent<v8::Object> rv;
+    QV4::PersistentValue rv;
 
     Q_ASSERT(parentCtxt && parentCtxt->engine);
     QQmlEnginePrivate *ep = QQmlEnginePrivate::get(parentCtxt->engine);
@@ -1221,8 +1219,6 @@ v8::Persistent<v8::Object> QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptD
     } else if (effectiveCtxt) {
         ctxt->imports = effectiveCtxt->imports;
         ctxt->importedScripts = effectiveCtxt->importedScripts;
-        for (int ii = 0; ii < ctxt->importedScripts.count(); ++ii)
-            ctxt->importedScripts[ii] = qPersistentNew<v8::Object>(ctxt->importedScripts[ii]);
     }
 
     if (ctxt->imports) {
@@ -1262,7 +1258,7 @@ v8::Persistent<v8::Object> QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptD
         }
     } 
 
-    rv = qPersistentNew<v8::Object>(qmlglobal);
+    rv = qmlglobal->v4Value();
     if (shared) {
         script->m_value = qPersistentNew<v8::Object>(qmlglobal);
         script->m_loaded = true;

diff --git a/src/qml/qml/qqmlvme_p.h b/src/qml/qml/qqmlvme_p.h
index 62ba47f..b145fa0 100644 (file)
--- a/src/qml/qml/qqmlvme_p.h
+++ b/src/qml/qml/qqmlvme_p.h
@@ -148,7 +148,7 @@ private:
                  , void *const**storeJumpTable = 0
 #endif
                 );
-    v8::Persistent<v8::Object> run(QQmlContextData *, QQmlScriptData *);
+    QV4::PersistentValue run(QQmlContextData *, QQmlScriptData *);
 
 #ifdef QML_THREADED_VME_INTERPRETER
     static void *const*instructionJumpTable();

diff --git a/src/qml/qml/v4/qv4value_p.h b/src/qml/qml/v4/qv4value_p.h
index 5b56c91..3cc651d 100644 (file)
--- a/src/qml/qml/v4/qv4value_p.h
+++ b/src/qml/qml/v4/qv4value_p.h
@@ -561,7 +561,7 @@ struct PersistentValuePrivate
     void deref();
 };
 
-class PersistentValue
+class Q_QML_EXPORT PersistentValue
 {
 public:
     PersistentValue() : d(0) {}

diff --git a/src/qml/qml/v8/qv8contextwrapper.cpp b/src/qml/qml/v8/qv8contextwrapper.cpp
index e366650..cc0e010 100644 (file)
--- a/src/qml/qml/v8/qv8contextwrapper.cpp
+++ b/src/qml/qml/v8/qv8contextwrapper.cpp
@@ -282,7 +282,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::Getter(v8::Handle<v8::String> property,
             if (r.scriptIndex != -1) {
                 int index = r.scriptIndex;
                 if (index < context->importedScripts.count())
-                    return context->importedScripts.at(index);
+                    return context->importedScripts.at(index).value();
                 else
                     return QV4::Value::undefinedValue();
             } else if (r.type) {

diff --git a/src/qml/qml/v8/qv8engine.cpp b/src/qml/qml/v8/qv8engine.cpp
index bf361a3..ef3c041 100644 (file)
--- a/src/qml/qml/v8/qv8engine.cpp
+++ b/src/qml/qml/v8/qv8engine.cpp
@@ -763,7 +763,8 @@ void QV8Engine::setExtensionData(int index, Deletable *data)
     m_extensionData[index] = data;
 }
 
-v8::Persistent<v8::Object> *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeStrong)
+
+QV4::PersistentValue *QV8Engine::findOwnerAndStrength(QObject *object, bool *shouldBeStrong)
 {
     QQmlData *data = QQmlData::get(object);
     if (data && data->rootObjectInCreation) { // When the object is still being created it may not show up to the GC.
@@ -806,11 +807,13 @@ void QV8Engine::addRelationshipForGC(QObject *object, v8::Persistent<v8::Value>
         return;
 
     bool handleShouldBeStrong = false;
-    v8::Persistent<v8::Object> *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong);
+    QV4::PersistentValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong);
     if (handleShouldBeStrong) {
         v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1);
-    } else if (!implicitOwner->IsEmpty()) {
-        v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1);
+    } else if (!implicitOwner->isEmpty()) {
+        // ### FIXME
+        qWarning() << "Fix object ownership";
+//        v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1);
     }
 }
 
@@ -820,14 +823,16 @@ void QV8Engine::addRelationshipForGC(QObject *object, QObject *other)
         return;
 
     bool handleShouldBeStrong = false;
-    v8::Persistent<v8::Object> *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong);
-    v8::Persistent<v8::Value> handle = QQmlData::get(other, true)->v8object;
-    if (handle.IsEmpty()) // no JS data to keep alive.
+    QV4::PersistentValue *implicitOwner = findOwnerAndStrength(object, &handleShouldBeStrong);
+    QV4::PersistentValue handle = QQmlData::get(other, true)->v8object;
+    if (handle.isEmpty()) // no JS data to keep alive.
         return;
-    else if (handleShouldBeStrong)
-        v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1);
-    else if (!implicitOwner->IsEmpty())
-        v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1);
+    // ### FIXME
+    qWarning() << "Fix object ownership";
+//    else if (handleShouldBeStrong)
+//        v8::V8::AddImplicitReferences(m_strongReferencer, &handle, 1);
+//    else if (!implicitOwner->IsEmpty())
+//        v8::V8::AddImplicitReferences(*implicitOwner, &handle, 1);
 }
 
 static QThreadStorage<QV8Engine::ThreadData*> perThreadEngineData;

diff --git a/src/qml/qml/v8/qv8engine_p.h b/src/qml/qml/v8/qv8engine_p.h
index 7d3bd21..b0dff7e 100644 (file)
--- a/src/qml/qml/v8/qv8engine_p.h
+++ b/src/qml/qml/v8/qv8engine_p.h
@@ -473,7 +473,7 @@ private:
     QVariantMap variantMapFromJS(QV4::Object *object, V8ObjectSet &visitedObjects);
     QVariant variantFromJS(const QV4::Value &value, V8ObjectSet &visitedObjects);
 
-    static v8::Persistent<v8::Object> *findOwnerAndStrength(QObject *object, bool *shouldBeStrong);
+    static QV4::PersistentValue *findOwnerAndStrength(QObject *object, bool *shouldBeStrong);
 
     Q_DISABLE_COPY(QV8Engine)
 };

diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index 86ae6a1..7120237 100644 (file)
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -1101,17 +1101,18 @@ v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
     if (!ddata) 
         return QV4::Value::undefinedValue();
 
-    if (ddata->v8objectid == m_id && !ddata->v8object.IsEmpty()) {
+    if (ddata->v8objectid == m_id && !ddata->v8object.isEmpty()) {
         // We own the v8object 
-        return ddata->v8object;
-    } else if (ddata->v8object.IsEmpty() && 
+        return ddata->v8object.value();
+    } else if (ddata->v8object.isEmpty() &&
                (ddata->v8objectid == m_id || // We own the QObject
                 ddata->v8objectid == 0 ||    // No one owns the QObject
                 !ddata->hasTaintedV8Object)) { // Someone else has used the QObject, but it isn't tainted
 
         v8::Handle<v8::Object> rv = newQObject(object, ddata, m_engine);
-        ddata->v8object = qPersistentNew<v8::Object>(rv);
-        ddata->v8object.MakeWeak(this, WeakQObjectReferenceCallback);
+        ddata->v8object = rv->v4Value();
+        // ### FIXME
+        //ddata->v8object.MakeWeak(this, WeakQObjectReferenceCallback);
         ddata->v8objectid = m_id;
         QV8QObjectResource *resource = v8_resource_check<QV8QObjectResource>(rv);
         registerWeakQObjectReference(resource);
@@ -1126,10 +1127,11 @@ v8::Handle<v8::Value> QV8QObjectWrapper::newQObject(QObject *object)
 
         // If our tainted handle doesn't exist or has been collected, and there isn't
         // a handle in the ddata, we can assume ownership of the ddata->v8object
-        if ((!found || (*iter)->v8object.IsEmpty()) && ddata->v8object.IsEmpty()) {
+        if ((!found || (*iter)->v8object.IsEmpty()) && ddata->v8object.isEmpty()) {
             v8::Handle<v8::Object> rv = newQObject(object, ddata, m_engine);
-            ddata->v8object = qPersistentNew<v8::Object>(rv);
-            ddata->v8object.MakeWeak(this, WeakQObjectReferenceCallback);
+            ddata->v8object = rv->v4Value();
+            // ### FIXME
+            //ddata->v8object.MakeWeak(this, WeakQObjectReferenceCallback);
             ddata->v8objectid = m_id;
             QV8QObjectResource *resource = v8_resource_check<QV8QObjectResource>(rv);
             registerWeakQObjectReference(resource);
@@ -1171,7 +1173,7 @@ bool QV8QObjectWrapper::deleteWeakQObject(QV8QObjectResource *resource, bool cal
                 return false;
             }
 
-            ddata->v8object.Clear();
+            ddata->v8object = QV4::PersistentValue();
             if (!object->parent() && !ddata->indestructible) {
                 // This object is notionally destroyed now
                 if (ddata->ownContext && ddata->context)

diff --git a/src/qml/qml/v8/qv8typewrapper.cpp b/src/qml/qml/v8/qv8typewrapper.cpp
index 5a1ffbb..d677afd 100644 (file)
--- a/src/qml/qml/v8/qv8typewrapper.cpp
+++ b/src/qml/qml/v8/qv8typewrapper.cpp
@@ -241,7 +241,7 @@ v8::Handle<v8::Value> QV8TypeWrapper::Getter(v8::Handle<v8::String> property,
             } else if (r.scriptIndex != -1) {
                 int index = r.scriptIndex;
                 if (index < context->importedScripts.count())
-                    return context->importedScripts.at(index);
+                    return context->importedScripts.at(index).value();
             } else if (r.importNamespace) {
                 return v8engine->typeWrapper()->newObject(object, context->imports, r.importNamespace);
             }