From cf959b4b4ea3d2dfd5243022fea393fadfd95b0d Mon Sep 17 00:00:00 2001
From: J-P Nurmi <jpnurmi@theqtcompany.com>
Date: Wed, 29 Oct 2014 13:59:46 +0100
Subject: [PATCH] Repeater & itemviews: fix setModel() JS array handling

QVariant comparison in setModel() started failing because
JS arrays are now passed as a QJSValue. Re-assigning the
same array content should not trigger a model change.

This change restores the old behavior it had before, when
JS arrays were passed as QVariantLists.

Change-Id: I1882b3531f2893b116dbd817edeecab1ae812ce8
Reviewed-by: Simon Hausmann <simon.hausmann@digia.com>
---
 src/quick/items/qquickitemview.cpp            |  6 +++-
 src/quick/items/qquickpathview.cpp            |  6 +++-
 src/quick/items/qquickrepeater.cpp            |  6 +++-
 .../qquickgridview/tst_qquickgridview.cpp     | 29 +++++++++++++++++++
 .../qquicklistview/tst_qquicklistview.cpp     | 29 +++++++++++++++++++
 .../qquickpathview/tst_qquickpathview.cpp     | 28 ++++++++++++++++++
 .../qquickrepeater/tst_qquickrepeater.cpp     | 28 ++++++++++++++++++
 7 files changed, 129 insertions(+), 3 deletions(-)

diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 2fd79715e1..93cb5e4e9d 100644
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -273,9 +273,13 @@ QVariant QQuickItemView::model() const
     return d->modelVariant;
 }
 
-void QQuickItemView::setModel(const QVariant &model)
+void QQuickItemView::setModel(const QVariant &m)
 {
     Q_D(QQuickItemView);
+    QVariant model = m;
+    if (model.userType() == qMetaTypeId<QJSValue>())
+        model = model.value<QJSValue>().toVariant();
+
     if (d->modelVariant == model)
         return;
     if (d->model) {
diff --git a/src/quick/items/qquickpathview.cpp b/src/quick/items/qquickpathview.cpp
index 825845eca9..6cf3e33de9 100644
--- a/src/quick/items/qquickpathview.cpp
+++ b/src/quick/items/qquickpathview.cpp
@@ -607,9 +607,13 @@ QVariant QQuickPathView::model() const
     return d->modelVariant;
 }
 
-void QQuickPathView::setModel(const QVariant &model)
+void QQuickPathView::setModel(const QVariant &m)
 {
     Q_D(QQuickPathView);
+    QVariant model = m;
+    if (model.userType() == qMetaTypeId<QJSValue>())
+        model = model.value<QJSValue>().toVariant();
+
     if (d->modelVariant == model)
         return;
 
diff --git a/src/quick/items/qquickrepeater.cpp b/src/quick/items/qquickrepeater.cpp
index 8e13947d78..e2a3043857 100644
--- a/src/quick/items/qquickrepeater.cpp
+++ b/src/quick/items/qquickrepeater.cpp
@@ -183,9 +183,13 @@ QVariant QQuickRepeater::model() const
     return d->dataSource;
 }
 
-void QQuickRepeater::setModel(const QVariant &model)
+void QQuickRepeater::setModel(const QVariant &m)
 {
     Q_D(QQuickRepeater);
+    QVariant model = m;
+    if (model.userType() == qMetaTypeId<QJSValue>())
+        model = model.value<QJSValue>().toVariant();
+
     if (d->dataSource == model)
         return;
 
diff --git a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
index b65aad543d..123d7f5032 100644
--- a/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
+++ b/tests/auto/quick/qquickgridview/tst_qquickgridview.cpp
@@ -204,6 +204,8 @@ private slots:
     void displayMargin();
     void negativeDisplayMargin();
 
+    void jsArrayChange();
+
 private:
     QList<int> toIntList(const QVariantList &list);
     void matchIndexLists(const QVariantList &indexLists, const QList<int> &expectedIndexes);
@@ -6430,6 +6432,33 @@ void tst_QQuickGridView::negativeDisplayMargin()
     delete window;
 }
 
+void tst_QQuickGridView::jsArrayChange()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import QtQuick 2.4; GridView {}", QUrl());
+
+    QScopedPointer<QQuickGridView> view(qobject_cast<QQuickGridView *>(component.create()));
+    QVERIFY(!view.isNull());
+
+    QSignalSpy spy(view.data(), SIGNAL(modelChanged()));
+    QVERIFY(spy.isValid());
+
+    QJSValue array1 = engine.newArray(3);
+    QJSValue array2 = engine.newArray(3);
+    for (int i = 0; i < 3; ++i) {
+        array1.setProperty(i, i);
+        array2.setProperty(i, i);
+    }
+
+    view->setModel(QVariant::fromValue(array1));
+    QCOMPARE(spy.count(), 1);
+
+    // no change
+    view->setModel(QVariant::fromValue(array2));
+    QCOMPARE(spy.count(), 1);
+}
+
 QTEST_MAIN(tst_QQuickGridView)
 
 #include "tst_qquickgridview.moc"
