Make sure to listen for itemDestroyed when using QQuickDeferredPointer

When using QQuickDeferredPointer for deferred properties in Qt Quick
Controls, it is necessary to also implement the itemDestroyed virtual
method because if the item pointed gets deleted the
QQuickDeferredPointer reference will be a dangling pointer when cleanup
occurs in the destructor. This patch is following the pattern laid out
by QQuickControlsPrivate for handling its QQuickDeferredPointer members.

This bug is exposed when aborting a component incubation for types that
did not do this cleanup in the form of a segfault in the destructor of
the offending class.

Fixes: QTBUG-116828
Change-Id: I6a486de74ee38ccdcaff6a31195ee389f51fc93c
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
(cherry picked from commit 4e41739c5860befa6d4c5d8601661b1309799465)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit d733885962d797ad2880374e014e962e2a64ee7c)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

7 files changed, 247 insertions, 0 deletions

diff --git a/src/quicktemplates/qquickcombobox.cpp b/src/quicktemplates/qquickcombobox.cpp
index ca8eda60c4..6e08019000 100644
--- a/src/quicktemplates/qquickcombobox.cpp
+++ b/src/quicktemplates/qquickcombobox.cpp
@@ -211,6 +211,7 @@ public:
     void hidePopup(bool accept);
     void togglePopup(bool accept);
     void popupVisibleChanged();
+    void popupDestroyed();
 
     void itemClicked();
     void itemHovered();
@@ -255,6 +256,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     void setInputMethodHints(Qt::InputMethodHints hints, bool force = false);
 
@@ -363,6 +365,13 @@ void QQuickComboBoxPrivate::popupVisibleChanged()
     }
 }
 
+void QQuickComboBoxPrivate::popupDestroyed()
+{
+    Q_Q(QQuickComboBox);
+    popup = nullptr;
+    emit q->popupChanged();
+}
+
 void QQuickComboBoxPrivate::itemClicked()
 {
     Q_Q(QQuickComboBox);
@@ -839,6 +848,16 @@ void QQuickComboBoxPrivate::itemImplicitHeightChanged(QQuickItem *item)
         emit q->implicitIndicatorHeightChanged();
 }
 
