Fix regression with meta object revision checking

Commit 00a07bcbd7a592072822b0e55ab2e75e90c3f9f5 replaced the CheckRevision
parameter on the call GetProperty on the scope QObject with a get that
would do IgnoreRevision.

This adds a property getter on the QV4::QObjectWrapper that allows for checking
the meta object revision. The plan is to move all of the property getter code
from QV8QObjectWrapper into QV4::QObjectWrapper incrementally.

Change-Id: I8e5a93ce3351a8c5dba13f14cd43e4036875b792
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

5 files changed, 44 insertions, 40 deletions

diff --git a/src/qml/qml/qqmlcontextwrapper.cpp b/src/qml/qml/qqmlcontextwrapper.cpp
index a4b8349aa8..f9dc3a3fc3 100644
--- a/src/qml/qml/qqmlcontextwrapper.cpp
+++ b/src/qml/qml/qqmlcontextwrapper.cpp
@@ -229,10 +229,9 @@ Value QmlContextWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bo
 
         // Search scope object
         if (scopeObject) {
-            QV4::Value wrapper = qobjectWrapper->newQObject(scopeObject)->v4Value();
-            if (QV4::Object *o = wrapper.asObject()) {
+            if (QV4::QObjectWrapper *o = qobjectWrapper->newQObject(scopeObject)->v4Value().as<QV4::QObjectWrapper>()) {
                 bool hasProp = false;
-                QV4::Value result = o->get(o->engine()->current, propertystring.string().asString(), &hasProp);
+                QV4::Value result = o->getProperty(o->engine()->current, propertystring.string().asString(), QV4::QObjectWrapper::CheckRevision, &hasProp);
                 if (hasProp) {
                     if (hasProperty)
                         *hasProperty = true;
@@ -246,7 +245,7 @@ Value QmlContextWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bo
         // Search context object
         if (context->contextObject) {
             QV4::Value result = qobjectWrapper->getProperty(context->contextObject, propertystring,
-                                                                       context, QV8QObjectWrapper::CheckRevision)->v4Value();
+                                                                       context, QV4::QObjectWrapper::CheckRevision)->v4Value();
             if (!result.isEmpty()) {
                 if (hasProperty)
                     *hasProperty = true;
@@ -299,14 +298,14 @@ void QmlContextWrapper::put(Managed *m, ExecutionContext *ctx, String *name, con
 
         // Search scope object
         if (scopeObject &&
-            qobjectWrapper->setProperty(scopeObject, propertystring, context, value, QV8QObjectWrapper::CheckRevision))
+            qobjectWrapper->setProperty(scopeObject, propertystring, context, value, QV4::QObjectWrapper::CheckRevision))
             return;
         scopeObject = 0;
 
         // Search context object
         if (context->contextObject &&
             qobjectWrapper->setProperty(context->contextObject, propertystring, context, value,
-                                        QV8QObjectWrapper::CheckRevision))
+                                        QV4::QObjectWrapper::CheckRevision))
             return;
 
         context = context->parent;
diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp
index 8cabebf5c4..5074590bcc 100644
--- a/src/qml/qml/qqmltypewrapper.cpp
+++ b/src/qml/qml/qqmltypewrapper.cpp
@@ -158,7 +158,7 @@ Value QmlTypeWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bool
                 }
 
                 // check for property.
-                return v8engine->qobjectWrapper()->getProperty(qobjectSingleton, propertystring, context, QV8QObjectWrapper::IgnoreRevision)->v4Value();
+                return v8engine->qobjectWrapper()->getProperty(qobjectSingleton, propertystring, context, QV4::QObjectWrapper::IgnoreRevision)->v4Value();
             } else if (!siinfo->scriptApi(e).isUndefined()) {
                 QV4::ExecutionEngine *engine = QV8Engine::getV4(v8engine);
                 // NOTE: if used in a binding, changes will not trigger re-evaluation since non-NOTIFYable.
@@ -183,7 +183,7 @@ Value QmlTypeWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bool
                 QObject *ao = qmlAttachedPropertiesObjectById(type->attachedPropertiesId(), object);
                 if (ao)
                     return v8engine->qobjectWrapper()->getProperty(ao, propertystring, context,
-                                                                   QV8QObjectWrapper::IgnoreRevision)->v4Value();
+                                                                   QV4::QObjectWrapper::IgnoreRevision)->v4Value();
 
                 // Fall through to base implementation
             }
