Skip to content

Commit 2c0a4d6

Browse files
torarnvtorarnv
committed
Reparent QWindow children when reparenting QWidget
When a QWidget was reparented, we would take care to reparent its backing QWidgetWindow as well, into the nearest QWindow of the new QWidget parent. However we would only do this for the reparented widget itself, and not any of its child widgets. In the case where the widget has native children with their own QWindows, the widget itself may not (yet) be native, e.g. if it hasn't been shown yet, or if the user has set Qt::WA_DontCreateNativeAncestors. In these scenarios, we would be left with dangling QWindows, still hanging off their original QWindow parents, which would eventually lead to crashes. We now reparent both the QWindow of the reparented widget (as long as it's not about to be destroyed), and any QQWindow children we can reach. For each child hierarchy we can stop once we reach a QWindow, as the QWindow children of that window will follow along once we reparent the QWindow. QWindowContainer widgets don't usually have their own windowHandle(), but still manage a QWindow inside their parent widget hierarchy. These will not be reparented during QWidgetPrivate::setParent_sys(), but instead do their own reparenting later in QWidget::setParent via QWindowContainer::parentWasChanged(). The only exception to this is when the top level is about to be destroyed, in which case we let the window container know during QWidgetPrivate::setParent_sys(). Finally, although there should not be any leftover QWindows in the reparented widget once we have done the QWidgetWindow and QWindowContainer reparenting, we still do a pass over any remaining QWindows and reparent those too, since the original code included this as a possibility. We could make further improvements in this areas, such as moving the QWindowContainer::parentWasChanged() call, but the goal was to keep this change as minimal as possible so we can back-port it. Fixes: QTBUG-122747 Pick-to: 6.5 Change-Id: I4d1217fce4c3c48cf5f7bfbe9d561ab408ceebb2 Reviewed-by: Volker Hilsheimer <[email protected]> (cherry picked from commit c956eb8) Reviewed-by: Tor Arne Vestbø <[email protected]> (cherry picked from commit 8ee25c6)
1 parent 3999eab commit 2c0a4d6

File tree

3 files changed

+245
-39
lines changed

3 files changed

+245
-39
lines changed

‎src/widgets/kernel/qwidget.cpp

Lines changed: 91 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ using namespace QNativeInterface::Private;
8282
using namespace Qt::StringLiterals;
8383

8484
Q_LOGGING_CATEGORY(lcWidgetPainting, "qt.widgets.painting", QtWarningMsg);
85+
Q_LOGGING_CATEGORY(lcWidgetWindow, "qt.widgets.window", QtWarningMsg);
8586