+void QQuickComboBoxPrivate::itemDestroyed(QQuickItem *item)
+{
+    Q_Q(QQuickComboBox);
+    QQuickControlPrivate::itemDestroyed(item);
+    if (item == indicator) {
+        indicator = nullptr;
+        emit q->indicatorChanged();
+    }
+}
+
 qreal QQuickComboBoxPrivate::getContentWidth() const
 {
     if (componentComplete) {
@@ -1339,6 +1358,7 @@ void QQuickComboBox::setPopup(QQuickPopup *popup)
         d->cancelPopup();
 
     if (d->popup) {
+        QObjectPrivate::disconnect(d->popup.data(), &QQuickPopup::destroyed, d, &QQuickComboBoxPrivate::popupDestroyed);
         QObjectPrivate::disconnect(d->popup.data(), &QQuickPopup::visibleChanged, d, &QQuickComboBoxPrivate::popupVisibleChanged);
         QQuickComboBoxPrivate::hideOldPopup(d->popup);
     }
@@ -1346,6 +1366,9 @@ void QQuickComboBox::setPopup(QQuickPopup *popup)
         QQuickPopupPrivate::get(popup)->allowVerticalFlip = true;
         popup->setClosePolicy(QQuickPopup::CloseOnEscape | QQuickPopup::CloseOnPressOutsideParent);
         QObjectPrivate::connect(popup, &QQuickPopup::visibleChanged, d, &QQuickComboBoxPrivate::popupVisibleChanged);
+        // QQuickPopup does not derive from QQuickItemChangeListener, so we cannot use
+        // QQuickItemChangeListener::itemDestroyed so we have to use QObject::destroyed
+        QObjectPrivate::connect(popup, &QQuickPopup::destroyed, d, &QQuickComboBoxPrivate::popupDestroyed);
 
 #if QT_CONFIG(quick_itemview)
         if (QQuickItemView *itemView = popup->findChild<QQuickItemView *>())
diff --git a/src/quicktemplates/qquickgroupbox.cpp b/src/quicktemplates/qquickgroupbox.cpp
index 0fa2d1a6d8..72ce1b42cb 100644
--- a/src/quicktemplates/qquickgroupbox.cpp
+++ b/src/quicktemplates/qquickgroupbox.cpp
@@ -61,6 +61,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     QPalette defaultPalette() const override { return QQuickTheme::palette(QQuickTheme::GroupBox); }
 
@@ -104,6 +105,16 @@ void QQuickGroupBoxPrivate::itemImplicitHeightChanged(QQuickItem *item)
         emit q->implicitLabelHeightChanged();
 }
 
+void QQuickGroupBoxPrivate::itemDestroyed(QQuickItem *item)
+{
+    Q_Q(QQuickGroupBox);
+    QQuickFramePrivate::itemDestroyed(item);
+    if (item == label) {
+        label = nullptr;
+        emit q->labelChanged();
+    }
+}
+
 QQuickGroupBox::QQuickGroupBox(QQuickItem *parent)
     : QQuickFrame(*(new QQuickGroupBoxPrivate), parent)
 {
diff --git a/src/quicktemplates/qquickrangeslider.cpp b/src/quicktemplates/qquickrangeslider.cpp
index 51d511b3ba..6c4f8d8561 100644
--- a/src/quicktemplates/qquickrangeslider.cpp
+++ b/src/quicktemplates/qquickrangeslider.cpp
@@ -354,6 +354,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     bool live = true;
     qreal from = defaultFrom;
@@ -594,6 +595,15 @@ void QQuickRangeSliderPrivate::itemImplicitHeightChanged(QQuickItem *item)
         emit second->implicitHandleHeightChanged();
 }
 
+void QQuickRangeSliderPrivate::itemDestroyed(QQuickItem *item)
+{
+    QQuickControlPrivate::itemDestroyed(item);
+    if (item == first->handle())
+        first->setHandle(nullptr);
+    else if (item == second->handle())
+        second->setHandle(nullptr);
+}
+
 QQuickRangeSlider::QQuickRangeSlider(QQuickItem *parent)
     : QQuickControl(*(new QQuickRangeSliderPrivate), parent)
 {
diff --git a/src/quicktemplates/qquickslider.cpp b/src/quicktemplates/qquickslider.cpp
index 12041be1a1..2989735392 100644
--- a/src/quicktemplates/qquickslider.cpp
+++ b/src/quicktemplates/qquickslider.cpp
@@ -74,6 +74,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     qreal from = 0;
     qreal to = 1;
@@ -236,6 +237,16 @@ void QQuickSliderPrivate::itemImplicitHeightChanged(QQuickItem *item)
         emit q->implicitHandleHeightChanged();
 }
 
+void QQuickSliderPrivate::itemDestroyed(QQuickItem *item)
+{
+    Q_Q(QQuickSlider);
+    QQuickControlPrivate::itemDestroyed(item);
+    if (item == handle) {
+        handle = nullptr;
+        emit q->handleChanged();
+    }
+}
+
 QQuickSlider::QQuickSlider(QQuickItem *parent)
     : QQuickControl(*(new QQuickSliderPrivate), parent)
 {
diff --git a/src/quicktemplates/qquickspinbox.cpp b/src/quicktemplates/qquickspinbox.cpp
index 5d98514746..172f2c4237 100644
--- a/src/quicktemplates/qquickspinbox.cpp
+++ b/src/quicktemplates/qquickspinbox.cpp
@@ -110,6 +110,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     QPalette defaultPalette() const override { return QQuickTheme::palette(QQuickTheme::SpinBox); }
 
@@ -407,6 +408,15 @@ void QQuickSpinBoxPrivate::itemImplicitHeightChanged(QQuickItem *item)
         emit down->implicitIndicatorHeightChanged();
 }
 
+void QQuickSpinBoxPrivate::itemDestroyed(QQuickItem *item)
+{
+    QQuickControlPrivate::itemDestroyed(item);
+    if (item == up->indicator())
+        up->setIndicator(nullptr);
+    else if (item == down->indicator())
+        down->setIndicator(nullptr);
+}
+
 QQuickSpinBox::QQuickSpinBox(QQuickItem *parent)
     : QQuickControl(*(new QQuickSpinBoxPrivate), parent)
 {
diff --git a/tests/auto/quickcontrols/deferred/data/abortedIncubation.qml b/tests/auto/quickcontrols/deferred/data/abortedIncubation.qml
new file mode 100644
index 0000000000..519a779402
--- /dev/null
+++ b/tests/auto/quickcontrols/deferred/data/abortedIncubation.qml
@@ -0,0 +1,161 @@
+import QtQuick
+import QtQuick.Controls
+import QtQuick.Dialogs
+import QtQuick.Controls.Basic
+
+ApplicationWindow {
+    id: appWindow
+
+    // QQuickDeferredPointer<QQuickItem> background;
+    background: Rectangle {
+        id: backgroundRect
+        color: "black"
+    }
+
+    // internal property handle
+    ColorDialog {
+        id: colorDialog
+        options: ColorDialog.DontUseNativeDialog
+    }
+
+    // internal property upButton
+    // internal property textField
+    FolderDialog {
+        id: folderDialog
+        options: FolderDialog.DontUseNativeDialog
+    }
+
+    // indicator property of AbstractButton
+    Button {
+        id: basicButton
+        indicator: Rectangle {
+            id: basicButtonIndicator
+            color: "pink"
+        }
+    }
+
+    Dial {
+        id: dial
+        // QQuickDeferredPointer<QQuickItem> handle;
+        handle: Item {
+            Rectangle {
+                id: dialRect
+            }
+        }
+    }
+
+    GroupBox {
+        id: groupBox
+        // QQuickDeferredPointer<QQuickItem> label;
+        label: Label {
+            id: groupBoxLabel
+            text: "yo"
+        }
+    }
+
+    Label {
+        id: label
+        text: "yo2"
+        background: Rectangle {
+            id: labelBackground
+            color: "green"
+        }
+    }
+
+    Menu {
+        id: menu
+
+        MenuItem {
+            id: menuItem
+            text: "New..."
+            arrow: Rectangle {
+                id: menuItemArrow
+                color: "pink"
+            }
+        }
+    }
+
+    ScrollBar {
+        id: scrollbar
+    }
+
+    SpinBox {
+        id: spinBox
+
+        up.indicator: Rectangle {
+            id: spinBoxUpIndicator
+            color: "pink"
+        }
+        down.indicator: Rectangle {
+            id: spinBoxDownIndicator
+            color: "blue"
+        }
+    }
+
+    Control {
+        id: genericControl
+        background: Rectangle {
+            id: genericControlBackground
+            color: "red"
+        }
+        contentItem: Canvas {
+            id: genericControlContentItem
+        }
+    }
+
+    ComboBox {
+        id: comboBox
+        model: ["foo", "bar"]
+
+        popup: Popup {
+            id: comboBoxPopup
+            contentItem: ListView {
+                ScrollBar.vertical: ScrollBar {}
+            }
+        }
+        indicator: Item {
+            id: emptyPopupItem
+        }
+    }
+
+    Slider {
+        id: slider
+        // QQuickDeferredPointer<QQuickItem> handle;
+        handle: Item {
+            Rectangle {
+                id: rect
+            }
+        }
+    }
+
+    RangeSlider {
+        id: rangeSlider
+        first.handle: Item {
+            Rectangle {
+                id: rangeSliderRect1
+            }
+        }
+        second.handle: Item {
+            Rectangle {
+                id: rangeSliderRect2
+            }
+        }
+    }
+
+    TextArea {
+        id: textArea
+        background: Rectangle {
+            id: textAreaBackground
+            color: "pink"
+        }
+    }
+
+    TextField {
+        id: textField
+        background: Rectangle {
+            id: textFieldBackground
+            color: "pink"
+        }
+    }
+}
+
diff --git a/tests/auto/quickcontrols/deferred/tst_qquickdeferred.cpp b/tests/auto/quickcontrols/deferred/tst_qquickdeferred.cpp
index 4d3eadbea4..87b893febe 100644
--- a/tests/auto/quickcontrols/deferred/tst_qquickdeferred.cpp
+++ b/tests/auto/quickcontrols/deferred/tst_qquickdeferred.cpp
@@ -6,6 +6,7 @@
 #include <QQmlEngine>
 #include <QtQuick/qquickitem.h>
 #include <QtQuickTemplates2/private/qquickdeferredexecute_p_p.h>
+#include <QQmlIncubator>
 
 class DeferredPropertyTester : public QObject
 {
@@ -48,6 +49,7 @@ public:
     tst_qquickdeferred() : QQmlDataTest(QT_QMLTEST_DATADIR) {}
 private slots:
     void noSpuriousBinding();
+    void abortedIncubation();
 };
 
 
@@ -63,6 +65,25 @@ void tst_qquickdeferred::noSpuriousBinding() {
     root->setProperty("toggle", false);
 }
 
+// QTBUG-116828
+// This test checks the case where we cancel incubation of a componet with a deferred property
+// Components that have deferred properties should also provide an itemDestoryed method that
+// that resets the deferred property to null to prevent issues with dangling pointers.
+void tst_qquickdeferred::abortedIncubation()
+{
+    QQmlEngine engine;
+    QQmlIncubationController controller;
+    engine.setIncubationController(&controller);
+
+    {
+        QQmlIncubator incubator;
+        QQmlComponent componet(&engine, testFileUrl("abortedIncubation.qml"));
+        componet.create(incubator);
+        controller.incubateFor(1);
+        incubator.clear(); // abort incubation (and dont crash)
+    }
+}
+
 QTEST_MAIN(tst_qquickdeferred)
 
 #include "tst_qquickdeferred.moc"