Fix deferred execution

If the QML engine refuses to defer execution of a delegate (it contains
an ID), we must make sure to cancel any pending deferred execution for
the same delegate. Otherwise, we may end up overriding a custom (non-
deferred) delegate with a default (deferred) delegate.

This patch adds a new test style "identified" to tst_customization.
This style contains delegates with IDs so we can test the behavior with
IDs in base styles. Furthermore, overriding delegates is now tested
in various ways (with and without IDs in the base and custom styles) in
a separate test method. This is done by generating QML code to override
delegates with dummy Item instances with appropriate IDs and names.

Task-number: QTBUG-65341
Change-Id: Ie6dca287cb74672004d9d8f599760b9d32c3a380
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

1 files changed, 125 insertions, 6 deletions

diff --git a/tests/auto/customization/tst_customization.cpp b/tests/auto/customization/tst_customization.cpp
index 4c8ea72b..48dd4048 100644
--- a/tests/auto/customization/tst_customization.cpp
+++ b/tests/auto/customization/tst_customization.cpp
@@ -115,6 +115,9 @@ private slots:
     void creation_data();
     void creation();
 
+    void override_data();
+    void override();
+
     void comboPopup();
 
 private:
@@ -122,7 +125,7 @@ private:
     void addHooks();
     void removeHooks();
 
-    QObject* createControl(const QString &type, QString *error);
+    QObject* createControl(const QString &type, const QString &qml, QString *error);
 
     QQmlEngine *engine = nullptr;
 };
@@ -190,10 +193,10 @@ void tst_customization::reset()
     qt_destroyedParentQObjects()->clear();
 }
 
-QObject* tst_customization::createControl(const QString &name, QString *error)
+QObject* tst_customization::createControl(const QString &name, const QString &qml, QString *error)
 {
     QQmlComponent component(engine);
-    component.setData("import QtQuick.Controls 2.2; " + name.toUtf8() + " { }", QUrl());
+    component.setData("import QtQuick 2.9; import QtQuick.Controls 2.2; " + name.toUtf8() + " { " + qml.toUtf8() + " }", QUrl());
     QObject *obj = component.create();
     if (!obj)
         *error = component.errorString();
@@ -214,6 +217,10 @@ void tst_customization::creation_data()
     for (const ControlInfo &control : ControlInfos)
         QTest::newRow(qPrintable("incomplete:" + control.type)) << "incomplete" << control.type << control.delegates;
 
+    // the "identified" style has IDs in the delegates (prevents deferred execution)
+    for (const ControlInfo &control : ControlInfos)
+        QTest::newRow(qPrintable("identified:" + control.type)) << "identified" << control.type << control.delegates;
+
     // the "simple" style simulates a proper style and contains bindings to/in delegates
     for (const ControlInfo &control : ControlInfos)
         QTest::newRow(qPrintable("simple:" + control.type)) << "simple" << control.type << control.delegates;
@@ -235,23 +242,38 @@ void tst_customization::creation()
     QQuickStyle::setStyle(testFile("styles/" + style));
 
     QString error;
-    QScopedPointer<QObject> control(createControl(type, &error));
+    QScopedPointer<QObject> control(createControl(type, "", &error));
     QVERIFY2(control, qPrintable(error));
 
     QByteArray templateType = "QQuick" + type.toUtf8();
     QVERIFY2(control->inherits(templateType), qPrintable(type + " does not inherit " + templateType + " (" + control->metaObject()->className() + ")"));
 
+    // <control>-<style>
     QString controlName = type.toLower() + "-" + style;
+    QCOMPARE(control->objectName(), controlName);
     QVERIFY2(qt_createdQObjects()->removeOne(controlName), qPrintable(controlName + " was not created as expected"));
 
     for (QString delegate : qAsConst(delegates)) {
-        if (!delegate.contains("-"))
-            delegate.append("-" + style);
+        QStringList properties = delegate.split(".", QString::SkipEmptyParts);
+
+        // <control>-<delegate>-<style>(-<override>)
+        delegate.append("-" + style);
         delegate.prepend(type.toLower() + "-");
 
         QVERIFY2(qt_createdQObjects()->removeOne(delegate), qPrintable(delegate + " was not created as expected"));
+
+        // verify that the delegate instance has the expected object name
+        // in case of grouped properties, we must query the properties step by step
+        QObject *instance = control.data();
+        while (!properties.isEmpty()) {
+            QString property = properties.takeFirst();
+            instance = instance->property(property.toUtf8()).value<QObject *>();
+            QVERIFY2(instance, qPrintable("property was null: " + property));
+        }
+        QCOMPARE(instance->objectName(), delegate);
     }
 
+    QEXPECT_FAIL("identified:ComboBox", "ComboBox::popup with an ID is created at construction time", Continue);
     QEXPECT_FAIL("override:Tumbler", "TODO", Abort);
 
     QVERIFY2(qt_createdQObjects()->isEmpty(), qPrintable("unexpectedly created: " + qt_createdQObjects->join(", ")));
@@ -260,6 +282,103 @@ void tst_customization::creation()
     QVERIFY2(qt_destroyedParentQObjects()->isEmpty(), qPrintable("delegates/children of: " + qt_destroyedParentQObjects->join(", ") + " were unexpectedly destroyed"));
 }
 
