Fix a crash on exit when using ToolTip in a specific item hierarchy

QQuickPopup connects its parent item's (MouseArea, in this case)
windowChanged() signal to QQuickPopupPrivate::setWindow(). It does this so
that:

1) QQuickOverlay can keep track of all of the popups that it manages.
2) Fonts, palettes and locales can be resolved.
3) If the QQuickPopup component has completed loading and the popup is visible
with a valid window, start the enter transition.

The problem arises only when using a very specific item hierarchy:

    Window {
        width: 640
        height: 480
        visible: true

        Item {
            anchors.fill: parent

            Item {
                anchors.fill: parent

                ColorOverlay {
                    source: parent
                    anchors.fill: parent
                }

                MouseArea {
                    anchors.fill: parent
                    hoverEnabled: true

                    ToolTip.visible: containsMouse
                    ToolTip.text: "ToolTip text"
                }
            }
        }
    }

When the window is closed and hence begins to be destroyed, the following events occur:

- QQuickWindow's destructor is called.
- The window's root item (QQuickRootItem) begins destruction.
- QQuickOverlay is destroyed.
- QQuickWindow's destructor is done, so the QWindow and then QObject
  destructors are called.
- The QQuickItem destructor for the outer Item is called.
- The child items of the outer Item have setParentItem(nullptr) called on them,
  one of which being the inner Item.
- The inner Item's setParentItem() function calls derefWindow(), which in turn
  calls derefWindow() on its children. One of those children is MouseArea.
- Since the MouseArea's window is deref'd, it emits the windowChanged() signal.
  MouseArea is the parentItem of the popup, so its windowChanged() signal
  causes QQuickPopupPrivate::setWindow() to be called.
- setWindow() tries to remove the popup from the old overlay, which has already
  been destroyed.

One approach I tried involved using QQuickOverlay::itemChange() to remove all
of the popups (via setWindow(nullptr), to ensure that their window pointer is
nullified), since that was called much earlier than the windowChanged() signal
is emitted. However, this still resulted in a heap-use-after-free in the same
place when running the newly added setOverlayParentToNull() test.

I also tried removing the popups in QQuickOverlay's destructor, but this
resulted in another heap-use-after-free (when accessing a popup in the
destructor) in tst_QQuickPopup::Universal::visible().

The remaining options were: store the window in a QPointer or return early in
overlay() if the wasDeleted member of the window was true. Using QPointer
seems like it would catch more issues than a single check in overlay(), so I
went with that.

Fixes: QTBUG-73243
Change-Id: Ieb5ce26dd76d45771d28297031ec43e27d958b5b
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

1 files changed, 42 insertions, 0 deletions

diff --git a/tests/auto/qquickpopup/tst_qquickpopup.cpp b/tests/auto/qquickpopup/tst_qquickpopup.cpp
index 636b2b21..4d238663 100644
--- a/tests/auto/qquickpopup/tst_qquickpopup.cpp
+++ b/tests/auto/qquickpopup/tst_qquickpopup.cpp
@@ -89,6 +89,8 @@ private slots:
     void disabledPalette();
     void disabledParentPalette();
     void countChanged();
+    void toolTipCrashOnClose();
+    void setOverlayParentToNull();
 };
 
 void tst_QQuickPopup::initTestCase()
@@ -1187,6 +1189,46 @@ void tst_QQuickPopup::countChanged()
     QVERIFY(window->setProperty("isModel1", false));
     QTRY_COMPARE(window->property("count").toInt(), 2);
 }
+
+// QTBUG-73243
+void tst_QQuickPopup::toolTipCrashOnClose()
+{
+    QQuickApplicationHelper helper(this, "toolTipCrashOnClose.qml");
+
+    QQuickWindow *window = helper.window;
+    window->show();
+    // TODO: Using ignoreMessage() fails in CI with macOS for release builds,
+    // so for now we let the warning through.
+//    QTest::ignoreMessage(QtWarningMsg, "ShaderEffectSource: 'recursive' must be set to true when rendering recursively.");
+    QVERIFY(QTest::qWaitForWindowActive(window));
+
+    QTest::mouseMove(window, QPoint(window->width() / 2, window->height() / 2));
+    QTRY_VERIFY(window->property("toolTipOpened").toBool());
+
+    QVERIFY(window->close());
+    // Shouldn't crash.
+}
+
+void tst_QQuickPopup::setOverlayParentToNull()
+{
+    QQuickApplicationHelper helper(this, "toolTipCrashOnClose.qml");
+
+    QQuickWindow *window = helper.window;
+    window->show();
+    // TODO: Using ignoreMessage() fails in CI with macOS for release builds,
+    // so for now we let the warning through.
+//    QTest::ignoreMessage(QtWarningMsg, "ShaderEffectSource: 'recursive' must be set to true when rendering recursively.");
+    QVERIFY(QTest::qWaitForWindowActive(window));
+
+    QVERIFY(QMetaObject::invokeMethod(window, "nullifyOverlayParent"));
+
+    QTest::mouseMove(window, QPoint(window->width() / 2, window->height() / 2));
+    QTRY_VERIFY(window->property("toolTipOpened").toBool());
+
+    QVERIFY(window->close());
+    // While nullifying the overlay parent doesn't make much sense, it shouldn't crash.
+}
+
 QTEST_QUICKCONTROLS_MAIN(tst_QQuickPopup)
 
 #include "tst_qquickpopup.moc"