diff options
author | Aaron Kennedy <aaron.kennedy@nokia.com> | 2011-06-09 14:40:44 +1000 |
---|---|---|
committer | Aaron Kennedy <aaron.kennedy@nokia.com> | 2011-06-09 14:40:44 +1000 |
commit | 8bb487e60899382f0890fd675eb272d5cc562882 (patch) | |
tree | 7842e975fd36747fcd612dbae2135243ba9ca4d6 /src/declarative/qml/v8 | |
parent | 4d2e0cebec26b7adcfdad4c4997f7ec22dd72cb8 (diff) |
Minor XXX fixups
Diffstat (limited to 'src/declarative/qml/v8')
-rw-r--r-- | src/declarative/qml/v8/qv8contextwrapper.cpp | 31 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8contextwrapper_p.h | 6 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8engine.cpp | 116 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8engine_p.h | 6 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8include.cpp | 4 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8listwrapper.cpp | 4 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8qobjectwrapper.cpp | 115 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8typewrapper.cpp | 10 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8valuetypewrapper.cpp | 10 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8variantwrapper.cpp | 2 | ||||
-rw-r--r-- | src/declarative/qml/v8/qv8worker.cpp | 10 |
11 files changed, 177 insertions, 137 deletions
diff --git a/src/declarative/qml/v8/qv8contextwrapper.cpp b/src/declarative/qml/v8/qv8contextwrapper.cpp index 27002c6dc0..f0e52b1d64 100644 --- a/src/declarative/qml/v8/qv8contextwrapper.cpp +++ b/src/declarative/qml/v8/qv8contextwrapper.cpp @@ -69,7 +69,12 @@ public: QObject *secondaryScope; - // XXX aakenned - this is somewhat of a horrible abuse of external strings :) + // This is a pretty horrible hack, and an abuse of external strings. When we create a + // sub-context (a context created by a Qt.include() in an external javascript file), + // we pass a specially crafted SubContext external string as the v8::Script::Data() to + // the script, which contains a pointer to the context. We can then access the + // v8::Script::Data() later on to resolve names and URLs against the sub-context instead + // of the main outer context. struct SubContext : public v8::String::ExternalStringResource { SubContext(QDeclarativeContextData *context) : context(context) {} QDeclarativeGuardedContextData context; @@ -146,7 +151,7 @@ void QV8ContextWrapper::init(QV8Engine *engine) v8::Local<v8::Object> QV8ContextWrapper::qmlScope(QDeclarativeContextData *ctxt, QObject *scope) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8ContextResource *r = new QV8ContextResource(m_engine, ctxt, scope); rv->SetExternalResource(r); @@ -160,7 +165,7 @@ v8::Local<v8::Object> QV8ContextWrapper::urlScope(const QUrl &url) context->isInternal = true; context->isJSContext = true; - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_urlConstructor->NewInstance(); QV8ContextResource *r = new QV8ContextResource(m_engine, context, 0); r->ownsContext = true; @@ -219,7 +224,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::NullGetter(v8::Local<v8::String> proper QV8ContextResource *resource = v8_resource_cast<QV8ContextResource>(info.This()); if (!resource) - return v8::Undefined(); // XXX Should we throw here? + return v8::Undefined(); QV8Engine *engine = resource->engine; @@ -234,13 +239,14 @@ v8::Handle<v8::Value> QV8ContextWrapper::Getter(v8::Local<v8::String> property, QV8ContextResource *resource = v8_resource_cast<QV8ContextResource>(info.This()); if (!resource) - return v8::Undefined(); // XXX Should we throw here? + return v8::Undefined(); - // XXX aakenned too agressive + // Its possible we could delay the calculation of the "actual" context (in the case + // of sub contexts) until it is definately needed. QDeclarativeContextData *context = resource->getContext(); if (!context) - return v8::Undefined(); // XXX Should we throw here? + return v8::Undefined(); // Search type (attached property/enum/imported scripts) names // Secondary scope object @@ -352,7 +358,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::NullSetter(v8::Local<v8::String> proper QV8ContextResource *resource = v8_resource_cast<QV8ContextResource>(info.This()); if (!resource) - return v8::Undefined(); // XXX Should we throw here? + return v8::Handle<v8::Value>(); QV8Engine *engine = resource->engine; @@ -362,7 +368,7 @@ v8::Handle<v8::Value> QV8ContextWrapper::NullSetter(v8::Local<v8::String> proper QString error = QLatin1String("Invalid write to global property \"") + engine->toString(property) + QLatin1String("\""); v8::ThrowException(v8::Exception::Error(engine->toString(error))); - return v8::Undefined(); + return v8::Handle<v8::Value>(); } } @@ -373,13 +379,14 @@ v8::Handle<v8::Value> QV8ContextWrapper::Setter(v8::Local<v8::String> property, QV8ContextResource *resource = v8_resource_cast<QV8ContextResource>(info.This()); if (!resource) - return v8::Undefined(); // XXX Should we throw here? + return v8::Undefined(); - // XXX aakenned too agressive + // Its possible we could delay the calculation of the "actual" context (in the case + // of sub contexts) until it is definately needed. QDeclarativeContextData *context = resource->getContext(); if (!context) - return v8::Undefined(); // XXX Should we throw here? + return v8::Undefined(); // See QV8ContextWrapper::Getter for resolution order diff --git a/src/declarative/qml/v8/qv8contextwrapper_p.h b/src/declarative/qml/v8/qv8contextwrapper_p.h index 3149569285..12e01bab76 100644 --- a/src/declarative/qml/v8/qv8contextwrapper_p.h +++ b/src/declarative/qml/v8/qv8contextwrapper_p.h @@ -79,7 +79,11 @@ public: void addSubContext(v8::Handle<v8::Object> qmlglobal, v8::Handle<v8::Script>, QDeclarativeContextData *ctxt); - // XXX aakenned - remove this abomination + // XXX We only use the secondary scope to pass the "arguments" of the signal to + // on<SignalName> properties. Instead of doing this we should rewrite the + // JavaScript closure function to accept these arguments as named parameters. + // To keep backwards compatibility we have to check that the argument names are + // not members of the QV8Engine::illegalNames() set. QObject *setSecondaryScope(v8::Handle<v8::Object>, QObject *); QDeclarativeContextData *callingContext(); diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp index 0314402ad5..3ddac007d2 100644 --- a/src/declarative/qml/v8/qv8engine.cpp +++ b/src/declarative/qml/v8/qv8engine.cpp @@ -62,8 +62,8 @@ #include <private/qdeclarativexmlhttprequest_p.h> #include <private/qdeclarativesqldatabase_p.h> -// XXX Need to check all the global functions will also work in a worker script where the QDeclarativeEngine -// is not available +// XXX TODO: Need to check all the global functions will also work in a worker script where the +// QDeclarativeEngine is not available QT_BEGIN_NAMESPACE QV8Engine::QV8Engine() @@ -84,6 +84,7 @@ QV8Engine::~QV8Engine() delete m_listModelData; m_listModelData = 0; + qPersistentDispose(m_freezeObject); qPersistentDispose(m_getOwnPropertyNames); m_valueTypeWrapper.destroy(); m_variantWrapper.destroy(); @@ -122,7 +123,7 @@ void QV8Engine::init(QDeclarativeEngine *engine) } initializeGlobal(m_context->Global()); - freezeGlobal(); + freezeObject(m_context->Global()); } QString QV8Engine::toStringStatic(v8::Handle<v8::Value> jsstr) @@ -335,8 +336,8 @@ v8::Handle<v8::Value> QV8Engine::fromVariant(const QVariant &variant) return v8::Null(); } } else if (type == qMetaTypeId<QList<QObject *> >()) { - // XXX aakenned Can this be more optimal? Just use Array as a prototype and - // implement directly against QList<QObject*>? + // XXX Can this be made more by using Array as a prototype and implementing + // directly against QList<QObject*>? const QList<QObject *> &list = *(QList<QObject *>*)ptr; v8::Local<v8::Array> array = v8::Array::New(list.count()); for (int ii = 0; ii < list.count(); ++ii) @@ -350,49 +351,12 @@ v8::Handle<v8::Value> QV8Engine::fromVariant(const QVariant &variant) return newQObject(obj); } - return m_variantWrapper.newVariant(variant); - - // XXX aakenned -#if 0 -#ifndef QT_NO_REGEXP - case QMetaType::QRegExp: - result = newRegExp(exec, *reinterpret_cast<const QRegExp *>(ptr)); - break; -#endif -#ifndef QT_NO_QOBJECT -#endif - case QMetaType::QVariant: - result = eng->newVariant(*reinterpret_cast<const QVariant*>(ptr)); - break; - default: - if (type == qMetaTypeId<QScriptValue>()) { - result = eng->scriptValueToJSCValue(*reinterpret_cast<const QScriptValue*>(ptr)); - if (!result) - return JSC::jsUndefined(); - } - -#ifndef QT_NO_QOBJECT - // lazy registration of some common list types - else if (type == qMetaTypeId<QObjectList>()) { - qScriptRegisterSequenceMetaType<QObjectList>(eng->q_func()); - return create(exec, type, ptr); - } -#endif - else if (type == qMetaTypeId<QList<int> >()) { - qScriptRegisterSequenceMetaType<QList<int> >(eng->q_func()); - return create(exec, type, ptr); - } + // XXX TODO: To be compatible, we still need to handle: + // + QScriptValue + // + QObjectList + // + QList<int> - else { - QByteArray typeName = QMetaType::typeName(type); - if (typeName.endsWith('*') && !*reinterpret_cast<void* const *>(ptr)) - return JSC::jsNull(); - else - result = eng->newVariant(QVariant(type, ptr)); - } - } - } -#endif + return m_variantWrapper.newVariant(variant); } // A handle scope and context must be entered @@ -522,8 +486,6 @@ void QV8Engine::initializeGlobal(v8::Handle<v8::Object> global) console->Set(v8::String::New("log"), printFn); console->Set(v8::String::New("debug"), printFn); - // XXX - Qt global object properties - v8::Local<v8::Object> qt = v8::Object::New(); // Set all the enums from the "Qt" namespace @@ -562,22 +524,21 @@ void QV8Engine::initializeGlobal(v8::Handle<v8::Object> global) qt->Set(v8::String::New("md5"), V8FUNCTION(md5, this)); qt->Set(v8::String::New("btoa"), V8FUNCTION(btoa, this)); qt->Set(v8::String::New("atob"), V8FUNCTION(atob, this)); - qt->Set(v8::String::New("quit"), V8FUNCTION(quit, this)); qt->Set(v8::String::New("resolvedUrl"), V8FUNCTION(resolvedUrl, this)); if (m_engine) { + qt->Set(v8::String::New("quit"), V8FUNCTION(quit, this)); qt->Set(v8::String::New("createQmlObject"), V8FUNCTION(createQmlObject, this)); qt->Set(v8::String::New("createComponent"), V8FUNCTION(createComponent, this)); } - // XXX translator functions + // XXX TODO - Implement translator functions global->Set(v8::String::New("print"), printFn); global->Set(v8::String::New("console"), console); global->Set(v8::String::New("Qt"), qt); global->Set(v8::String::New("gc"), V8FUNCTION(gc, this)); - // XXX mainthread only m_xmlHttpRequestData = qt_add_qmlxmlhttprequest(this); m_sqlDatabaseData = qt_add_qmlsqldatabase(this); @@ -588,29 +549,38 @@ void QV8Engine::initializeGlobal(v8::Handle<v8::Object> global) for (quint32 ii = 0; ii < namesArray->Length(); ++ii) m_illegalNames.insert(toString(namesArray->Get(ii))); } + + { +#define FREEZE_SOURCE "(function freeze_recur(obj) { "\ + " if (Qt.isQtObject(obj)) return;"\ + " if (obj != Function.connect && obj != Function.disconnect && "\ + " obj instanceof Object) {"\ + " var properties = Object.getOwnPropertyNames(obj);"\ + " for (var prop in properties) { "\ + " if (prop == \"connect\" || prop == \"disconnect\") {"\ + " Object.freeze(obj[prop]); "\ + " continue;"\ + " }"\ + " freeze_recur(obj[prop]);"\ + " }"\ + " }"\ + " if (obj instanceof Object) {"\ + " Object.freeze(obj);"\ + " }"\ + "})" + + v8::Local<v8::Script> freeze = v8::Script::New(v8::String::New(FREEZE_SOURCE)); + v8::Local<v8::Value> result = freeze->Run(); + Q_ASSERT(result->IsFunction()); + m_freezeObject = qPersistentNew(v8::Local<v8::Function>::Cast(result)); +#undef FREEZE_SOURCE + } } -void QV8Engine::freezeGlobal() +void QV8Engine::freezeObject(v8::Handle<v8::Value> value) { - // Freeze the global object - // XXX I don't think this is sufficient as it misses non-enumerable properties -#define FREEZE "(function freeze_recur(obj) { "\ - " if (Qt.isQtObject(obj)) return;"\ - " for (var prop in obj) { " \ - " if (prop == \"connect\" || prop == \"disconnect\") {" \ - " Object.freeze(obj[prop]); "\ - " continue;" \ - " }" \ - " freeze_recur(obj[prop]);" \ - " }" \ - " if (obj instanceof Object) {" \ - " Object.freeze(obj);" \ - " }"\ - "})(this);" - v8::Local<v8::Script> test = v8::Script::New(v8::String::New(FREEZE)); -#undef FREEZE - - test->Run(); + v8::Handle<v8::Value> args[] = { value }; + m_freezeObject->Call(global(), 1, args); } void QV8Engine::gc() @@ -1270,7 +1240,6 @@ v8::Handle<v8::Value> QV8Engine::openUrlExternally(const v8::Arguments &args) v8::Handle<v8::Value> QV8Engine::resolvedUrl(const v8::Arguments &args) { QUrl url = V8ENGINE()->toVariant(args[0], -1).toUrl(); - // XXX uses QDeclarativeEngine which means it wont work in worker script? QDeclarativeEngine *e = V8ENGINE()->engine(); QDeclarativeEnginePrivate *p = 0; if (e) p = QDeclarativeEnginePrivate::get(e); @@ -1349,7 +1318,6 @@ QDeclarativeEngine::quit() signal to the QCoreApplication::quit() slot. */ v8::Handle<v8::Value> QV8Engine::quit(const v8::Arguments &args) { - // XXX worker script? QDeclarativeEnginePrivate::get(V8ENGINE()->engine())->sendQuit(); return v8::Undefined(); } diff --git a/src/declarative/qml/v8/qv8engine_p.h b/src/declarative/qml/v8/qv8engine_p.h index 1843460f76..7398944ff5 100644 --- a/src/declarative/qml/v8/qv8engine_p.h +++ b/src/declarative/qml/v8/qv8engine_p.h @@ -225,6 +225,7 @@ public: QDeclarativeContextData *callingContext(); v8::Local<v8::Array> getOwnPropertyNames(v8::Handle<v8::Object>); + void freezeObject(v8::Handle<v8::Value>); inline QString toString(v8::Handle<v8::Value> string); inline QString toString(v8::Handle<v8::String> string); @@ -295,6 +296,7 @@ private: QV8ValueTypeWrapper m_valueTypeWrapper; v8::Persistent<v8::Function> m_getOwnPropertyNames; + v8::Persistent<v8::Function> m_freezeObject; void *m_xmlHttpRequestData; void *m_sqlDatabaseData; @@ -307,7 +309,6 @@ private: QVariant toBasicVariant(v8::Handle<v8::Value>); void initializeGlobal(v8::Handle<v8::Object>); - void freezeGlobal(); static v8::Handle<v8::Value> gc(const v8::Arguments &args); static v8::Handle<v8::Value> print(const v8::Arguments &args); @@ -424,7 +425,8 @@ v8::Handle<v8::Value> QV8Engine::newValueType(const QVariant &value, QDeclarativ return m_valueTypeWrapper.newValueType(value, type); } -// XXX perf? +// XXX Can this be made more optimal? It is called prior to resolving each and every +// unqualified name in QV8ContextWrapper. bool QV8Engine::startsWithUpper(v8::Handle<v8::String> string) { uint16_t buffer[2]; diff --git a/src/declarative/qml/v8/qv8include.cpp b/src/declarative/qml/v8/qv8include.cpp index 3b8a081a3b..f8ef8a6af4 100644 --- a/src/declarative/qml/v8/qv8include.cpp +++ b/src/declarative/qml/v8/qv8include.cpp @@ -78,7 +78,7 @@ QV8Include::~QV8Include() v8::Local<v8::Object> QV8Include::resultValue(Status status) { - // XXX aakenned inefficient + // XXX It seems inefficient to create this object from scratch each time. v8::Local<v8::Object> result = v8::Object::New(); result->Set(v8::String::New("OK"), v8::Integer::New(Ok)); result->Set(v8::String::New("LOADING"), v8::Integer::New(Loading)); @@ -94,7 +94,7 @@ void QV8Include::callback(QV8Engine *engine, v8::Handle<v8::Function> callback, { if (!callback.IsEmpty()) { v8::Handle<v8::Value> args[] = { status }; - // XXX TryCatch? + v8::TryCatch tc; callback->Call(engine->global(), 1, args); } } diff --git a/src/declarative/qml/v8/qv8listwrapper.cpp b/src/declarative/qml/v8/qv8listwrapper.cpp index 13b3b30c35..5f2d9fb91e 100644 --- a/src/declarative/qml/v8/qv8listwrapper.cpp +++ b/src/declarative/qml/v8/qv8listwrapper.cpp @@ -88,7 +88,7 @@ v8::Handle<v8::Value> QV8ListWrapper::newList(QObject *object, int propId, int p if (!object || propId == -1) return v8::Null(); - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8ListResource *r = new QV8ListResource(m_engine); r->object = object; @@ -101,7 +101,7 @@ v8::Handle<v8::Value> QV8ListWrapper::newList(QObject *object, int propId, int p v8::Handle<v8::Value> QV8ListWrapper::newList(const QDeclarativeListProperty<QObject> &prop, int propType) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8ListResource *r = new QV8ListResource(m_engine); r->object = prop.object; diff --git a/src/declarative/qml/v8/qv8qobjectwrapper.cpp b/src/declarative/qml/v8/qv8qobjectwrapper.cpp index 75a97ac8b5..02eb346137 100644 --- a/src/declarative/qml/v8/qv8qobjectwrapper.cpp +++ b/src/declarative/qml/v8/qv8qobjectwrapper.cpp @@ -70,7 +70,7 @@ Q_DECLARE_METATYPE(QDeclarativeV8Handle); #define QOBJECT_TOSTRING_INDEX -2 #define QOBJECT_DESTROY_INDEX -3 -// XXX Need to check all calls to QDeclarativeEngine *engine() to confirm this class works +// XXX TODO: Need to review all calls to QDeclarativeEngine *engine() to confirm QObjects work // correctly in a worker thread class QV8QObjectResource : public QV8ObjectResource @@ -297,7 +297,8 @@ v8::Handle<v8::Value> QV8QObjectWrapper::GetProperty(QV8Engine *engine, QObject v8::Handle<v8::String> property, QV8QObjectWrapper::RevisionMode revisionMode) { - // XXX aakenned This can't possibly be the best solution!!! + // XXX More recent versions of V8 introduced "Callable" objects. It is possible that these + // will be a faster way of creating QObject method objects. struct MethodClosure { static v8::Handle<v8::Value> create(QV8Engine *engine, QObject *object, v8::Handle<v8::Value> *objectHandle, @@ -505,12 +506,9 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Getter(v8::Local<v8::String> property, return v8engine->typeWrapper()->newObject(object, data->typeNamespace, QV8TypeWrapper::ExcludeEnums); } } + } - return v8::Undefined(); - } else { - // XXX throw? - return v8::Undefined(); - } + return v8::Undefined(); } v8::Handle<v8::Value> QV8QObjectWrapper::Setter(v8::Local<v8::String> property, @@ -584,8 +582,6 @@ v8::Handle<v8::Array> QV8QObjectWrapper::Enumerator(const v8::AccessorInfo &info if (ddata) { cache->addref(); ddata->propertyCache = cache; } } else { // Not cachable - fall back to QMetaObject (eg. dynamic meta object) - // XXX QDeclarativeOpenMetaObject has a cache, so this is suboptimal. - // XXX This is a workaround for QTBUG-9420. const QMetaObject *mo = object->metaObject(); int pc = mo->propertyCount(); int po = mo->propertyOffset(); @@ -679,9 +675,9 @@ v8::Local<v8::Object> QDeclarativePropertyCache::newQObject(QObject *object, QV8 QString toString = QLatin1String("toString"); QString destroy = QLatin1String("destroy"); - // XXX Enables fast property accessors. These more than double the property access + // XXX TODO: Enables fast property accessors. These more than double the property access // performance, but the cost of setting up this structure hasn't been measured so - // its not guarenteed that this is a win overall + // its not guarenteed that this is a win overall. We need to try and measure the cost. for (StringCache::ConstIterator iter = stringCache.begin(); iter != stringCache.end(); ++iter) { Data *property = *iter; if (property->isFunction() || !property->isWritable() || @@ -758,7 +754,7 @@ v8::Local<v8::Object> QV8QObjectWrapper::newQObject(QObject *object, QDeclarativ if (ddata->propertyCache) { rv = ddata->propertyCache->newQObject(object, engine); } else { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized rv = m_constructor->NewInstance(); QV8QObjectResource *r = new QV8QObjectResource(engine, object); rv->SetExternalResource(r); @@ -872,21 +868,47 @@ struct QV8QObjectConnectionList : public QObject, public QDeclarativeGuard<QObje ~QV8QObjectConnectionList(); struct Connection { + Connection() + : needsDestroy(false) {} + Connection(const Connection &other) + : thisObject(other.thisObject), function(other.function), needsDestroy(false) {} + Connection &operator=(const Connection &other) { + thisObject = other.thisObject; + function = other.function; + needsDestroy = other.needsDestroy; + return *this; + } + v8::Persistent<v8::Object> thisObject; v8::Persistent<v8::Function> function; + + void dispose() { + qPersistentDispose(thisObject); + qPersistentDispose(function); + } + + bool needsDestroy; + }; + + struct ConnectionList : public QList<Connection> { + ConnectionList() : connectionsInUse(0), connectionsNeedClean(false) {} + int connectionsInUse; + bool connectionsNeedClean; }; QV8Engine *engine; - typedef QHash<int, QList<Connection> > SlotHash; + typedef QHash<int, ConnectionList> SlotHash; SlotHash slotHash; + bool needsDestroy; + int inUse; virtual void objectDestroyed(QObject *); virtual int qt_metacall(QMetaObject::Call, int, void **); }; QV8QObjectConnectionList::QV8QObjectConnectionList(QObject *object, QV8Engine *engine) -: QDeclarativeGuard<QObject>(object), engine(engine) +: QDeclarativeGuard<QObject>(object), engine(engine), needsDestroy(false), inUse(0) { } @@ -905,7 +927,11 @@ QV8QObjectConnectionList::~QV8QObjectConnectionList() void QV8QObjectConnectionList::objectDestroyed(QObject *object) { engine->qobjectWrapper()->m_connections.remove(object); - delete this; + + if (inUse) + needsDestroy = true; + else + delete this; } int QV8QObjectConnectionList::qt_metacall(QMetaObject::Call method, int index, void **metaArgs) @@ -914,13 +940,20 @@ int QV8QObjectConnectionList::qt_metacall(QMetaObject::Call method, int index, v SlotHash::Iterator iter = slotHash.find(index); if (iter == slotHash.end()) return -1; - QList<Connection> &connections = *iter; - if (connections.isEmpty()) + ConnectionList &connectionList = *iter; + if (connectionList.isEmpty()) return -1; - // XXX optimize + inUse++; + + connectionList.connectionsInUse++; + + QList<Connection> connections = connectionList; + QMetaMethod method = data()->metaObject()->method(index); Q_ASSERT(method.methodType() == QMetaMethod::Signal); + // XXX TODO: We should figure out a way to cache the parameter types to avoid resolving + // them each time. QList<QByteArray> params = method.parameterTypes(); v8::HandleScope handle_scope; @@ -938,15 +971,33 @@ int QV8QObjectConnectionList::qt_metacall(QMetaObject::Call method, int index, v } } - // XXX what if this list changes or this object is deleted during the calls? for (int ii = 0; ii < connections.count(); ++ii) { Connection &connection = connections[ii]; + if (connection.needsDestroy) + continue; if (connection.thisObject.IsEmpty()) { connection.function->Call(engine->global(), argCount, args.data()); } else { connection.function->Call(connection.thisObject, argCount, args.data()); } } + + connectionList.connectionsInUse--; + if (connectionList.connectionsInUse == 0 && connectionList.connectionsNeedClean) { + for (QList<Connection>::Iterator iter = connectionList.begin(); + iter != connectionList.end(); ) { + if (iter->needsDestroy) { + iter->dispose(); + iter = connectionList.erase(iter); + } else { + ++iter; + } + } + } + + inUse--; + if (inUse == 0 && needsDestroy) + delete this; } return -1; @@ -1000,7 +1051,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Connect(const v8::Arguments &args) QV8QObjectConnectionList *connectionList = *iter; QV8QObjectConnectionList::SlotHash::Iterator slotIter = connectionList->slotHash.find(signalIndex); if (slotIter == connectionList->slotHash.end()) { - slotIter = connectionList->slotHash.insert(signalIndex, QList<QV8QObjectConnectionList::Connection>()); + slotIter = connectionList->slotHash.insert(signalIndex, QV8QObjectConnectionList::ConnectionList()); QMetaObject::connect(signalObject, signalIndex, connectionList, signalIndex); } @@ -1064,7 +1115,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Disconnect(const v8::Arguments &args) if (slotIter == connectionList->slotHash.end()) return v8::Undefined(); // Nothing to disconnect from - QList<QV8QObjectConnectionList::Connection> &connections = *slotIter; + QV8QObjectConnectionList::ConnectionList &connections = *slotIter; v8::Local<v8::Function> function = v8::Local<v8::Function>::Cast(functionValue); QPair<QObject *, int> functionData = ExtractQtMethod(engine, function); @@ -1080,9 +1131,12 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Disconnect(const v8::Arguments &args) QPair<QObject *, int> connectedFunctionData = ExtractQtMethod(engine, connection.function); if (connectedFunctionData == functionData) { // Match! - qPersistentDispose(connection.thisObject); - qPersistentDispose(connection.function); - connections.removeAt(ii); + if (connections.connectionsInUse) { + connection.needsDestroy = true; + } else { + connection.dispose(); + connections.removeAt(ii); + } return v8::Undefined(); } } @@ -1096,9 +1150,12 @@ v8::Handle<v8::Value> QV8QObjectWrapper::Disconnect(const v8::Arguments &args) connection.thisObject.IsEmpty() == functionThisValue.IsEmpty() && (connection.thisObject.IsEmpty() || connection.thisObject->StrictEquals(functionThisValue))) { // Match! - qPersistentDispose(connection.thisObject); - qPersistentDispose(connection.function); - connections.removeAt(ii); + if (connections.connectionsInUse) { + connection.needsDestroy = true; + } else { + connection.dispose(); + connections.removeAt(ii); + } return v8::Undefined(); } } @@ -1790,8 +1847,8 @@ v8::Handle<v8::Value> MetaCallArgument::toValue(QV8Engine *engine) QDeclarativeData::get(object, true)->setImplicitDestructible(); return engine->newQObject(object); } else if (type == qMetaTypeId<QList<QObject *> >()) { - // XXX aakenned Can this be more optimal? Just use Array as a prototype and - // implement directly against QList<QObject*>? + // XXX Can this be made more by using Array as a prototype and implementing + // directly against QList<QObject*>? QList<QObject *> &list = *(QList<QObject *>*)&data; v8::Local<v8::Array> array = v8::Array::New(list.count()); for (int ii = 0; ii < list.count(); ++ii) diff --git a/src/declarative/qml/v8/qv8typewrapper.cpp b/src/declarative/qml/v8/qv8typewrapper.cpp index b4a510e2be..70af6037de 100644 --- a/src/declarative/qml/v8/qv8typewrapper.cpp +++ b/src/declarative/qml/v8/qv8typewrapper.cpp @@ -98,7 +98,7 @@ void QV8TypeWrapper::init(QV8Engine *engine) v8::Local<v8::Object> QV8TypeWrapper::newObject(QObject *o, QDeclarativeType *t, TypeNameMode mode) { Q_ASSERT(t); - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8TypeResource *r = new QV8TypeResource(m_engine); r->mode = mode; r->object = o; r->type = t; @@ -109,7 +109,7 @@ v8::Local<v8::Object> QV8TypeWrapper::newObject(QObject *o, QDeclarativeType *t, v8::Local<v8::Object> QV8TypeWrapper::newObject(QObject *o, QDeclarativeTypeNameCache *t, TypeNameMode mode) { Q_ASSERT(t); - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8TypeResource *r = new QV8TypeResource(m_engine); t->addref(); @@ -169,8 +169,8 @@ v8::Handle<v8::Value> QV8TypeWrapper::Getter(v8::Local<v8::String> property, if (d && d->type) { return v8engine->typeWrapper()->newObject(object, d->type, resource->mode); } else if (QDeclarativeMetaType::ModuleApiInstance *moduleApi = typeNamespace->moduleApi()) { - - // XXX QtScript/JSC required + // XXX TODO: Currently module APIs are implemented against QScriptValues. Consequently we + // can't do anything here until the QtScript/V8 binding is complete. return v8::Undefined(); } @@ -195,7 +195,7 @@ v8::Handle<v8::Value> QV8TypeWrapper::Setter(v8::Local<v8::String> property, QV8Engine *v8engine = resource->engine; - // XXX module api + // XXX TODO: Implement writes to module API objects if (resource->type && resource->object) { QDeclarativeType *type = resource->type; diff --git a/src/declarative/qml/v8/qv8valuetypewrapper.cpp b/src/declarative/qml/v8/qv8valuetypewrapper.cpp index b16a81e607..f4f79ce92e 100644 --- a/src/declarative/qml/v8/qv8valuetypewrapper.cpp +++ b/src/declarative/qml/v8/qv8valuetypewrapper.cpp @@ -117,7 +117,7 @@ void QV8ValueTypeWrapper::init(QV8Engine *engine) v8::Local<v8::Object> QV8ValueTypeWrapper::newValueType(QObject *object, int property, QDeclarativeValueType *type) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8ValueTypeReferenceResource *r = new QV8ValueTypeReferenceResource(m_engine); r->type = type; r->object = object; r->property = property; @@ -127,7 +127,7 @@ v8::Local<v8::Object> QV8ValueTypeWrapper::newValueType(QObject *object, int pro v8::Local<v8::Object> QV8ValueTypeWrapper::newValueType(const QVariant &value, QDeclarativeValueType *type) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv = m_constructor->NewInstance(); QV8ValueTypeCopyResource *r = new QV8ValueTypeCopyResource(m_engine); r->type = type; r->value = value; @@ -172,9 +172,9 @@ v8::Handle<v8::Value> QV8ValueTypeWrapper::Getter(v8::Local<v8::String> property QV8ValueTypeResource *r = v8_resource_cast<QV8ValueTypeResource>(info.This()); if (!r) return v8::Undefined(); - // XXX aakenned - this is horribly inefficient. People seem to have taken a - // liking to value type properties, so we should probably try and optimize it - // a little. + // XXX This is horribly inefficient. Sadly people seem to have taken a liking to + // value type properties, so we should probably try and optimize it a little. + // We should probably just replace all value properties with dedicated accessors. QByteArray propName = r->engine->toString(property).toUtf8(); int index = r->type->metaObject()->indexOfProperty(propName.constData()); diff --git a/src/declarative/qml/v8/qv8variantwrapper.cpp b/src/declarative/qml/v8/qv8variantwrapper.cpp index ef1f972ad1..37e998d988 100644 --- a/src/declarative/qml/v8/qv8variantwrapper.cpp +++ b/src/declarative/qml/v8/qv8variantwrapper.cpp @@ -106,7 +106,7 @@ v8::Local<v8::Object> QV8VariantWrapper::newVariant(const QVariant &value) bool scarceResource = value.type() == QVariant::Pixmap || value.type() == QVariant::Image; - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local<v8::Object> rv; QV8VariantResource *r = new QV8VariantResource(m_engine, value); diff --git a/src/declarative/qml/v8/qv8worker.cpp b/src/declarative/qml/v8/qv8worker.cpp index 846fdb7759..0d396d3304 100644 --- a/src/declarative/qml/v8/qv8worker.cpp +++ b/src/declarative/qml/v8/qv8worker.cpp @@ -133,9 +133,9 @@ static inline void *popPtr(const char *&data) return rv; } -// XXX double check exception safety +// XXX TODO: Check that worker script is exception safe in the case of +// serialization/deserialization failures -#include <QDebug> #define ALIGN(size) (((size) + 3) & ~3) void QV8Worker::serialize(QByteArray &data, v8::Handle<v8::Value> v, QV8Engine *engine) { @@ -166,7 +166,8 @@ void QV8Worker::serialize(QByteArray &data, v8::Handle<v8::Value> v, QV8Engine * string->Write((uint16_t*)buffer); } else if (v->IsFunction()) { - // XXX + // XXX TODO: Implement passing function objects between the main and + // worker scripts push(data, valueheader(WorkerUndefined)); } else if (v->IsArray()) { v8::Handle<v8::Array> array = v8::Handle<v8::Array>::Cast(v); @@ -238,7 +239,8 @@ void QV8Worker::serialize(QByteArray &data, v8::Handle<v8::Value> v, QV8Engine * } } } else if (engine->isQObject(v)) { - // XXX Can we generalize this? + // XXX TODO: Generalize passing objects between the main thread and worker scripts so + // that others can trivially plug in their elements. QDeclarativeListModel *lm = qobject_cast<QDeclarativeListModel *>(engine->toQObject(v)); if (lm && lm->agent()) { QDeclarativeListModelWorkerAgent *agent = lm->agent(); |