From 9569779c37c2d98f8550a917b7274baffd4ae6c0 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Tue, 15 Jun 2021 12:45:03 +0200
Subject: Avoid unnecessary palette allocations

The palette provider allocates QQuickPalette instances lazily in the
palette() implementation. Since we only only test here whether a palette
is present, uses the non-mutating providesPalette instead.

As a drive-by, remove the paletteData() check from setCurrentColorGroup;
paletteData() asserts if it would return nullptr, so this check is wrong
and misleading.

Amends 3675f2b235f32e05cf6d754e81e0e8f8ddd59752.

Change-Id: I9701b3520998ec538ef560106a6c6078e7f1c4d8
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit 7b5c1063de0034d4b9e4345b9493aa3beba62a89)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/quick/items/qquickitem.cpp                     |  2 +-
 .../quick/qquickitem2/data/paletteAllocate.qml     | 35 ++++++++++++++++++
 tests/auto/quick/qquickitem2/tst_qquickitem.cpp    | 41 ++++++++++++++++++++++
 3 files changed, 77 insertions(+), 1 deletion(-)
 create mode 100644 tests/auto/quick/qquickitem2/data/paletteAllocate.qml

diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 5b8048c303..ff269363bb 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -8507,7 +8507,7 @@ bool QQuickItem::event(QEvent *ev)
         break;
     case QEvent::WindowActivate:
     case QEvent::WindowDeactivate:
-        if (d->palette())
+        if (d->providesPalette())
             d->setCurrentColorGroup();
         for (QQuickItem *item : d->childItems)
             QCoreApplication::sendEvent(item, ev);
diff --git a/tests/auto/quick/qquickitem2/data/paletteAllocate.qml b/tests/auto/quick/qquickitem2/data/paletteAllocate.qml
new file mode 100644
index 0000000000..6c959ddbcb
--- /dev/null
+++ b/tests/auto/quick/qquickitem2/data/paletteAllocate.qml
@@ -0,0 +1,35 @@
+import QtQuick
+
+Item {
+    id: root
+    width: 300
+    height: 300
+    visible: true
+
+    palette.active.base: "blue"
+    palette.inactive.base: "red"
+    palette.disabled.base: "gray"
+
+    Rectangle {
+        id: background
+        objectName: "background"
+
+        anchors.centerIn: parent
+        width: parent.width / 2
+        height: parent.height / 2
+
+        color: root.palette.base
+
+        Rectangle {
+            id: foreground
+            objectName: "foreground"
+
+            anchors.centerIn: parent
+            width: parent.width / 2
+            height: parent.height / 2
+
+            color: root.palette.base
+            border.color: "black"
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
index 843b916eb1..4d381d59a3 100644
--- a/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem2/tst_qquickitem.cpp
@@ -134,7 +134,9 @@ private slots:
     void isAncestorOf();
 
     void grab();
+
     void colorGroup();
+    void paletteAllocated();
 
 private:
     QQmlEngine engine;
@@ -3770,6 +3772,45 @@ void tst_QQuickItem::colorGroup()
     QCOMPARE(foreground->property("color").value<QColor>(), palette->inactive()->base());
 }
 
+/*!
+    Verify that items don't allocate their own QQuickPalette instance
+    unnecessarily.
+*/
+void tst_QQuickItem::paletteAllocated()
+{
+    QQuickView view;
+
+    view.setSource(testFileUrl("paletteAllocate.qml"));
+
+    QQuickItem *root = qobject_cast<QQuickItem *>(view.rootObject());
+    QQuickItem *background = root->findChild<QQuickItem *>("background");
+    QVERIFY(background);
+    QQuickItem *foreground = root->findChild<QQuickItem *>("foreground");
+    QVERIFY(foreground);
+
+    bool backgroundHasPalette = false;
+    bool foregroundHasPalette = false;
+    QObject::connect(background, &QQuickItem::paletteCreated, this, [&]{ backgroundHasPalette = true; });
+    QObject::connect(foreground, &QQuickItem::paletteCreated, this, [&]{ foregroundHasPalette = true; });
+
+    view.show();
+    view.requestActivate();
+    QVERIFY(QTest::qWaitForWindowActive(&view));
+
+    QVERIFY(!backgroundHasPalette);
+    QVERIFY(!foregroundHasPalette);
+
+    view.close();
+
+    QVERIFY(!backgroundHasPalette);
+    QVERIFY(!foregroundHasPalette);
+
+    auto quickpalette = foreground->property("palette").value<QQuickPalette*>();
+    QVERIFY(!backgroundHasPalette);
+    QVERIFY(quickpalette);
+    QVERIFY(foregroundHasPalette);
+}
+
 QTEST_MAIN(tst_QQuickItem)
 
 #include "tst_qquickitem.moc"
-- 
cgit v1.2.3