From 8bb487e60899382f0890fd675eb272d5cc562882 Mon Sep 17 00:00:00 2001 From: Aaron Kennedy Date: Thu, 9 Jun 2011 14:40:44 +1000 Subject: [PATCH] Minor XXX fixups --- src/declarative/qml/qdeclarativebinding.cpp | 10 -- src/declarative/qml/qdeclarativecomponent.cpp | 1 - src/declarative/qml/qdeclarativecontext_p.h | 2 - src/declarative/qml/qdeclarativeexpression.cpp | 25 +++-- src/declarative/qml/qdeclarativeexpression_p.h | 1 + src/declarative/qml/qdeclarativetypeloader.cpp | 2 +- src/declarative/qml/qdeclarativevme.cpp | 1 - src/declarative/qml/qdeclarativevmemetaobject.cpp | 1 - src/declarative/qml/qdeclarativexmlhttprequest.cpp | 19 ++-- src/declarative/qml/v8/qv8contextwrapper.cpp | 31 +++--- src/declarative/qml/v8/qv8contextwrapper_p.h | 6 +- src/declarative/qml/v8/qv8engine.cpp | 116 ++++++++------------- src/declarative/qml/v8/qv8engine_p.h | 6 +- src/declarative/qml/v8/qv8include.cpp | 4 +- src/declarative/qml/v8/qv8listwrapper.cpp | 4 +- src/declarative/qml/v8/qv8qobjectwrapper.cpp | 115 ++++++++++++++------ src/declarative/qml/v8/qv8typewrapper.cpp | 10 +- src/declarative/qml/v8/qv8valuetypewrapper.cpp | 10 +- src/declarative/qml/v8/qv8variantwrapper.cpp | 2 +- src/declarative/qml/v8/qv8worker.cpp | 10 +- 20 files changed, 204 insertions(+), 172 deletions(-) diff --git a/src/declarative/qml/qdeclarativebinding.cpp b/src/declarative/qml/qdeclarativebinding.cpp index 106cc10..6cca327 100644 --- a/src/declarative/qml/qdeclarativebinding.cpp +++ b/src/declarative/qml/qdeclarativebinding.cpp @@ -387,16 +387,6 @@ void QDeclarativeBinding::update(QDeclarativePropertyPrivate::WriteFlags flags) value = QVariant::fromValue((QObject *)0); } else { value = ep->v8engine.toVariant(result, d->property.propertyType()); - if (value.userType() == QMetaType::QObjectStar && !qvariant_cast(value)) { - // If the object is null, we extract the predicted type. While this isn't - // 100% reliable, in many cases it gives us better error messages if we - // assign this null-object to an incompatible property - // XXX aakenned - // int type = ep->objectClass->objectType(scriptValue); - int type = QMetaType::QObjectStar; - QObject *o = 0; - value = QVariant(type, (void *)&o); - } } diff --git a/src/declarative/qml/qdeclarativecomponent.cpp b/src/declarative/qml/qdeclarativecomponent.cpp index c7e5bcb..e99928d 100644 --- a/src/declarative/qml/qdeclarativecomponent.cpp +++ b/src/declarative/qml/qdeclarativecomponent.cpp @@ -982,7 +982,6 @@ void QDeclarativeComponentPrivate::complete(QDeclarativeEnginePrivate *enginePri state->bindValues.at(ii); for (int jj = 0; jj < bv.count; ++jj) { if(bv.at(jj)) { - // XXX akennedy bv.at(jj)->m_mePtr = 0; bv.at(jj)->setEnabled(true, QDeclarativePropertyPrivate::BypassInterceptor | QDeclarativePropertyPrivate::DontRemoveBinding); diff --git a/src/declarative/qml/qdeclarativecontext_p.h b/src/declarative/qml/qdeclarativecontext_p.h index e4312f1..93b9d3b 100644 --- a/src/declarative/qml/qdeclarativecontext_p.h +++ b/src/declarative/qml/qdeclarativecontext_p.h @@ -150,9 +150,7 @@ public: QObject *contextObject; // Any script blocks that exist on this context - // XXX aakenned QList > importedScripts; -// QList importedScripts; // Context base url QUrl url; diff --git a/src/declarative/qml/qdeclarativeexpression.cpp b/src/declarative/qml/qdeclarativeexpression.cpp index 517d4fc..095d9be 100644 --- a/src/declarative/qml/qdeclarativeexpression.cpp +++ b/src/declarative/qml/qdeclarativeexpression.cpp @@ -68,8 +68,9 @@ bool QDeclarativeDelayedError::addError(QDeclarativeEnginePrivate *e) } QDeclarativeQtScriptExpression::QDeclarativeQtScriptExpression() -: dataRef(0), expressionFunctionMode(ExplicitContext), scopeObject(0), trackChange(false), - guardList(0), guardListLength(0), guardObject(0), guardObjectNotifyIndex(-1), deleted(0) +: dataRef(0), extractExpressionFromFunction(false), expressionFunctionMode(ExplicitContext), + scopeObject(0), trackChange(false), guardList(0), guardListLength(0), guardObject(0), + guardObjectNotifyIndex(-1), deleted(0) { } @@ -105,15 +106,13 @@ void QDeclarativeExpressionPrivate::init(QDeclarativeContextData *ctxt, const QS void QDeclarativeExpressionPrivate::init(QDeclarativeContextData *ctxt, v8::Handle func, QObject *me) { - // XXX aakenned - // expression = func.toString(); - QDeclarativeAbstractExpression::setContext(ctxt); scopeObject = me; v8function = qPersistentNew(func); expressionFunctionMode = ExplicitContext; expressionFunctionValid = true; + extractExpressionFromFunction = true; } void QDeclarativeExpressionPrivate::init(QDeclarativeContextData *ctxt, void *expr, @@ -155,15 +154,16 @@ QDeclarativeExpressionPrivate::evalFunction(QDeclarativeContextData *ctxt, QObje QDeclarativeEngine *engine = ctxt->engine; QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(engine); - // XXX aakenned optimize + // XXX TODO: Implement script caching, like we used to do with QScriptProgram in the + // QtScript days v8::HandleScope handle_scope; v8::Context::Scope ctxtscope(ep->v8engine.context()); - // XXX try/catch? - + v8::TryCatch tc; v8::Local scopeobject = ep->v8engine.qmlScope(ctxt, scope); v8::Local script = ep->v8engine.qmlModeCompile(code, filename, line); v8::Local result = script->Run(scopeobject); + if (tc.HasCaught()) return v8::Persistent(); if (qmlscope) *qmlscope = qPersistentNew(scopeobject); return qPersistentNew(v8::Local::Cast(result)); } @@ -335,6 +335,13 @@ QDeclarativeContext *QDeclarativeExpression::context() const QString QDeclarativeExpression::expression() const { Q_D(const QDeclarativeExpression); + if (d->extractExpressionFromFunction && context()->engine()) { + QV8Engine *v8engine = QDeclarativeEnginePrivate::getV8Engine(context()->engine()); + v8::HandleScope handle_scope; + v8::Context::Scope scope(v8engine->context()); + + return v8engine->toString(v8::Handle(d->v8function)); + } return d->expression; } @@ -472,7 +479,6 @@ v8::Local QDeclarativeQtScriptExpression::eval(QObject *secondaryScop restoreSecondaryScope = ep->v8engine.contextWrapper()->setSecondaryScope(v8qmlscope, secondaryScope); v8::TryCatch try_catch; - v8::Context::Scope scope(ep->v8engine.context()); // XXX is this needed? v8::Handle This; @@ -494,6 +500,7 @@ v8::Local QDeclarativeQtScriptExpression::eval(QObject *secondaryScop if (watcher.wasDeleted()) { } else if (try_catch.HasCaught()) { + v8::Context::Scope scope(ep->v8engine.context()); v8::Local message = try_catch.Message(); if (!message.IsEmpty()) { QDeclarativeExpressionPrivate::exceptionToError(message, error); diff --git a/src/declarative/qml/qdeclarativeexpression_p.h b/src/declarative/qml/qdeclarativeexpression_p.h index c183cf2..b03c709 100644 --- a/src/declarative/qml/qdeclarativeexpression_p.h +++ b/src/declarative/qml/qdeclarativeexpression_p.h @@ -122,6 +122,7 @@ public: QDeclarativeRefCount *dataRef; QString expression; + bool extractExpressionFromFunction; Mode expressionFunctionMode; v8::Persistent v8function; diff --git a/src/declarative/qml/qdeclarativetypeloader.cpp b/src/declarative/qml/qdeclarativetypeloader.cpp index b4e6ef4..a788b4b 100644 --- a/src/declarative/qml/qdeclarativetypeloader.cpp +++ b/src/declarative/qml/qdeclarativetypeloader.cpp @@ -1232,7 +1232,7 @@ void QDeclarativeScriptBlob::done() m_scriptData->pragmas = m_pragmas; - // XXX aakenned - what about error handling? + // XXX TODO: Handle errors that occur duing the script compile QV8Engine *v8engine = &QDeclarativeEnginePrivate::get(engine)->v8engine; v8::HandleScope handle_scope; v8::Context::Scope scope(v8engine->context()); diff --git a/src/declarative/qml/qdeclarativevme.cpp b/src/declarative/qml/qdeclarativevme.cpp index c99551f..146c856 100644 --- a/src/declarative/qml/qdeclarativevme.cpp +++ b/src/declarative/qml/qdeclarativevme.cpp @@ -995,7 +995,6 @@ v8::Persistent QDeclarativeVME::run(QDeclarativeContextData *parentC ctxt->importedScripts << run(ctxt, script->scripts.at(ii)->scriptData()); } - // XXX aakenned optimize v8::HandleScope handle_scope; v8::Context::Scope scope(v8engine->context()); diff --git a/src/declarative/qml/qdeclarativevmemetaobject.cpp b/src/declarative/qml/qdeclarativevmemetaobject.cpp index 218b1c1..247c1aa 100644 --- a/src/declarative/qml/qdeclarativevmemetaobject.cpp +++ b/src/declarative/qml/qdeclarativevmemetaobject.cpp @@ -652,7 +652,6 @@ int QDeclarativeVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a) QDeclarativeEnginePrivate *ep = QDeclarativeEnginePrivate::get(ctxt->engine); ep->referenceScarceResources(); // "hold" scarce resources in memory during evaluation. - // XXX aakenned v8::Handle function = method(id); QDeclarativeVMEMetaData::MethodData *data = metaData->methodData() + id; diff --git a/src/declarative/qml/qdeclarativexmlhttprequest.cpp b/src/declarative/qml/qdeclarativexmlhttprequest.cpp index 22ff408..64719d5 100644 --- a/src/declarative/qml/qdeclarativexmlhttprequest.cpp +++ b/src/declarative/qml/qdeclarativexmlhttprequest.cpp @@ -536,7 +536,7 @@ v8::Handle Node::prototype(QV8Engine *engine) 0, v8::External::Wrap(engine)); d->nodePrototype->SetAccessor(v8::String::New("attributes"), attributes, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->nodePrototype); } return d->nodePrototype; } @@ -586,7 +586,7 @@ v8::Handle Element::prototype(QV8Engine *engine) d->elementPrototype->SetPrototype(Node::prototype(engine)); d->elementPrototype->SetAccessor(v8::String::New("tagName"), nodeName, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->elementPrototype); } return d->elementPrototype; } @@ -603,7 +603,7 @@ v8::Handle Attr::prototype(QV8Engine *engine) 0, v8::External::Wrap(engine)); d->attrPrototype->SetAccessor(v8::String::New("ownerElement"), ownerElement, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->attrPrototype); } return d->attrPrototype; } @@ -654,7 +654,7 @@ v8::Handle CharacterData::prototype(QV8Engine *engine) 0, v8::External::Wrap(engine)); d->characterDataPrototype->SetAccessor(v8::String::New("length"), length, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->characterDataPrototype); } return d->characterDataPrototype; } @@ -687,7 +687,7 @@ v8::Handle Text::prototype(QV8Engine *engine) 0, v8::External::Wrap(engine)); d->textPrototype->SetAccessor(v8::String::New("wholeText"), wholeText, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->textPrototype); } return d->textPrototype; } @@ -698,7 +698,7 @@ v8::Handle CDATA::prototype(QV8Engine *engine) if (d->cdataPrototype.IsEmpty()) { d->cdataPrototype = qPersistentNew(v8::Object::New()); d->cdataPrototype->SetPrototype(Text::prototype(engine)); - // XXX freeze + engine->freezeObject(d->cdataPrototype); } return d->cdataPrototype; } @@ -717,7 +717,7 @@ v8::Handle Document::prototype(QV8Engine *engine) 0, v8::External::Wrap(engine)); d->documentPrototype->SetAccessor(v8::String::New("documentElement"), documentElement, 0, v8::External::Wrap(engine)); - // XXX freeze + engine->freezeObject(d->documentPrototype); } return d->documentPrototype; } @@ -879,7 +879,7 @@ v8::Handle NamedNodeMap::prototype(QV8Engine *engine) ot->SetIndexedPropertyHandler(indexed, 0, 0, 0, 0, v8::External::Wrap(engine)); ot->SetFallbackPropertyHandler(named, 0, 0, 0, 0, v8::External::Wrap(engine)); d->namedNodeMapPrototype = qPersistentNew(ot->NewInstance()); - // XXX freeze + engine->freezeObject(d->namedNodeMapPrototype); } return d->namedNodeMapPrototype; } @@ -906,7 +906,6 @@ v8::Handle NodeList::indexed(uint32_t index, const v8::AccessorInfo& if (index < r->d->children.count()) { return Node::create(engine, r->d->children.at(index)); } else { - // XXX RangeError exception? return v8::Undefined(); } } @@ -928,7 +927,7 @@ v8::Handle NodeList::prototype(QV8Engine *engine) ot->SetAccessor(v8::String::New("length"), length, 0, v8::External::Wrap(engine)); ot->SetIndexedPropertyHandler(indexed, 0, 0, 0, 0, v8::External::Wrap(engine)); d->nodeListPrototype = qPersistentNew(ot->NewInstance()); - // XXX freeze + engine->freezeObject(d->nodeListPrototype); } return d->nodeListPrototype; } diff --git a/src/declarative/qml/v8/qv8contextwrapper.cpp b/src/declarative/qml/v8/qv8contextwrapper.cpp index 27002c6..f0e52b1 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 QV8ContextWrapper::qmlScope(QDeclarativeContextData *ctxt, QObject *scope) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local rv = m_constructor->NewInstance(); QV8ContextResource *r = new QV8ContextResource(m_engine, ctxt, scope); rv->SetExternalResource(r); @@ -160,7 +165,7 @@ v8::Local 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 rv = m_urlConstructor->NewInstance(); QV8ContextResource *r = new QV8ContextResource(m_engine, context, 0); r->ownsContext = true; @@ -219,7 +224,7 @@ v8::Handle QV8ContextWrapper::NullGetter(v8::Local proper QV8ContextResource *resource = v8_resource_cast(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 QV8ContextWrapper::Getter(v8::Local property, QV8ContextResource *resource = v8_resource_cast(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 QV8ContextWrapper::NullSetter(v8::Local proper QV8ContextResource *resource = v8_resource_cast(info.This()); if (!resource) - return v8::Undefined(); // XXX Should we throw here? + return v8::Handle(); QV8Engine *engine = resource->engine; @@ -362,7 +368,7 @@ v8::Handle QV8ContextWrapper::NullSetter(v8::Local 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(); } } @@ -373,13 +379,14 @@ v8::Handle QV8ContextWrapper::Setter(v8::Local property, QV8ContextResource *resource = v8_resource_cast(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 3149569..12e01ba 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 qmlglobal, v8::Handle, QDeclarativeContextData *ctxt); - // XXX aakenned - remove this abomination + // XXX We only use the secondary scope to pass the "arguments" of the signal to + // on 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, QObject *); QDeclarativeContextData *callingContext(); diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp index 0314402..3ddac00 100644 --- a/src/declarative/qml/v8/qv8engine.cpp +++ b/src/declarative/qml/v8/qv8engine.cpp @@ -62,8 +62,8 @@ #include #include -// 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 jsstr) @@ -335,8 +336,8 @@ v8::Handle QV8Engine::fromVariant(const QVariant &variant) return v8::Null(); } } else if (type == qMetaTypeId >()) { - // XXX aakenned Can this be more optimal? Just use Array as a prototype and - // implement directly against QList? + // XXX Can this be made more by using Array as a prototype and implementing + // directly against QList? const QList &list = *(QList*)ptr; v8::Local array = v8::Array::New(list.count()); for (int ii = 0; ii < list.count(); ++ii) @@ -350,49 +351,12 @@ v8::Handle 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(ptr)); - break; -#endif -#ifndef QT_NO_QOBJECT -#endif - case QMetaType::QVariant: - result = eng->newVariant(*reinterpret_cast(ptr)); - break; - default: - if (type == qMetaTypeId()) { - result = eng->scriptValueToJSCValue(*reinterpret_cast(ptr)); - if (!result) - return JSC::jsUndefined(); - } - -#ifndef QT_NO_QOBJECT - // lazy registration of some common list types - else if (type == qMetaTypeId()) { - qScriptRegisterSequenceMetaType(eng->q_func()); - return create(exec, type, ptr); - } -#endif - else if (type == qMetaTypeId >()) { - qScriptRegisterSequenceMetaType >(eng->q_func()); - return create(exec, type, ptr); - } + // XXX TODO: To be compatible, we still need to handle: + // + QScriptValue + // + QObjectList + // + QList - else { - QByteArray typeName = QMetaType::typeName(type); - if (typeName.endsWith('*') && !*reinterpret_cast(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 global) console->Set(v8::String::New("log"), printFn); console->Set(v8::String::New("debug"), printFn); - // XXX - Qt global object properties - v8::Local qt = v8::Object::New(); // Set all the enums from the "Qt" namespace @@ -562,22 +524,21 @@ void QV8Engine::initializeGlobal(v8::Handle 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 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 freeze = v8::Script::New(v8::String::New(FREEZE_SOURCE)); + v8::Local result = freeze->Run(); + Q_ASSERT(result->IsFunction()); + m_freezeObject = qPersistentNew(v8::Local::Cast(result)); +#undef FREEZE_SOURCE + } } -void QV8Engine::freezeGlobal() +void QV8Engine::freezeObject(v8::Handle 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 test = v8::Script::New(v8::String::New(FREEZE)); -#undef FREEZE - - test->Run(); + v8::Handle args[] = { value }; + m_freezeObject->Call(global(), 1, args); } void QV8Engine::gc() @@ -1270,7 +1240,6 @@ v8::Handle QV8Engine::openUrlExternally(const v8::Arguments &args) v8::Handle 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 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 1843460..7398944 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 getOwnPropertyNames(v8::Handle); + void freezeObject(v8::Handle); inline QString toString(v8::Handle string); inline QString toString(v8::Handle string); @@ -295,6 +296,7 @@ private: QV8ValueTypeWrapper m_valueTypeWrapper; v8::Persistent m_getOwnPropertyNames; + v8::Persistent m_freezeObject; void *m_xmlHttpRequestData; void *m_sqlDatabaseData; @@ -307,7 +309,6 @@ private: QVariant toBasicVariant(v8::Handle); void initializeGlobal(v8::Handle); - void freezeGlobal(); static v8::Handle gc(const v8::Arguments &args); static v8::Handle print(const v8::Arguments &args); @@ -424,7 +425,8 @@ v8::Handle 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 string) { uint16_t buffer[2]; diff --git a/src/declarative/qml/v8/qv8include.cpp b/src/declarative/qml/v8/qv8include.cpp index 3b8a081..f8ef8a6 100644 --- a/src/declarative/qml/v8/qv8include.cpp +++ b/src/declarative/qml/v8/qv8include.cpp @@ -78,7 +78,7 @@ QV8Include::~QV8Include() v8::Local QV8Include::resultValue(Status status) { - // XXX aakenned inefficient + // XXX It seems inefficient to create this object from scratch each time. v8::Local 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 callback, { if (!callback.IsEmpty()) { v8::Handle 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 13b3b30..5f2d9fb 100644 --- a/src/declarative/qml/v8/qv8listwrapper.cpp +++ b/src/declarative/qml/v8/qv8listwrapper.cpp @@ -88,7 +88,7 @@ v8::Handle 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 rv = m_constructor->NewInstance(); QV8ListResource *r = new QV8ListResource(m_engine); r->object = object; @@ -101,7 +101,7 @@ v8::Handle QV8ListWrapper::newList(QObject *object, int propId, int p v8::Handle QV8ListWrapper::newList(const QDeclarativeListProperty &prop, int propType) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local 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 75a97ac..02eb346 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 QV8QObjectWrapper::GetProperty(QV8Engine *engine, QObject v8::Handle 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 create(QV8Engine *engine, QObject *object, v8::Handle *objectHandle, @@ -505,12 +506,9 @@ v8::Handle QV8QObjectWrapper::Getter(v8::Local property, return v8engine->typeWrapper()->newObject(object, data->typeNamespace, QV8TypeWrapper::ExcludeEnums); } } + } - return v8::Undefined(); - } else { - // XXX throw? - return v8::Undefined(); - } + return v8::Undefined(); } v8::Handle QV8QObjectWrapper::Setter(v8::Local property, @@ -584,8 +582,6 @@ v8::Handle 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 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 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 thisObject; v8::Persistent function; + + void dispose() { + qPersistentDispose(thisObject); + qPersistentDispose(function); + } + + bool needsDestroy; + }; + + struct ConnectionList : public QList { + ConnectionList() : connectionsInUse(0), connectionsNeedClean(false) {} + int connectionsInUse; + bool connectionsNeedClean; }; QV8Engine *engine; - typedef QHash > SlotHash; + typedef QHash 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(object), engine(engine) +: QDeclarativeGuard(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 &connections = *iter; - if (connections.isEmpty()) + ConnectionList &connectionList = *iter; + if (connectionList.isEmpty()) return -1; - // XXX optimize + inUse++; + + connectionList.connectionsInUse++; + + QList 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 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::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 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()); + slotIter = connectionList->slotHash.insert(signalIndex, QV8QObjectConnectionList::ConnectionList()); QMetaObject::connect(signalObject, signalIndex, connectionList, signalIndex); } @@ -1064,7 +1115,7 @@ v8::Handle QV8QObjectWrapper::Disconnect(const v8::Arguments &args) if (slotIter == connectionList->slotHash.end()) return v8::Undefined(); // Nothing to disconnect from - QList &connections = *slotIter; + QV8QObjectConnectionList::ConnectionList &connections = *slotIter; v8::Local function = v8::Local::Cast(functionValue); QPair functionData = ExtractQtMethod(engine, function); @@ -1080,9 +1131,12 @@ v8::Handle QV8QObjectWrapper::Disconnect(const v8::Arguments &args) QPair 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 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 MetaCallArgument::toValue(QV8Engine *engine) QDeclarativeData::get(object, true)->setImplicitDestructible(); return engine->newQObject(object); } else if (type == qMetaTypeId >()) { - // XXX aakenned Can this be more optimal? Just use Array as a prototype and - // implement directly against QList? + // XXX Can this be made more by using Array as a prototype and implementing + // directly against QList? QList &list = *(QList*)&data; v8::Local 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 b4a510e..70af603 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 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 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 QV8TypeWrapper::newObject(QObject *o, QDeclarativeType *t, v8::Local 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 rv = m_constructor->NewInstance(); QV8TypeResource *r = new QV8TypeResource(m_engine); t->addref(); @@ -169,8 +169,8 @@ v8::Handle QV8TypeWrapper::Getter(v8::Local 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 QV8TypeWrapper::Setter(v8::Local 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 b16a81e..f4f79ce 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 QV8ValueTypeWrapper::newValueType(QObject *object, int property, QDeclarativeValueType *type) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local 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 QV8ValueTypeWrapper::newValueType(QObject *object, int pro v8::Local QV8ValueTypeWrapper::newValueType(const QVariant &value, QDeclarativeValueType *type) { - // XXX aakenned - NewInstance() is slow for our case + // XXX NewInstance() should be optimized v8::Local rv = m_constructor->NewInstance(); QV8ValueTypeCopyResource *r = new QV8ValueTypeCopyResource(m_engine); r->type = type; r->value = value; @@ -172,9 +172,9 @@ v8::Handle QV8ValueTypeWrapper::Getter(v8::Local property QV8ValueTypeResource *r = v8_resource_cast(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 ef1f972..37e998d 100644 --- a/src/declarative/qml/v8/qv8variantwrapper.cpp +++ b/src/declarative/qml/v8/qv8variantwrapper.cpp @@ -106,7 +106,7 @@ v8::Local 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 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 846fdb7..0d396d3 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 #define ALIGN(size) (((size) + 3) & ~3) void QV8Worker::serialize(QByteArray &data, v8::Handle v, QV8Engine *engine) { @@ -166,7 +166,8 @@ void QV8Worker::serialize(QByteArray &data, v8::Handle 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 array = v8::Handle::Cast(v); @@ -238,7 +239,8 @@ void QV8Worker::serialize(QByteArray &data, v8::Handle 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(engine->toQObject(v)); if (lm && lm->agent()) { QDeclarativeListModelWorkerAgent *agent = lm->agent(); -- 2.7.4