diff --git a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
index 5122b6aa38..9e8a813d54 100644
--- a/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
+++ b/tests/auto/quick/qquicklistview/tst_qquicklistview.cpp
@@ -239,6 +239,8 @@ private slots:
     void QTBUG_39492_data();
     void QTBUG_39492();
 
+    void jsArrayChange();
+
 private:
     template <class T> void items(const QUrl &source);
     template <class T> void changed(const QUrl &source);
@@ -7945,6 +7947,33 @@ void tst_QQuickListView::QTBUG_39492()
     releaseView(window);
 }
 
+void tst_QQuickListView::jsArrayChange()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import QtQuick 2.4; ListView {}", QUrl());
+
+    QScopedPointer<QQuickListView> view(qobject_cast<QQuickListView *>(component.create()));
+    QVERIFY(!view.isNull());
+
+    QSignalSpy spy(view.data(), SIGNAL(modelChanged()));
+    QVERIFY(spy.isValid());
+
+    QJSValue array1 = engine.newArray(3);
+    QJSValue array2 = engine.newArray(3);
+    for (int i = 0; i < 3; ++i) {
+        array1.setProperty(i, i);
+        array2.setProperty(i, i);
+    }
+
+    view->setModel(QVariant::fromValue(array1));
+    QCOMPARE(spy.count(), 1);
+
+    // no change
+    view->setModel(QVariant::fromValue(array2));
+    QCOMPARE(spy.count(), 1);
+}
+
 QTEST_MAIN(tst_QQuickListView)
 
 #include "tst_qquicklistview.moc"
diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
index 0e9994899f..992b2347dd 100644
--- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
+++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp
@@ -139,6 +139,7 @@ private slots:
     void changePathDuringRefill();
     void nestedinFlickable();
     void flickableDelegate();
+    void jsArrayChange();
 };
 
 class TestObject : public QObject
@@ -2290,6 +2291,33 @@ void tst_QQuickPathView::flickableDelegate()
     QCOMPARE(moveStartedSpy.count(), 0);
 }
 
+void tst_QQuickPathView::jsArrayChange()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import QtQuick 2.4; PathView {}", QUrl());
+
+    QScopedPointer<QQuickPathView> view(qobject_cast<QQuickPathView *>(component.create()));
+    QVERIFY(!view.isNull());
+
+    QSignalSpy spy(view.data(), SIGNAL(modelChanged()));
+    QVERIFY(spy.isValid());
+
+    QJSValue array1 = engine.newArray(3);
+    QJSValue array2 = engine.newArray(3);
+    for (int i = 0; i < 3; ++i) {
+        array1.setProperty(i, i);
+        array2.setProperty(i, i);
+    }
+
+    view->setModel(QVariant::fromValue(array1));
+    QCOMPARE(spy.count(), 1);
+
+    // no change
+    view->setModel(QVariant::fromValue(array2));
+    QCOMPARE(spy.count(), 1);
+}
+
 QTEST_MAIN(tst_QQuickPathView)
 
 #include "tst_qquickpathview.moc"
diff --git a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
index 1396dd4783..258eaee981 100644
--- a/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
+++ b/tests/auto/quick/qquickrepeater/tst_qquickrepeater.cpp
@@ -72,6 +72,7 @@ private slots:
     void dynamicModelCrash();
     void visualItemModelCrash();
     void invalidContextCrash();
+    void jsArrayChange();
 };
 
 class TestObject : public QObject
@@ -779,6 +780,33 @@ void tst_QQuickRepeater::invalidContextCrash()
     root.reset(0);
 }
 
+void tst_QQuickRepeater::jsArrayChange()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import QtQuick 2.4; Repeater {}", QUrl());
+
+    QScopedPointer<QQuickRepeater> repeater(qobject_cast<QQuickRepeater *>(component.create()));
+    QVERIFY(!repeater.isNull());
+
+    QSignalSpy spy(repeater.data(), SIGNAL(modelChanged()));
+    QVERIFY(spy.isValid());
+
+    QJSValue array1 = engine.newArray(3);
+    QJSValue array2 = engine.newArray(3);
+    for (int i = 0; i < 3; ++i) {
+        array1.setProperty(i, i);
+        array2.setProperty(i, i);
+    }
+
+    repeater->setModel(QVariant::fromValue(array1));
+    QCOMPARE(spy.count(), 1);
+
+    // no change
+    repeater->setModel(QVariant::fromValue(array2));
+    QCOMPARE(spy.count(), 1);
+}
+
 QTEST_MAIN(tst_QQuickRepeater)
 
 #include "tst_qquickrepeater.moc"
-- 
GitLab