Material: fix binding loops when binding between attached properties

The paletteChanged signal was used as the change signal for a lot of
properties. The problem with this was this binding, for example:

Material.foreground: Material.toolTextColor

results in foreground being set, which emits paletteChanged.
toolTextColor has paletteChanged as its change signal, so that is
triggered and then the foreground binding is re-evaluated in the middle
of already being evaluated.

I haven't found a way to fix this for toolTextColor yet, so we
temporarily skip emission of toolTextColorChanged when foreground
changes. This means that some text will be the wrong color when
foreground is changed after startup.

For other properties, using more specific change handlers is enough
to solve any binding loops.

Task-number: QTBUG-85699
Pick-to: 5.15
Change-Id: Ied52d4c38914765ed5c75e234954f4baabaaa9af
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

4 files changed, 116 insertions, 4 deletions

diff --git a/tests/auto/qquickmaterialstyle/CMakeLists.txt b/tests/auto/qquickmaterialstyle/CMakeLists.txt
index cf43db51..2b213448 100644
--- a/tests/auto/qquickmaterialstyle/CMakeLists.txt
+++ b/tests/auto/qquickmaterialstyle/CMakeLists.txt
@@ -17,6 +17,7 @@ qt_add_test(tst_qquickmaterialstyle
         tst_qquickmaterialstyle.cpp
     PUBLIC_LIBRARIES
         Qt::Gui
+        Qt::Qml
     TESTDATA ${test_data}
 )
 
diff --git a/tests/auto/qquickmaterialstyle/data/tst_material.qml b/tests/auto/qquickmaterialstyle/data/tst_material.qml
index dfb0e770..c0a3891f 100644
--- a/tests/auto/qquickmaterialstyle/data/tst_material.qml
+++ b/tests/auto/qquickmaterialstyle/data/tst_material.qml
@@ -55,6 +55,8 @@ import QtQuick.Templates as T
 import QtQuick.Controls
 import QtQuick.Controls.Material
 
