Variants should compare as equal

We need to implement an object comparison callback to ensure
that two variants with identical values (although different JS
objects) compare as equal.

We also add a v8 autotest for this callback.

Change-Id: Idd1ab602d31b398a937d4df4a7bd091aa205de24
Reviewed-on: http://codereview.qt.nokia.com/989
Reviewed-by: Aaron Kennedy <aaron.kennedy@nokia.com>

diff --git a/src/declarative/qml/v8/qv8engine.cpp b/src/declarative/qml/v8/qv8engine.cpp
index cb89d0f..67679b9 100644 (file)
--- a/src/declarative/qml/v8/qv8engine.cpp
+++ b/src/declarative/qml/v8/qv8engine.cpp
@@ -66,6 +66,34 @@
 // QDeclarativeEngine is not available
 QT_BEGIN_NAMESPACE
 
+static bool ObjectComparisonCallback(v8::Local<v8::Object> lhs, v8::Local<v8::Object> rhs)
+{
+    if (lhs == rhs)
+        return true;
+
+    QV8ObjectResource *lhsr = static_cast<QV8ObjectResource*>(lhs->GetExternalResource());
+    QV8ObjectResource *rhsr = static_cast<QV8ObjectResource*>(rhs->GetExternalResource());
+
+    Q_ASSERT(lhsr->engine == rhsr->engine);
+
+    if (lhsr && rhsr) {
+        QV8ObjectResource::ResourceType lhst = lhsr->resourceType();
+        QV8ObjectResource::ResourceType rhst = rhsr->resourceType();
+
+        switch (lhst) {
+        case QV8ObjectResource::VariantType:
+            if (rhst == QV8ObjectResource::VariantType)
+                return lhsr->engine->variantWrapper()->toVariant(lhsr) == 
+                       lhsr->engine->variantWrapper()->toVariant(rhsr);
+            break;
+        default:
+            break;
+        }
+    }
+
+    return false;
+}
+
 QV8Engine::QV8Engine()
 : m_xmlHttpRequestData(0), m_sqlDatabaseData(0), m_listModelData(0)
 {
@@ -109,6 +137,8 @@ void QV8Engine::init(QDeclarativeEngine *engine)
     qPersistentRegister(m_context);
     v8::Context::Scope context_scope(m_context);
 
+    v8::V8::SetUserObjectComparisonCallbackFunction(ObjectComparisonCallback);
+
     m_stringWrapper.init();
     m_contextWrapper.init(this);
     m_qobjectWrapper.init(this);
@@ -658,6 +688,7 @@ void QV8Engine::setExtensionData(int index, Deletable *data)
 
 v8::Handle<v8::Value> QV8Engine::gc(const v8::Arguments &args)
 {
+    Q_UNUSED(args);
     gc();
     return v8::Undefined();
 }

diff --git a/src/declarative/qml/v8/qv8variantwrapper.cpp b/src/declarative/qml/v8/qv8variantwrapper.cpp
index 9d486d0..a5602fb 100644 (file)
--- a/src/declarative/qml/v8/qv8variantwrapper.cpp
+++ b/src/declarative/qml/v8/qv8variantwrapper.cpp
@@ -76,6 +76,7 @@ void QV8VariantWrapper::init(QV8Engine *engine)
     v8::Local<v8::FunctionTemplate> ft = v8::FunctionTemplate::New();
     ft->InstanceTemplate()->SetFallbackPropertyHandler(Getter, Setter);
     ft->InstanceTemplate()->SetHasExternalResource(true);
+    ft->InstanceTemplate()->MarkAsUseUserObjectComparison();
     ft->InstanceTemplate()->SetAccessor(v8::String::New("toString"), ToStringGetter, 0, 
                                         m_toString, v8::DEFAULT, 
                                         v8::PropertyAttribute(v8::ReadOnly | v8::DontDelete));
@@ -87,6 +88,7 @@ void QV8VariantWrapper::init(QV8Engine *engine)
     v8::Local<v8::FunctionTemplate> ft = v8::FunctionTemplate::New();
     ft->InstanceTemplate()->SetFallbackPropertyHandler(Getter, Setter);
     ft->InstanceTemplate()->SetHasExternalResource(true);
+    ft->InstanceTemplate()->MarkAsUseUserObjectComparison();
     ft->InstanceTemplate()->SetAccessor(v8::String::New("preserve"), PreserveGetter, 0, 
                                         m_preserve, v8::DEFAULT, 
                                         v8::PropertyAttribute(v8::ReadOnly | v8::DontDelete));

diff --git a/tests/auto/declarative/v8/tst_v8.cpp b/tests/auto/declarative/v8/tst_v8.cpp
index 4749da0..32d100e 100644 (file)
--- a/tests/auto/declarative/v8/tst_v8.cpp
+++ b/tests/auto/declarative/v8/tst_v8.cpp
@@ -54,6 +54,7 @@ private slots:
     void cleanupTestCase() {}
 
     void eval();
+    void userobjectcompare();
 };
 
 void tst_v8::eval()
