Minor XXX fixups
authorAaron Kennedy <aaron.kennedy@nokia.com>
Thu, 9 Jun 2011 04:40:44 +0000 (14:40 +1000)
committerAaron Kennedy <aaron.kennedy@nokia.com>
Thu, 9 Jun 2011 04:40:44 +0000 (14:40 +1000)
20 files changed:
src/declarative/qml/qdeclarativebinding.cpp
src/declarative/qml/qdeclarativecomponent.cpp
src/declarative/qml/qdeclarativecontext_p.h
src/declarative/qml/qdeclarativeexpression.cpp
src/declarative/qml/qdeclarativeexpression_p.h
src/declarative/qml/qdeclarativetypeloader.cpp
src/declarative/qml/qdeclarativevme.cpp
src/declarative/qml/qdeclarativevmemetaobject.cpp
src/declarative/qml/qdeclarativexmlhttprequest.cpp
src/declarative/qml/v8/qv8contextwrapper.cpp
src/declarative/qml/v8/qv8contextwrapper_p.h
src/declarative/qml/v8/qv8engine.cpp
src/declarative/qml/v8/qv8engine_p.h
src/declarative/qml/v8/qv8include.cpp
src/declarative/qml/v8/qv8listwrapper.cpp
src/declarative/qml/v8/qv8qobjectwrapper.cpp
src/declarative/qml/v8/qv8typewrapper.cpp
src/declarative/qml/v8/qv8valuetypewrapper.cpp
src/declarative/qml/v8/qv8variantwrapper.cpp
src/declarative/qml/v8/qv8worker.cpp

index 106cc10..6cca327 100644 (file)
@@ -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<QObject*>(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);
-                }
             }
 
 
index c7e5bcb..e99928d 100644 (file)
@@ -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);
index e4312f1..93b9d3b 100644 (file)
@@ -150,9 +150,7 @@ public:
     QObject *contextObject;
 
     // Any script blocks that exist on this context
-    // XXX aakenned
     QList<v8::Persistent<v8::Object> > importedScripts;
-//    QList<QScriptValue> importedScripts;
 
     // Context base url
     QUrl url;
index 517d4fc..095d9be 100644 (file)
@@ -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<v8::Function> func,
                                          QObject *me)
 {
-    // XXX aakenned
-    // expression = func.toString();
-
     QDeclarativeAbstractExpression::setContext(ctxt);
     scopeObject = me;
 
     v8function = qPersistentNew<v8::Function>(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<v8::Object> scopeobject = ep->v8engine.qmlScope(ctxt, scope);
     v8::Local<v8::Script> script = ep->v8engine.qmlModeCompile(code, filename, line);
     v8::Local<v8::Value> result = script->Run(scopeobject);
+    if (tc.HasCaught()) return v8::Persistent<v8::Function>();
     if (qmlscope) *qmlscope = qPersistentNew<v8::Object>(scopeobject);
     return qPersistentNew<v8::Function>(v8::Local<v8::Function>::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<v8::Value>(d->v8function));
+    }
     return d->expression;
 }
 
@@ -472,7 +479,6 @@ v8::Local<v8::Value> 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<v8::Object> This;
 
@@ -494,6 +500,7 @@ v8::Local<v8::Value> QDeclarativeQtScriptExpression::eval(QObject *secondaryScop
 
     if (watcher.wasDeleted()) {
     } else if (try_catch.HasCaught()) {
+        v8::Context::Scope scope(ep->v8engine.context());
         v8::Local<v8::Message> message = try_catch.Message();
         if (!message.IsEmpty()) {
             QDeclarativeExpressionPrivate::exceptionToError(message, error);
index c183cf2..b03c709 100644 (file)
@@ -122,6 +122,7 @@ public:
     QDeclarativeRefCount *dataRef;
 
     QString expression;
+    bool extractExpressionFromFunction;
 
     Mode expressionFunctionMode;
     v8::Persistent<v8::Function> v8function;
index b4e6ef4..a788b4b 100644 (file)
@@ -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());
index c99551f..146c856 100644 (file)
@@ -995,7 +995,6 @@ v8::Persistent<v8::Object> 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());
 
index 218b1c1..247c1aa 100644 (file)
@@ -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<v8::Function> function = method(id);
                 QDeclarativeVMEMetaData::MethodData *data = metaData->methodData() + id;
 
index 22ff408..64719d5 100644 (file)
@@ -536,7 +536,7 @@ v8::Handle<v8::Object> 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<v8::Object> 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<v8::Object> 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<v8::Object> 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<v8::Object> 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<v8::Object> CDATA::prototype(QV8Engine *engine)
     if (d->cdataPrototype.IsEmpty()) {
         d->cdataPrototype = qPersistentNew<v8::Object>(v8::Object::New());
         d->cdataPrototype->SetPrototype(Text::prototype(engine));
-        // XXX freeze
+        engine->freezeObject(d->cdataPrototype);
     }
     return d->cdataPrototype;
 }
@@ -717,7 +717,7 @@ v8::Handle<v8::Object> 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<v8::Object> 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<v8::Object>(ot->NewInstance());
-        // XXX freeze
+        engine->freezeObject(d->namedNodeMapPrototype);
     }
     return d->namedNodeMapPrototype;
 }
@@ -906,7 +906,6 @@ v8::Handle<v8::Value> 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<v8::Object> 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<v8::Object>(ot->NewInstance());
-        // XXX freeze
+        engine->freezeObject(d->nodeListPrototype);
     }
     return d->nodeListPrototype;
 }
index 27002c6..f0e52b1 100644 (file)
@@ -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
 
index 3149569..12e01ba 100644 (file)
@@ -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();
index 0314402..3ddac00 100644 (file)
@@ -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();
 }
index 1843460..7398944 100644 (file)
@@ -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];
index 3b8a081..f8ef8a6 100644 (file)
@@ -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);
     }
 }
index 13b3b30..5f2d9fb 100644 (file)
@@ -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;
index 75a97ac..02eb346 100644 (file)
@@ -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) 
index b4a510e..70af603 100644 (file)
@@ -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;
index b16a81e..f4f79ce 100644 (file)
@@ -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());
index ef1f972..37e998d 100644 (file)
@@ -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);
 
index 846fdb7..0d396d3 100644 (file)
@@ -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();