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
Change-Id: Ied52d4c38914765ed5c75e234954f4baabaaa9af
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 7f29e89c26ae2babc358b1c4e6f965af6ec759f4)

3 files changed, 115 insertions, 4 deletions

diff --git a/tests/auto/qquickmaterialstyle/data/tst_material.qml b/tests/auto/qquickmaterialstyle/data/tst_material.qml
index 9f2456b8..1f2c8b25 100644
--- a/tests/auto/qquickmaterialstyle/data/tst_material.qml
+++ b/tests/auto/qquickmaterialstyle/data/tst_material.qml
@@ -55,6 +55,8 @@ import QtQuick.Templates 2.12 as T
 import QtQuick.Controls 2.12
 import QtQuick.Controls.Material 2.12
 
+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"