+void tst_customization::override_data()
+{
+    QTest::addColumn<QString>("style");
+    QTest::addColumn<QString>("type");
+    QTest::addColumn<QStringList>("delegates");
+    QTest::addColumn<QString>("nonDeferred");
+    QTest::addColumn<bool>("identify");
+
+    // NOTE: delegates with IDs prevent deferred execution
+
+    // default delegates with IDs, override with custom delegates with no IDs
+    for (const ControlInfo &control : ControlInfos)
+        QTest::newRow(qPrintable("identified:" + control.type)) << "identified" << control.type << control.delegates << "identified" << false;
+
+    // default delegates with no IDs, override with custom delegates with IDs
+    for (const ControlInfo &control : ControlInfos)
+        QTest::newRow(qPrintable("simple:" + control.type)) << "simple" << control.type << control.delegates << "" << true;
+
+    // default delegates with IDs, override with custom delegates with IDs
+    for (const ControlInfo &control : ControlInfos)
+        QTest::newRow(qPrintable("overidentified:" + control.type)) << "identified" << control.type << control.delegates << "identified" << true;
+
+    // test that the built-in styles don't have undesired IDs in their delegates
+    const QStringList styles = QStringList() << "Default"; // ### TODO: QQuickStyle::availableStyles();
+    for (const QString &style : styles) {
+        for (const ControlInfo &control : ControlInfos)
+            QTest::newRow(qPrintable(style + ":" + control.type)) << style << control.type << control.delegates << "" << false;
+    }
+}
+
+void tst_customization::override()
+{
+    QFETCH(QString, style);
+    QFETCH(QString, type);
+    QFETCH(QStringList, delegates);
+    QFETCH(QString, nonDeferred);
+    QFETCH(bool, identify);
+
+    QQuickStyle::setStyle(testFile("styles/" + style));
+
+    QString qml;
+    qml += QString("objectName: '%1-%2-override'; ").arg(type.toLower()).arg(style);
+    for (const QString &delegate : delegates) {
+        QString id = identify ? QString("id: %1;").arg(delegate) : QString();
+        qml += QString("%1: Item { %2 objectName: '%3-%1-%4-override' } ").arg(delegate).arg(id.replace(".", "")).arg(type.toLower()).arg(style);
+    }
+
+    QString error;
+    QScopedPointer<QObject> control(createControl(type, qml, &error));
+    QVERIFY2(control, qPrintable(error));
+
+    // <control>-<style>-override
+    QString controlName = type.toLower() + "-" + style + "-override";
+    QCOMPARE(control->objectName(), controlName);
+    QVERIFY2(qt_createdQObjects()->removeOne(controlName), qPrintable(controlName + " was not created as expected"));
+
+    for (QString delegate : qAsConst(delegates)) {
+        QStringList properties = delegate.split(".", QString::SkipEmptyParts);
+
+        // <control>-<delegate>-<style>(-override)
+        delegate.append("-" + style);
+        delegate.prepend(type.toLower() + "-");
+
+        if (!nonDeferred.isEmpty())
+            QVERIFY2(qt_createdQObjects()->removeOne(delegate), qPrintable(delegate + " was not created as expected"));
+
+        delegate.append("-override");
+        QVERIFY2(qt_createdQObjects()->removeOne(delegate), qPrintable(delegate + " was not created as expected"));
+
+        // verify that the delegate instance has the expected object name
+        // in case of grouped properties, we must query the properties step by step
+        QObject *instance = control.data();
+        while (!properties.isEmpty()) {
+            QString property = properties.takeFirst();
+            instance = instance->property(property.toUtf8()).value<QObject *>();
+            QVERIFY2(instance, qPrintable("property was null: " + property));
+        }
+        QCOMPARE(instance->objectName(), delegate);
+    }
+
+    QEXPECT_FAIL("identified:ComboBox", "ComboBox::popup with an ID is created at construction time", Continue);
+    QEXPECT_FAIL("overidentified:ComboBox", "ComboBox::popup with an ID is created at construction time", Continue);
+    QEXPECT_FAIL("simple:Tumbler", "TODO: remove internal ID", Abort);
+    QVERIFY2(qt_createdQObjects()->isEmpty(), qPrintable("unexpectedly created: " + qt_createdQObjects->join(", ")));
+
+    if (!nonDeferred.isEmpty()) {
+        for (QString delegate : qAsConst(delegates)) {
+            if (!delegate.contains("-"))
+                delegate.append("-" + nonDeferred);
+            delegate.prepend(type.toLower() + "-");
+            QVERIFY2(qt_destroyedQObjects()->removeOne(delegate), qPrintable(delegate + " was not destroyed as expected"));
+        }
+    }
+
+    QVERIFY2(qt_destroyedQObjects()->isEmpty(), qPrintable("unexpectedly destroyed: " + qt_destroyedQObjects->join(", ")));
+}
+
 void tst_customization::comboPopup()
 {
     QQuickStyle::setStyle(testFile("styles/simple"));