+import org.qtproject.Test 1.0
+
 TestCase {
     id: testCase
     width: 200
@@ -523,11 +525,11 @@ TestCase {
         compare(control.Material[prop], "#80808080")
 
         // unknown
-        ignoreWarning(Qt.resolvedUrl("tst_material.qml") + ":68:9: QML Button: unknown Material." + prop + " value: 123")
+        ignoreWarning(new RegExp("QML Button: unknown Material." + prop + " value: 123"))
         control.Material[prop] = 123
-        ignoreWarning(Qt.resolvedUrl("tst_material.qml") + ":68:9: QML Button: unknown Material." + prop + " value: foo")
+        ignoreWarning(new RegExp("QML Button: unknown Material." + prop + " value: foo"))
         control.Material[prop] = "foo"
-        ignoreWarning(Qt.resolvedUrl("tst_material.qml") + ":68:9: QML Button: unknown Material." + prop + " value: #1")
+        ignoreWarning(new RegExp("QML Button: unknown Material." + prop + " value: #1"))
         control.Material[prop] = "#1"
 
         control.destroy()
@@ -715,4 +717,75 @@ TestCase {
 
         control.destroy()
     }
+
+    // We can't declare components with JS syntax (when creating a data row),
+    // so we use introspection to get the list of all components we should test.
+    QtObject {
+        id: bindingLoopComponents
+
+        property Component row_foregroundToPrimaryTextColor: Item { Material.foreground: Material.primaryTextColor }
+        // Not all properties can be bound without binding loops. For example, it's not possible to bind
+        // foreground to primaryHighlightedTextColor, because primaryHighlightedTextColor() depends on
+        // m_explicitForeground, which is modified when the foreground is set.
+        // So, we use background instead.
+        property Component row_backgroundToPrimaryHighlightedTextColor: Item { Material.background: Material.primaryHighlightedTextColor }
+        property Component row_foregroundToSecondaryTextColor: Item { Material.foreground: Material.secondaryTextColor }
+        property Component row_foregroundToSecondaryTextColorWithTheme: Item {
+            Material.foreground: Material.theme === Material.Dark ? Material.secondaryTextColor : Material.Red
+        }
+        property Component row_foregroundToHintTextColor: Item { Material.foreground: Material.secondaryTextColor }
+        property Component row_foregroundToTextSelectionColor: Item { Material.foreground: Material.textSelectionColor }
+        property Component row_foregroundToDropShadowColor: Item { Material.foreground: Material.dropShadowColor }
+        property Component row_foregroundToDividerColor: Item { Material.foreground: Material.dividerColor }
+        property Component row_foregroundToIconColor: Item { Material.foreground: Material.iconColor }
+        property Component row_foregroundToIconDisabledColor: Item { Material.foreground: Material.iconDisabledColor }
+        property Component row_foregroundToButtonColor: Item { Material.foreground: Material.buttonColor }
+        property Component row_foregroundToButtonDisabledColor: Item { Material.foreground: Material.buttonDisabledColor }
+        property Component row_foregroundToHighlightedButtonColor: Item { Material.foreground: Material.highlightedButtonColor }
+        property Component row_foregroundToFrameColor: Item { Material.foreground: Material.frameColor }
+        property Component row_foregroundToRippleColor: Item { Material.foreground: Material.rippleColor }
+        property Component row_foregroundToHighlightedRippleColor: Item { Material.foreground: Material.highlightedRippleColor }
+        property Component row_foregroundToSwitchUncheckedTrackColor: Item { Material.foreground: Material.switchUncheckedTrackColor }
+        property Component row_foregroundToSwitchCheckedTrackColor: Item { Material.foreground: Material.switchCheckedTrackColor }
+        property Component row_foregroundToSwitchUncheckedHandleColor: Item { Material.foreground: Material.switchUncheckedHandleColor }
+        property Component row_foregroundToSwitchCheckedHandleColor: Item { Material.foreground: Material.switchCheckedHandleColor }
+        property Component row_foregroundToSwitchDisabledTrackColor: Item { Material.foreground: Material.switchDisabledTrackColor }
+        property Component row_foregroundToSwitchDisabledHandleColor: Item { Material.foreground: Material.switchDisabledHandleColor }
+        property Component row_foregroundToScrollBarColor: Item { Material.foreground: Material.scrollBarColor }
+        property Component row_foregroundToScrollBarHoveredColor: Item { Material.foreground: Material.scrollBarHoveredColor }
+        property Component row_foregroundToScrollBarPressedColor: Item { Material.foreground: Material.scrollBarPressedColor }
+        property Component row_foregroundToDialogColor: Item { Material.foreground: Material.dialogColor }
+        property Component row_foregroundToBackgroundDimColor: Item { Material.foreground: Material.backgroundDimColor }
+        property Component row_foregroundToListHighlightColor: Item { Material.foreground: Material.listHighlightColor }
+        property Component row_foregroundToTooltipColor: Item { Material.foreground: Material.tooltipColor }
+        property Component row_foregroundToToolBarColor: Item { Material.foreground: Material.toolBarColor }
+        property Component row_backgroundToToolTextColor: Item { Material.background: Material.toolTextColor }
+        property Component row_foregroundToSpinBoxDisabledIconColor: Item { Material.foreground: Material.spinBoxDisabledIconColor }
+        property Component row_foregroundToSliderDisableColor: Item { Material.foreground: Material.sliderDisableColor }
+    }
+
+    function test_propertyBindingLoop_data() {
+        let data = []
+        for (let propertyName in bindingLoopComponents) {
+            if (!propertyName.startsWith("row_") || propertyName.endsWith("Changed"))
+                continue
+
+            let row = {}
+            row.tag = propertyName.substr(4)
+            row.component = bindingLoopComponents[propertyName]
+            data.push(row)
+        }
+        return data
+    }
+
+    /*
+        Test that binding attached Material properties to other (private, non-settable)
+        Material properties does not result in a binding loop.
+    */
+    function test_propertyBindingLoop(data) {
+        let item = createTemporaryObject(data.component, testCase)
+        verify(item)
+        verify(!BindingLoopDetector.bindingLoopDetected, "Detected binding loop")
+        BindingLoopDetector.reset()
+    }
 }
diff --git a/tests/auto/qquickmaterialstyle/qquickmaterialstyle.pro b/tests/auto/qquickmaterialstyle/qquickmaterialstyle.pro
index dac2176a..99633379 100644
--- a/tests/auto/qquickmaterialstyle/qquickmaterialstyle.pro
+++ b/tests/auto/qquickmaterialstyle/qquickmaterialstyle.pro
@@ -1,6 +1,7 @@
 TEMPLATE = app
 TARGET = tst_qquickmaterialstyle
 CONFIG += qmltestcase
+QT += qml
 
 SOURCES += \
     $$PWD/tst_qquickmaterialstyle.cpp
diff --git a/tests/auto/qquickmaterialstyle/tst_qquickmaterialstyle.cpp b/tests/auto/qquickmaterialstyle/tst_qquickmaterialstyle.cpp
index cd08923a..6b6eae15 100644
--- a/tests/auto/qquickmaterialstyle/tst_qquickmaterialstyle.cpp
+++ b/tests/auto/qquickmaterialstyle/tst_qquickmaterialstyle.cpp
@@ -34,5 +34,42 @@
 **
 ****************************************************************************/
 
+#include <QtQml/qqmlengine.h>
+#include <QtQml/qqmlcontext.h>
 #include <QtQuickTest/quicktest.h>
-QUICK_TEST_MAIN(tst_qquickmaterialstyle)
+
+class Setup : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(bool bindingLoopDetected READ wasBindingLoopDetected FINAL)
+
+public:
+    Setup() {}
+
+    bool wasBindingLoopDetected() const { return mBindingLoopDetected; }
+
+public slots:
+    void reset() { mBindingLoopDetected = false; }
+
+    void qmlEngineAvailable(QQmlEngine *engine)
+    {
+        connect(engine, &QQmlEngine::warnings, this, &Setup::qmlWarnings);
+
+        qmlRegisterSingletonInstance("org.qtproject.Test", 1, 0, "BindingLoopDetector", this);
+    }
+
+    void qmlWarnings(const QList<QQmlError> &warnings)
+    {
+        for (const auto error : warnings) {
+            if (error.messageType() == QtWarningMsg && error.description().contains(QStringLiteral("Binding loop detected")))
+                mBindingLoopDetected = true;
+        }
+    }
+
+private:
+    bool mBindingLoopDetected = false;
+};
+
+QUICK_TEST_MAIN_WITH_SETUP(tst_qquickmaterialstyle, Setup)
+
+#include "tst_qquickmaterialstyle.moc"