@@ -243,7 +243,7 @@ void QmlTypeWrapper::put(Managed *m, ExecutionContext *ctx, String *name, const
         QObject *ao = qmlAttachedPropertiesObjectById(type->attachedPropertiesId(), object);
         if (ao) 
             v8engine->qobjectWrapper()->setProperty(ao, propertystring, context, value,
-                                                    QV8QObjectWrapper::IgnoreRevision);
+                                                    QV4::QObjectWrapper::IgnoreRevision);
     } else if (type && type->isSingleton()) {
         QQmlEngine *e = v8engine->engine();
         QQmlType::SingletonInstanceInfo *siinfo = type->singletonInstanceInfo();
@@ -252,7 +252,7 @@ void QmlTypeWrapper::put(Managed *m, ExecutionContext *ctx, String *name, const
         QObject *qobjectSingleton = siinfo->qobjectApi(e);
         if (qobjectSingleton) {
             v8engine->qobjectWrapper()->setProperty(qobjectSingleton, propertystring, context, value,
-                                                    QV8QObjectWrapper::IgnoreRevision);
+                                                    QV4::QObjectWrapper::IgnoreRevision);
         } else if (!siinfo->scriptApi(e).isUndefined()) {
             QV4::Object *apiprivate = QJSValuePrivate::get(siinfo->scriptApi(e))->value.asObject();
             if (!apiprivate) {
diff --git a/src/qml/qml/qqmlvaluetypewrapper.cpp b/src/qml/qml/qqmlvaluetypewrapper.cpp
index d17b50c1f3..abf0be7eff 100644
--- a/src/qml/qml/qqmlvaluetypewrapper.cpp
+++ b/src/qml/qml/qqmlvaluetypewrapper.cpp
@@ -284,7 +284,7 @@ Value QmlValueTypeWrapper::get(Managed *m, ExecutionContext *ctx, String *name,
     if (result->isFunction()) {
         // calling a Q_INVOKABLE function of a value type
         QQmlContextData *context = r->v8->callingContext();
-        return r->v8->qobjectWrapper()->getProperty(r->type, propertystring, context, QV8QObjectWrapper::IgnoreRevision)->v4Value();
+        return r->v8->qobjectWrapper()->getProperty(r->type, propertystring, context, QV4::QObjectWrapper::IgnoreRevision)->v4Value();
     }
 
 #define VALUE_TYPE_LOAD(metatype, cpptype, constructor) \
diff --git a/src/qml/qml/v8/qv8qobjectwrapper.cpp b/src/qml/qml/v8/qv8qobjectwrapper.cpp
index 084cd4f974..ed64b34ec1 100644
--- a/src/qml/qml/v8/qv8qobjectwrapper.cpp
+++ b/src/qml/qml/v8/qv8qobjectwrapper.cpp
@@ -119,24 +119,22 @@ void QObjectWrapper::deleteQObject(bool deleteInstantly)
     }
 }
 
-QV4::Value QObjectWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bool *hasProperty)
+Value QObjectWrapper::getProperty(ExecutionContext *ctx, String *name, QObjectWrapper::RevisionMode revisionMode, bool *hasProperty)
 {
-    QObjectWrapper *that = static_cast<QObjectWrapper*>(m);
-
-    if (QQmlData::wasDeleted(that->m_object)) {
+    if (QQmlData::wasDeleted(m_object)) {
         if (hasProperty)
             *hasProperty = false;
         return QV4::Value::undefinedValue();
     }
 
-    if (name->isEqualTo(that->m_destroy) || name->isEqualTo(that->m_toString)) {
+    if (name->isEqualTo(m_destroy) || name->isEqualTo(m_toString)) {
         bool hasProp = false;
-        QV4::Value method = QV4::Object::get(m, ctx, name, &hasProp);
+        QV4::Value method = QV4::Object::get(this, ctx, name, &hasProp);
 
         if (!hasProp) {
-            int index = name->isEqualTo(that->m_destroy) ? QV4::QObjectMethod::DestroyMethod : QV4::QObjectMethod::ToStringMethod;
-            method = QV4::Value::fromObject(new (ctx->engine->memoryManager) QV4::QObjectMethod(ctx->engine->rootContext, that->m_object, index, QV4::Value::undefinedValue()));
-            QV4::Object::put(m, ctx, name, method);
+            int index = name->isEqualTo(m_destroy) ? QV4::QObjectMethod::DestroyMethod : QV4::QObjectMethod::ToStringMethod;
+            method = QV4::Value::fromObject(new (ctx->engine->memoryManager) QV4::QObjectMethod(ctx->engine->rootContext, m_object, index, QV4::Value::undefinedValue()));
+            QV4::Object::put(this, ctx, name, method);
         }
 
         if (hasProperty)
@@ -147,11 +145,9 @@ QV4::Value QObjectWrapper::get(Managed *m, ExecutionContext *ctx, String *name,
 
     QHashedV4String propertystring(QV4::Value::fromString(name));
 
-    QV8Engine *v8engine = that->v8Engine;
-    QQmlContextData *context = v8engine->callingContext();
+    QQmlContextData *context = QV4::QmlContextWrapper::callingContext(ctx->engine);
 
-    v8::Handle<v8::Value> result = QV8QObjectWrapper::GetProperty(v8engine, that->m_object, propertystring,
-                                                                  context, QV8QObjectWrapper::IgnoreRevision);
+    v8::Handle<v8::Value> result = QV8QObjectWrapper::GetProperty(v8Engine, m_object, propertystring, context, revisionMode);
     if (!result.IsEmpty()) {
         if (hasProperty)
             *hasProperty = true;
@@ -167,16 +163,22 @@ QV4::Value QObjectWrapper::get(Managed *m, ExecutionContext *ctx, String *name,
                 if (r.scriptIndex != -1) {
                     return QV4::Value::undefinedValue();
                 } else if (r.type) {
-                    return QmlTypeWrapper::create(v8engine, that->m_object, r.type, QmlTypeWrapper::ExcludeEnums);
+                    return QmlTypeWrapper::create(v8Engine, m_object, r.type, QmlTypeWrapper::ExcludeEnums);
                 } else if (r.importNamespace) {
-                    return QmlTypeWrapper::create(v8engine, that->m_object, context->imports, r.importNamespace, QmlTypeWrapper::ExcludeEnums);
+                    return QmlTypeWrapper::create(v8Engine, m_object, context->imports, r.importNamespace, QmlTypeWrapper::ExcludeEnums);
                 }
                 Q_ASSERT(!"Unreachable");
             }
         }
     }
 
-    return QV4::Object::get(m, ctx, name, hasProperty);
+    return QV4::Object::get(this, ctx, name, hasProperty);
+}
+
+QV4::Value QObjectWrapper::get(Managed *m, ExecutionContext *ctx, String *name, bool *hasProperty)
+{
+    QObjectWrapper *that = static_cast<QObjectWrapper*>(m);
+    return that->getProperty(ctx, name, IgnoreRevision, hasProperty);
 }
 
 void QObjectWrapper::put(Managed *m, ExecutionContext *ctx, String *name, const Value &value)
@@ -192,7 +194,7 @@ void QObjectWrapper::put(Managed *m, ExecutionContext *ctx, String *name, const
 
     QV8Engine *v8engine = that->v8Engine;
     QQmlContextData *context = v8engine->callingContext();
-    bool result = QV8QObjectWrapper::SetProperty(v8engine, object, propertystring, context, value, QV8QObjectWrapper::IgnoreRevision);
+    bool result = QV8QObjectWrapper::SetProperty(v8engine, object, propertystring, context, value, QV4::QObjectWrapper::IgnoreRevision);
 
     if (!result) {
         QString error = QLatin1String("Cannot assign to non-existent property \"") +
@@ -508,7 +510,7 @@ static QV4::Value LoadProperty(QV8Engine *engine, QObject *object,
 QV4::Value QV8QObjectWrapper::GetProperty(QV8Engine *engine, QObject *object,
                                                      const QHashedV4String &property,
                                                      QQmlContextData *context,
-                                                     QV8QObjectWrapper::RevisionMode revisionMode)
+                                                     QV4::QObjectWrapper::RevisionMode revisionMode)
 {
     // XXX More recent versions of V8 introduced "Callable" objects.  It is possible that these
     // will be a faster way of creating QObject method objects.
@@ -543,7 +545,7 @@ QV4::Value QV8QObjectWrapper::GetProperty(QV8Engine *engine, QObject *object,
 
     QQmlData::flushPendingBinding(object, result->coreIndex);
 
-    if (revisionMode == QV8QObjectWrapper::CheckRevision && result->hasRevision()) {
+    if (revisionMode == QV4::QObjectWrapper::CheckRevision && result->hasRevision()) {
         QQmlData *ddata = QQmlData::get(object);
         if (ddata && ddata->propertyCache && !ddata->propertyCache->isAllowedInRevision(result))
             return QV4::Value::emptyValue();
@@ -721,7 +723,7 @@ static inline void StoreProperty(QV8Engine *engine, QObject *object, QQmlPropert
 }
 
 bool QV8QObjectWrapper::SetProperty(QV8Engine *engine, QObject *object, const QHashedV4String &property, QQmlContextData *context,
-                                    v8::Handle<v8::Value> value, QV8QObjectWrapper::RevisionMode revisionMode)
+                                    v8::Handle<v8::Value> value, QV4::QObjectWrapper::RevisionMode revisionMode)
 {
     if (QQmlData::wasDeleted(object))
         return false;
@@ -733,7 +735,7 @@ bool QV8QObjectWrapper::SetProperty(QV8Engine *engine, QObject *object, const QH
     if (!result)
         return false;
 
-    if (revisionMode == QV8QObjectWrapper::CheckRevision && result->hasRevision()) {
+    if (revisionMode == QV4::QObjectWrapper::CheckRevision && result->hasRevision()) {
         QQmlData *ddata = QQmlData::get(object);
         if (ddata && ddata->propertyCache && !ddata->propertyCache->isAllowedInRevision(result))
             return false;
diff --git a/src/qml/qml/v8/qv8qobjectwrapper_p.h b/src/qml/qml/v8/qv8qobjectwrapper_p.h
index b88b3cea99..f8d128fc57 100644
--- a/src/qml/qml/v8/qv8qobjectwrapper_p.h
+++ b/src/qml/qml/v8/qv8qobjectwrapper_p.h
@@ -83,7 +83,9 @@ struct Q_QML_EXPORT QObjectWrapper : public QV4::Object
 {
     Q_MANAGED
 
-    QObjectWrapper(ExecutionEngine *v8Engine, QObject *m_object);
+    enum RevisionMode { IgnoreRevision, CheckRevision };
+
+    QObjectWrapper(ExecutionEngine *v8Engine, QObject *object);
     ~QObjectWrapper();
 
     QV8Engine *v8Engine; // ### Remove again.
@@ -92,6 +94,8 @@ struct Q_QML_EXPORT QObjectWrapper : public QV4::Object
 
     void deleteQObject(bool deleteInstantly = false);
 
+    Value getProperty(ExecutionContext *ctx, String *name, RevisionMode revisionMode, bool *hasProperty = 0);
+
 private:
     QQmlGuard<QObject> m_object;
     String *m_destroy;
@@ -170,9 +174,8 @@ public:
 
     v8::Handle<v8::Value> newQObject(QObject *object);
 
-    enum RevisionMode { IgnoreRevision, CheckRevision };
-    inline v8::Handle<v8::Value> getProperty(QObject *, const QHashedV4String &, QQmlContextData *, RevisionMode);
-    inline bool setProperty(QObject *, const QHashedV4String &, QQmlContextData *, v8::Handle<v8::Value>, RevisionMode);
+    inline v8::Handle<v8::Value> getProperty(QObject *, const QHashedV4String &, QQmlContextData *, QV4::QObjectWrapper::RevisionMode);
+    inline bool setProperty(QObject *, const QHashedV4String &, QQmlContextData *, v8::Handle<v8::Value>, QV4::QObjectWrapper::RevisionMode);
 
 private:
     friend class QQmlPropertyCache;
@@ -182,9 +185,9 @@ private:
 
     v8::Handle<v8::Object> newQObject(QObject *, QQmlData *, QV8Engine *);
     static QV4::Value GetProperty(QV8Engine *, QObject *,
-                                             const QHashedV4String &, QQmlContextData *, QV8QObjectWrapper::RevisionMode);
+                                             const QHashedV4String &, QQmlContextData *, QV4::QObjectWrapper::RevisionMode);
     static bool SetProperty(QV8Engine *, QObject *, const QHashedV4String &, QQmlContextData *,
-                            v8::Handle<v8::Value>, QV8QObjectWrapper::RevisionMode);
+                            v8::Handle<v8::Value>, QV4::QObjectWrapper::RevisionMode);
     static QV4::Value Connect(QV4::SimpleCallContext *ctx);
     static QV4::Value Disconnect(QV4::SimpleCallContext *ctx);
     static QPair<QObject *, int> ExtractQtMethod(QV8Engine *, QV4::FunctionObject *);
@@ -198,7 +201,7 @@ private:
 };
 
 v8::Handle<v8::Value> QV8QObjectWrapper::getProperty(QObject *object, const QHashedV4String &string,
-                                                     QQmlContextData *context, RevisionMode mode)
+                                                     QQmlContextData *context, QV4::QObjectWrapper::RevisionMode mode)
 {
     QQmlData *dd = QQmlData::get(object, false);
     if (!dd || !dd->propertyCache ||
@@ -210,7 +213,7 @@ v8::Handle<v8::Value> QV8QObjectWrapper::getProperty(QObject *object, const QHas
 }
 
 bool QV8QObjectWrapper::setProperty(QObject *object, const QHashedV4String &string,
-                                    QQmlContextData *context, v8::Handle<v8::Value> value, RevisionMode mode)
+                                    QQmlContextData *context, v8::Handle<v8::Value> value, QV4::QObjectWrapper::RevisionMode mode)
 {
     QQmlData *dd = QQmlData::get(object, false);
     if (!dd || !dd->propertyCache ||