8687
static inline bool qRectIntersects(const QRect &r1, const QRect &r2)
8788
{
@@ -10901,57 +10902,61 @@ void QWidgetPrivate::setParent_sys(QWidget *newparent, Qt::WindowFlags f)
1090110902

1090210903
setWinId(0);
1090310904

10904-
if (parent != newparent) {
10905-
QObjectPrivate::setParent_helper(newparent); //### why does this have to be done in the _sys function???
10906-
if (q->windowHandle()) {
10907-
q->windowHandle()->setFlags(f);
10908-
QWidget *parentWithWindow =
10909-
newparent ? (newparent->windowHandle() ? newparent : newparent->nativeParentWidget()) : nullptr;
10910-
if (parentWithWindow) {
10911-
QWidget *topLevel = parentWithWindow->window();
10912-
if ((f & Qt::Window) && topLevel && topLevel->windowHandle()) {
10913-
q->windowHandle()->setTransientParent(topLevel->windowHandle());
10914-
q->windowHandle()->setParent(nullptr);
10915-
} else {
10916-
q->windowHandle()->setTransientParent(nullptr);
10917-
q->windowHandle()->setParent(parentWithWindow->windowHandle());
10918-
}
10919-
} else {
10920-
q->windowHandle()->setTransientParent(nullptr);
10921-
q->windowHandle()->setParent(nullptr);
10922-
}
10923-
}
10924-
}
10925-
1092610905
if (!newparent) {
1092710906
f |= Qt::Window;
1092810907
if (parent)
1092910908
targetScreen = q->parentWidget()->window()->screen();
1093010909
}
1093110910

10911+
const bool destroyWindow = (
10912+
// Reparenting top level to child
10913+
(oldFlags & Qt::Window) && !(f & Qt::Window)
10914+
// And we can dispose of the window
10915+
&& wasCreated && !q->testAttribute(Qt::WA_NativeWindow)
10916+
);
10917+
10918+
if (parent != newparent) {
10919+
// Update object parent now, so we can resolve new parent window below
10920+
QObjectPrivate::setParent_helper(newparent);
10921+
10922+
if (q->windowHandle())
10923+
q->windowHandle()->setFlags(f);
10924+
10925+
// If the widget itself or any of its children have been created,
10926+
// we need to reparent their QWindows as well.
10927+
QWidget *parentWithWindow = closestParentWidgetWithWindowHandle();
10928+
// But if the widget is about to be destroyed we must skip the
10929+
// widget itself, and only reparent children.
10930+
if (destroyWindow)
10931+
reparentWidgetWindowChildren(parentWithWindow);
10932+
else
10933+
reparentWidgetWindows(parentWithWindow, f);
10934+
}
10935+
1093210936
bool explicitlyHidden = q->testAttribute(Qt::WA_WState_Hidden) && q->testAttribute(Qt::WA_WState_ExplicitShowHide);
1093310937

10934-
// Reparenting toplevel to child
10935-
if (wasCreated && !(f & Qt::Window) && (oldFlags & Qt::Window) && !q->testAttribute(Qt::WA_NativeWindow)) {
10938+
if (destroyWindow) {
1093610939
if (extra && extra->hasWindowContainer)
1093710940
QWindowContainer::toplevelAboutToBeDestroyed(q);
1093810941

10939-
QWindow *newParentWindow = newparent->windowHandle();
10940-
if (!newParentWindow)
10941-
if (QWidget *npw = newparent->nativeParentWidget())
10942-
newParentWindow = npw->windowHandle();
10943-
10944-
for (QObject *child : q->windowHandle()->children()) {
10945-
QWindow *childWindow = qobject_cast<QWindow *>(child);
10946-
if (!childWindow)
10947-
continue;
10948-
10949-
QWidgetWindow *childWW = qobject_cast<QWidgetWindow *>(childWindow);
10950-
QWidget *childWidget = childWW ? childWW->widget() : nullptr;
10951-
if (!childWW || (childWidget && childWidget->testAttribute(Qt::WA_NativeWindow)))
10952-
childWindow->setParent(newParentWindow);
10942+
// There shouldn't be any QWindow children left, but if there
10943+
// are, re-parent them now, before we destroy.
10944+
if (!q->windowHandle()->children().isEmpty()) {
10945+
QWidget *parentWithWindow = closestParentWidgetWithWindowHandle();
10946+
QWindow *newParentWindow = parentWithWindow ? parentWithWindow->windowHandle() : nullptr;
10947+
for (QObject *child : q->windowHandle()->children()) {
10948+
if (QWindow *childWindow = qobject_cast<QWindow *>(child)) {
10949+
qCWarning(lcWidgetWindow) << "Reparenting" << childWindow
10950+
<< "before destroying" << this;
10951+
childWindow->setParent(newParentWindow);
10952+
}
10953+
}
1095310954
}
10954-
q->destroy();
10955+
10956+
// We have reparented any child windows of the widget we are
10957+
// about to destroy to the new parent window handle, so we can
10958+
// safely destroy this widget without destroying sub windows.
10959+
q->destroy(true, false);
1095510960
}
1095610961

1095710962
adjustFlags(f, q);
@@ -10977,6 +10982,53 @@ void QWidgetPrivate::setParent_sys(QWidget *newparent, Qt::WindowFlags f)
1097710982
}
1097810983
}
1097910984

10985+
void QWidgetPrivate::reparentWidgetWindows(QWidget *parentWithWindow, Qt::WindowFlags windowFlags)
10986+
{
10987+
if (QWindow *window = windowHandle()) {
10988+
// Reparent this QWindow, and all QWindow children will follow
10989+
if (parentWithWindow) {
10990+
// The reparented widget has not updated its window flags yet,
10991+
// so we can't ask the widget directly. And we can't use the
10992+
// QWindow flags, as unlike QWidgets the QWindow flags always
10993+
// reflect Qt::Window, even for child windows. And we can't use
10994+
// QWindow::isTopLevel() either, as that depends on the parent,
10995+
// which we are in the process of updating. So we propagate the
10996+
// new flags of the reparented window from setParent_sys().
10997+
if (windowFlags & Qt::Window) {
10998+
// Top level windows can only have transient parents,
10999+
// and the transient parent must be another top level.
11000+
QWidget *topLevel = parentWithWindow->window();
11001+
auto *transientParent = topLevel->windowHandle();
11002+
Q_ASSERT(transientParent);
11003+
qCDebug(lcWidgetWindow) << "Setting" << window << "transient parent to" << transientParent;
11004+
window->setTransientParent(transientParent);
11005+
window->setParent(nullptr);
11006+
} else {
11007+
auto *parentWindow = parentWithWindow->windowHandle();
11008+
qCDebug(lcWidgetWindow) << "Reparenting" << window << "into" << parentWindow;
11009+
window->setTransientParent(nullptr);
11010+
window->setParent(parentWindow);
11011+
}
11012+
} else {
11013+
qCDebug(lcWidgetWindow) << "Making" << window << "top level window";
11014+
window->setTransientParent(nullptr);
11015+
window->setParent(nullptr);
11016+
}
11017+
} else {
11018+
reparentWidgetWindowChildren(parentWithWindow);
11019+
}
11020+
}
11021+
11022+
void QWidgetPrivate::reparentWidgetWindowChildren(QWidget *parentWithWindow)
11023+
{
11024+
for (auto *child : std::as_const(children)) {
11025+
if (auto *childWidget = qobject_cast<QWidget*>(child)) {
11026+
auto *childPrivate = QWidgetPrivate::get(childWidget);
11027+
childPrivate->reparentWidgetWindows(parentWithWindow);
11028+
}
11029+
}
11030+
}
11031+
1098011032
/*!
1098111033
Scrolls the widget including its children \a dx pixels to the
1098211034
right and \a dy downward. Both \a dx and \a dy may be negative.

‎src/widgets/kernel/qwidget_p.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,8 @@ class Q_WIDGETS_EXPORT QWidgetPrivate : public QObjectPrivate
371371
void showChildren(bool spontaneous);
372372
void hideChildren(bool spontaneous);
373373
void setParent_sys(QWidget *parent, Qt::WindowFlags);
374+
void reparentWidgetWindows(QWidget *parentWithWindow, Qt::WindowFlags windowFlags = {});
375+
void reparentWidgetWindowChildren(QWidget *parentWithWindow);
374376
void scroll_sys(int dx, int dy);
375377
void scroll_sys(int dx, int dy, const QRect &r);
376378
void deactivateWidgetCleanup();

‎tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,9 @@ private slots:
435435

436436
void setVisibleDuringDestruction();
437437

438+
void reparentWindowHandles_data();
439+
void reparentWindowHandles();
440+
438441
private:
439442
const QString m_platform;
440443
QSize m_testWidgetSize;
@@ -13433,5 +13436,154 @@ void tst_QWidget::setVisibleDuringDestruction()
1343313436
QTRY_COMPARE(signalSpy.count(), 4);
1343413437
}
1343513438

13439+
void tst_QWidget::reparentWindowHandles_data()
13440+
{
13441+
QTest::addColumn<int>("stage");
13442+
QTest::addRow("reparent child") << 1;
13443+
QTest::addRow("top level to child") << 2;
13444+
QTest::addRow("transient parent") << 3;
13445+
QTest::addRow("window container") << 4;
13446+
}
13447+
13448+
void tst_QWidget::reparentWindowHandles()
13449+
{
13450+
const bool nativeSiblingsOriginal = qApp->testAttribute(Qt::AA_DontCreateNativeWidgetSiblings);
13451+
qApp->setAttribute(Qt::AA_DontCreateNativeWidgetSiblings, true);
13452+
auto nativeSiblingGuard = qScopeGuard([&]{
13453+
qApp->setAttribute(Qt::AA_DontCreateNativeWidgetSiblings, nativeSiblingsOriginal);
13454+
});
13455+
13456+
QFETCH(int, stage);
13457+
13458+
switch (stage) {
13459+
case 1: {
13460+
// Reparent child widget
13461+
13462+
QWidget topLevel;
13463+
topLevel.setAttribute(Qt::WA_NativeWindow);
13464+
QVERIFY(topLevel.windowHandle());
13465+
QPointer<QWidget> child = new QWidget(&topLevel);
13466+
child->setAttribute(Qt::WA_DontCreateNativeAncestors);
13467+
child->setAttribute(Qt::WA_NativeWindow);
13468+
QVERIFY(child->windowHandle());
13469+
13470+
QWidget anotherTopLevel;
13471+
anotherTopLevel.setAttribute(Qt::WA_NativeWindow);
13472+
QVERIFY(anotherTopLevel.windowHandle());
13473+
QPointer<QWidget> intermediate = new QWidget(&anotherTopLevel);
13474+
QPointer<QWidget> leaf = new QWidget(intermediate);
13475+
leaf->setAttribute(Qt::WA_DontCreateNativeAncestors);
13476+
leaf->setAttribute(Qt::WA_NativeWindow);
13477+
QVERIFY(leaf->windowHandle());
13478+
QVERIFY(!intermediate->windowHandle());
13479+
13480+
// Reparenting a native widget should reparent the QWindow
13481+
child->setParent(leaf);
13482+
QCOMPARE(child->windowHandle()->parent(), leaf->windowHandle());
13483+
QCOMPARE(child->windowHandle()->transientParent(), nullptr);
13484+
QVERIFY(!intermediate->windowHandle());
13485+
13486+
// So should reparenting a non-native widget with native children
13487+
intermediate->setParent(&topLevel);
13488+
QVERIFY(!intermediate->windowHandle());
13489+
QCOMPARE(leaf->windowHandle()->parent(), topLevel.windowHandle());
13490+
QCOMPARE(leaf->windowHandle()->transientParent(), nullptr);
13491+
QCOMPARE(child->windowHandle()->parent(), leaf->windowHandle());
13492+
QCOMPARE(child->windowHandle()->transientParent(), nullptr);
13493+
}
13494+
break;
13495+
case 2: {
13496+
// Top level to child
13497+
13498+
QWidget topLevel;
13499+
topLevel.setAttribute(Qt::WA_NativeWindow);
13500+
13501+
// A regular top level loses its nativeness
13502+
QPointer<QWidget> regularToplevel = new QWidget;
13503+
regularToplevel->show();
13504+
QVERIFY(QTest::qWaitForWindowExposed(regularToplevel));
13505+
QVERIFY(regularToplevel->windowHandle());
13506+
regularToplevel->setParent(&topLevel);
13507+
QVERIFY(!regularToplevel->windowHandle());
13508+
13509+
// A regular top level loses its nativeness
13510+
QPointer<QWidget> regularToplevelWithNativeChildren = new QWidget;
13511+
QPointer<QWidget> nativeChild = new QWidget(regularToplevelWithNativeChildren);
13512+
nativeChild->setAttribute(Qt::WA_DontCreateNativeAncestors);
13513+
nativeChild->setAttribute(Qt::WA_NativeWindow);
13514+
QVERIFY(nativeChild->windowHandle());
13515+
regularToplevelWithNativeChildren->show();
13516+
QVERIFY(QTest::qWaitForWindowExposed(regularToplevelWithNativeChildren));
13517+
QVERIFY(regularToplevelWithNativeChildren->windowHandle());
13518+
regularToplevelWithNativeChildren->setParent(&topLevel);
13519+
QVERIFY(!regularToplevelWithNativeChildren->windowHandle());
13520+
// But the native child does not
13521+
QVERIFY(nativeChild->windowHandle());
13522+
QCOMPARE(nativeChild->windowHandle()->parent(), topLevel.windowHandle());
13523+
13524+
// An explicitly native top level keeps its nativeness, and the window handle moves
13525+
QPointer<QWidget> nativeTopLevel = new QWidget;
13526+
nativeTopLevel->setAttribute(Qt::WA_NativeWindow);
13527+
QVERIFY(nativeTopLevel->windowHandle());
13528+
nativeTopLevel->setParent(&topLevel);
13529+
QVERIFY(nativeTopLevel->windowHandle());
13530+
QCOMPARE(nativeTopLevel->windowHandle()->parent(), topLevel.windowHandle());
13531+
}
13532+
break;
13533+
case 3: {
13534+
// Transient parent
13535+
13536+
QWidget topLevel;
13537+
topLevel.setAttribute(Qt::WA_NativeWindow);
13538+
QVERIFY(topLevel.windowHandle());
13539+
QPointer<QWidget> child = new QWidget(&topLevel);
13540+
child->setAttribute(Qt::WA_NativeWindow);
13541+
QVERIFY(child->windowHandle());
13542+
13543+
QWidget anotherTopLevel;
13544+
anotherTopLevel.setAttribute(Qt::WA_NativeWindow);
13545+
QVERIFY(anotherTopLevel.windowHandle());
13546+
13547+
// Make transient child of top level
13548+
anotherTopLevel.setParent(&topLevel, Qt::Window);
13549+
QCOMPARE(anotherTopLevel.windowHandle()->parent(), nullptr);
13550+
QCOMPARE(anotherTopLevel.windowHandle()->transientParent(), topLevel.windowHandle());
13551+
13552+
// Make transient child of child
13553+
anotherTopLevel.setParent(child, Qt::Window);
13554+
QCOMPARE(anotherTopLevel.windowHandle()->parent(), nullptr);
13555+
QCOMPARE(anotherTopLevel.windowHandle()->transientParent(), topLevel.windowHandle());
13556+
}
13557+
break;
13558+
case 4: {
13559+
// Window container
13560+
13561+
QWidget topLevel;
13562+
topLevel.setAttribute(Qt::WA_NativeWindow);
13563+
QVERIFY(topLevel.windowHandle());
13564+
13565+
QPointer<QWidget> child = new QWidget(&topLevel);
13566+
QVERIFY(!child->windowHandle());
13567+
13568+
QWindow *window = new QWindow;
13569+
QWidget *container = QWidget::createWindowContainer(window);
13570+
container->setParent(child);
13571+
topLevel.show();
13572+
QVERIFY(QTest::qWaitForWindowExposed(&topLevel));
13573+
QCOMPARE(window->parent(), topLevel.windowHandle());
13574+
13575+
QWidget anotherTopLevel;
13576+
anotherTopLevel.setAttribute(Qt::WA_NativeWindow);
13577+
QVERIFY(anotherTopLevel.windowHandle());
13578+
13579+
child->setParent(&anotherTopLevel);
13580+
QCOMPARE(window->parent(), anotherTopLevel.windowHandle());
13581+
}
13582+
break;
13583+
default:
13584+
Q_UNREACHABLE();
13585+
}
13586+
}
13587+
1343613588
QTEST_MAIN(tst_QWidget)
1343713589
#include "tst_qwidget.moc"

0 commit comments

Comments
 (0)