1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
|
From 73ad6e87bbeceea5830ab3a6b3dc66fa99e30f45 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 28 Oct 2019 13:41:11 +0100
Subject: [PATCH] QQuickItem::setParentItem: add child earlier
Calling (de)refWindow can trigger QQuickItem::windowChanged, which in turn
can call a user defined windowChanged handler. If that signal handler
were to call setParentItem, we would encounter an inconsistent state:
The item already has its parent set, but that parent would lack the item
in its children list (as we would only call refWindow at a later point).
Fixes: QTBUG-79573
Fixes: QTBUG-73439
Change-Id: I46adaa54a0521b5cd7f37810b3dd1a206e6a09c6
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>
---
src/quick/items/qquickitem.cpp | 21 +++++++++++++++++----
.../qquickitem/data/setParentInWindowChange.qml | 12 ++++++++++++
tests/auto/quick/qquickitem/tst_qquickitem.cpp | 8 ++++++++
3 files changed, 37 insertions(+), 4 deletions(-)
create mode 100644 tests/auto/quick/qquickitem/data/setParentInWindowChange.qml
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp
index 396012e1e67..26f02aeed7f 100644
--- a/src/quick/items/qquickitem.cpp
+++ b/src/quick/items/qquickitem.cpp
@@ -2748,22 +2748,35 @@ void QQuickItem::setParentItem(QQuickItem *parentItem)
}
QQuickWindow *parentWindow = parentItem ? QQuickItemPrivate::get(parentItem)->window : nullptr;
+ bool alreadyAddedChild = false;
if (d->window == parentWindow) {
// Avoid freeing and reallocating resources if the window stays the same.
d->parentItem = parentItem;
} else {
- if (d->window)
- d->derefWindow();
+ auto oldParentItem = d->parentItem;
d->parentItem = parentItem;
+ if (d->parentItem) {
+ QQuickItemPrivate::get(d->parentItem)->addChild(this);
+ alreadyAddedChild = true;
+ }
+ if (d->window) {
+ d->derefWindow();
+ // as we potentially changed d->parentWindow above
+ // the check in derefWindow could not work
+ // thus, we redo it here with the old parent
+ if (!oldParentItem) {
+ QQuickWindowPrivate::get(d->window)->parentlessItems.remove(this);
+ }
+ }
if (parentWindow)
d->refWindow(parentWindow);
}
d->dirty(QQuickItemPrivate::ParentChanged);
- if (d->parentItem)
+ if (d->parentItem && !alreadyAddedChild)
QQuickItemPrivate::get(d->parentItem)->addChild(this);
- else if (d->window)
+ else if (d->window && !alreadyAddedChild)
QQuickWindowPrivate::get(d->window)->parentlessItems.insert(this);
d->setEffectiveVisibleRecur(d->calcEffectiveVisible());
diff --git a/tests/auto/quick/qquickitem/data/setParentInWindowChange.qml b/tests/auto/quick/qquickitem/data/setParentInWindowChange.qml
new file mode 100644
index 00000000000..d68b7adb72a
--- /dev/null
+++ b/tests/auto/quick/qquickitem/data/setParentInWindowChange.qml
@@ -0,0 +1,12 @@
+import QtQuick 2.12
+
+Rectangle {
+ width: 800
+ height: 600
+ Item {
+ id: it
+ onWindowChanged: () => it.parent = newParent
+ }
+
+ Item { id: newParent }
+}
diff --git a/tests/auto/quick/qquickitem/tst_qquickitem.cpp b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
index 7e132f97b67..9ce9766c925 100644
--- a/tests/auto/quick/qquickitem/tst_qquickitem.cpp
+++ b/tests/auto/quick/qquickitem/tst_qquickitem.cpp
@@ -197,6 +197,8 @@ private slots:
void qtBug60123();
#endif
+ void setParentCalledInOnWindowChanged();
+
private:
enum PaintOrderOp {
@@ -2145,6 +2147,12 @@ void tst_qquickitem::qtBug60123()
activateWindowAndTestPress(&window);
}
#endif
+void tst_qquickitem::setParentCalledInOnWindowChanged()
+{
+ QQuickView view;
+ view.setSource(testFileUrl("setParentInWindowChange.qml"));
+ QVERIFY(ensureFocus(&view)); // should not crash
+}
QTEST_MAIN(tst_qquickitem)
--
2.16.3
|