@@ -61,6 +62,11 @@ void tst_v8::eval()
     QVERIFY(v8test_eval());
 }
 
+void tst_v8::userobjectcompare()
+{
+    QVERIFY(v8test_userobjectcompare());
+}
+
 int main(int argc, char *argv[]) 
 { 
     V8::SetFlagsFromCommandLine(&argc, argv, true);

diff --git a/tests/auto/declarative/v8/v8main.cpp b/tests/auto/declarative/v8/v8main.cpp
index bf37e2d..5930f53 100644 (file)
--- a/tests/auto/declarative/v8/v8main.cpp
+++ b/tests/auto/declarative/v8/v8main.cpp
@@ -11,6 +11,7 @@ int main(int argc, char *argv[])
     v8::V8::SetFlagsFromCommandLine(&argc, argv, true);
 
     RUN_TEST(eval);
+    RUN_TEST(userobjectcompare);
 
     return -1;
 }

diff --git a/tests/auto/declarative/v8/v8test.cpp b/tests/auto/declarative/v8/v8test.cpp
index 9b80b08..27d39c5 100644 (file)
--- a/tests/auto/declarative/v8/v8test.cpp
+++ b/tests/auto/declarative/v8/v8test.cpp
@@ -48,6 +48,7 @@ using namespace v8;
 
 #define VERIFY(expr) { \
     if (!(expr)) { \
+        fprintf(stderr, "FAIL: %s:%d %s\n", __FILE__, __LINE__, # expr); \
         _testPassed = false; \
         goto cleanup; \
     }  \
@@ -59,11 +60,11 @@ bool v8test_eval()
     BEGINTEST();
 
     HandleScope handle_scope;
-    v8::Persistent<v8::Context> context = Context::New();
+    Persistent<Context> context = Context::New();
     Context::Scope context_scope(context);
 
     Local<Object> qmlglobal = Object::New();
-    qmlglobal->Set(String::New("a"), v8::Integer::New(1922));
+    qmlglobal->Set(String::New("a"), Integer::New(1922));
 
     Local<Script> script = Script::Compile(String::New("eval(\"a\")"), NULL, NULL, 
                                            Handle<String>(), Script::QmlMode);
@@ -80,3 +81,174 @@ cleanup:
     ENDTEST();
 }
 
+static int userObjectComparisonCalled = 0;
+static bool userObjectComparisonReturn = false;
+static Local<Object> expectedLhs;
+static Local<Object> expectedRhs;
+static bool expectedObjectsCompared = false;
+
+#define SET_EXPECTED(lhs, rhs) { \
+    expectedObjectsCompared = false; \
+    expectedLhs = lhs; \
+    expectedRhs = rhs; \
+}
+
+static bool UserObjectComparison(Local<Object> lhs, Local<Object> rhs)
+{
+    userObjectComparisonCalled++;
+
+    expectedObjectsCompared = (lhs == expectedLhs && rhs == expectedRhs);
+
+    return userObjectComparisonReturn;
+}
+
+inline bool runscript(const char *source) {
+    Local<Script> script = Script::Compile(String::New(source));
+    Local<Value> result = script->Run();
+    return result->BooleanValue();
+}
+
+bool v8test_userobjectcompare()
+{
+    BEGINTEST();
+
+    HandleScope handle_scope;
+    Persistent<Context> context = Context::New();
+    Context::Scope context_scope(context);
+
+    V8::SetUserObjectComparisonCallbackFunction(UserObjectComparison);
+
+    Local<ObjectTemplate> ot = ObjectTemplate::New();
+    ot->MarkAsUseUserObjectComparison();
+
+    Local<Object> uoc1 = ot->NewInstance();
+    Local<Object> uoc2 = ot->NewInstance();
+    context->Global()->Set(String::New("uoc1a"), uoc1);
+    context->Global()->Set(String::New("uoc1b"), uoc1);
+    context->Global()->Set(String::New("uoc2"), uoc2);
+    Local<Object> obj1 = Object::New();
+    context->Global()->Set(String::New("obj1a"), obj1);
+    context->Global()->Set(String::New("obj1b"), obj1);
+    context->Global()->Set(String::New("obj2"), Object::New());
+    Local<String> string1 = String::New("Hello World");
+    context->Global()->Set(String::New("string1a"), string1);
+    context->Global()->Set(String::New("string1b"), string1);
+    context->Global()->Set(String::New("string2"), v8::String::New("Goodbye World"));
+
+    // XXX Opportunity for optimization - don't invoke user callback if objects are
+    // equal.  
+#if 0
+    userObjectComparisonCalled = 0; userObjectComparisonReturn = false;
+    VERIFY(true == runscript("uoc1a == uoc1b"));
+    VERIFY(userObjectComparisonCalled == 0);
+#endif
+
+    // Comparing two uoc objects invokes uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(false == runscript("uoc1a == uoc2"));
+    VERIFY(userObjectComparisonCalled == 1);
+
+    VERIFY(false == runscript("uoc2 == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 2);
+    userObjectComparisonReturn = true;
+    VERIFY(true == runscript("uoc1a == uoc2"));
+    VERIFY(userObjectComparisonCalled == 3);
+    VERIFY(true == runscript("uoc2 == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 4);
+
+    // != on two uoc object invokes uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(true == runscript("uoc1a != uoc2"));
+    VERIFY(userObjectComparisonCalled == 1);
+    VERIFY(true == runscript("uoc2 != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 2);
+    userObjectComparisonReturn = true;
+    VERIFY(false == runscript("uoc1a != uoc2"));
+    VERIFY(userObjectComparisonCalled == 3);
+    VERIFY(false == runscript("uoc2 != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 4);
+
+    // Comparison against a non-object doesn't invoke uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(false == runscript("uoc1a == string1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    VERIFY(false == runscript("string1a == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    VERIFY(false == runscript("2 == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    VERIFY(true == runscript("uoc1a != string1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    VERIFY(true == runscript("string1a != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    VERIFY(true == runscript("2 != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 0);
+    
+    // Comparison against a non-uoc-object still invokes uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(false == runscript("uoc1a == obj1a"));
+    VERIFY(userObjectComparisonCalled == 1);
+    VERIFY(false == runscript("obj1a == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 2);
+    userObjectComparisonReturn = true;
+    VERIFY(true == runscript("uoc1a == obj1a"));
+    VERIFY(userObjectComparisonCalled == 3);
+    VERIFY(true == runscript("obj1a == uoc1a"));
+    VERIFY(userObjectComparisonCalled == 4);
+
+    // != comparison against a non-uoc-object still invokes uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(true == runscript("uoc1a != obj1a"));
+    VERIFY(userObjectComparisonCalled == 1);
+    VERIFY(true == runscript("obj1a != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 2);
+    userObjectComparisonReturn = true;
+    VERIFY(false == runscript("uoc1a != obj1a"));
+    VERIFY(userObjectComparisonCalled == 3);
+    VERIFY(false == runscript("obj1a != uoc1a"));
+    VERIFY(userObjectComparisonCalled == 4);
+
+    // Comparing two non-uoc objects does not invoke uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    VERIFY(true == runscript("obj1a == obj1a"));
+    VERIFY(true == runscript("obj1a == obj1b"));
+    VERIFY(false == runscript("obj1a == obj2"));
+    VERIFY(false == runscript("obj1a == string1a"));
+    VERIFY(true == runscript("string1a == string1a"));
+    VERIFY(true == runscript("string1a == string1b"));
+    VERIFY(false == runscript("string1a == string2"));
+    VERIFY(userObjectComparisonCalled == 0);
+
+    // Correct lhs and rhs passed to uoc
+    userObjectComparisonCalled = 0; 
+    userObjectComparisonReturn = false;
+    SET_EXPECTED(uoc1, uoc2);
+    VERIFY(false == runscript("uoc1a == uoc2"));
+    VERIFY(true == expectedObjectsCompared);
+    SET_EXPECTED(uoc2, uoc1);
+    VERIFY(false == runscript("uoc2 == uoc1a"));
+    VERIFY(true == expectedObjectsCompared);
+    SET_EXPECTED(uoc1, uoc2);
+    VERIFY(true == runscript("uoc1a != uoc2"));
+    VERIFY(true == expectedObjectsCompared);
+    SET_EXPECTED(uoc2, uoc1);
+    VERIFY(true == runscript("uoc2 != uoc1a"));
+    VERIFY(true == expectedObjectsCompared);
+    SET_EXPECTED(uoc1, obj1);
+    VERIFY(false == runscript("uoc1a == obj1a"));
+    VERIFY(true == expectedObjectsCompared);
+    SET_EXPECTED(obj1, uoc1);
+    VERIFY(false == runscript("obj1a == uoc1a"));
+    VERIFY(true == expectedObjectsCompared);
+
+cleanup:
+    V8::SetUserObjectComparisonCallbackFunction(0);
+    context.Dispose();
+
+    ENDTEST();
+}

diff --git a/tests/auto/declarative/v8/v8test.h b/tests/auto/declarative/v8/v8test.h
index f887846..31acefc 100644 (file)
--- a/tests/auto/declarative/v8/v8test.h
+++ b/tests/auto/declarative/v8/v8test.h
@@ -45,6 +45,7 @@
 #include "../../../../src/3rdparty/v8/include/v8.h"
 
 bool v8test_eval();
+bool v8test_userobjectcompare();
 
 #endif // V8TEST_H