Ubuntu UI Toolkit

Merge lp:~zsombi/ubuntu-ui-toolkit/layouts-fix1280359 into lp:ubuntu-ui-toolkit

  1. layouts-fix1280359
  2. Merge into trunk
Proposed by Zsombor Egri
Status: Merged
Approved by: Florian Boucault
Approved revision: 939
Merged at revision: 943
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/layouts-fix1280359
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 345 lines (+260/-1)
8 files modified
modules/Ubuntu/Layouts/plugin/ullayouts.cpp (+16/-0)
modules/Ubuntu/Layouts/plugin/ullayouts.h (+2/-0)
modules/Ubuntu/Layouts/plugin/ullayouts_p.h (+1/-0)
tests/unit_x11/tst_layouts/DialerCrash.qml (+55/-0)
tests/unit_x11/tst_layouts/ExcludedItemDeleted.qml (+67/-0)
tests/unit_x11/tst_layouts/ItemInstanceAsProperty.qml (+63/-0)
tests/unit_x11/tst_layouts/tst_layouts.cpp (+52/-0)
tests/unit_x11/tst_layouts/tst_layouts.pro (+4/-1)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/layouts-fix1280359
Reviewer Review Type Date Requested Status
Florian Boucault (community) Approve
PS Jenkins bot continuous-integration Approve
Nekhelesh Ramananthan (community) testing Approve
Review via email: mp+206930@code.launchpad.net

Commit message

Item instances set as property values were excluded from layouting, causing segfaults when re-layouting was performed or when the component got deleted due to or during re-layouting.

Description of the change

Item instances set as property values were excluded from layouting, causing segfaults when re-layouting was performed or when the component got deleted due to or during re-layouting.

To post a comment you must log in.
Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

I confirm that with this re-layouting works as expected without any crashes for the clock app.

review: Approve (testing)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:939
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/1734/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/3283
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2950
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-amd64-ci/682
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/682
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-trusty-armhf-ci/682/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2889
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3285
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3285/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2952
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2952/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/5354
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4039

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-ui-toolkit-ci/1734/rebuild

review: Approve (continuous-integration)
Revision history for this message
Florian Boucault (fboucault) wrote :

looks great to me!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'modules/Ubuntu/Layouts/plugin/ullayouts.cpp'
--- modules/Ubuntu/Layouts/plugin/ullayouts.cpp 2013-06-26 11:06:39 +0000
+++ modules/Ubuntu/Layouts/plugin/ullayouts.cpp 2014-02-18 13:41:01 +0000
@@ -200,6 +200,11 @@
200 .addChange(new PropertyChange(item, "enabled", activate));200 .addChange(new PropertyChange(item, "enabled", activate));
201}201}
202202
203// remove the deleted item from the excluded ones
204void ULLayoutsPrivate::_q_removeExcludedItem(QObject *excludedItem)
205{
206 excludedFromLayout.removeAll(static_cast<QQuickItem*>(excludedItem));
207}
203208
204/*209/*
205 * Validates the declared conditional layouts by checking whether they have name210 * Validates the declared conditional layouts by checking whether they have name
@@ -256,6 +261,12 @@
256 // check if the item's parent is included in the layout261 // check if the item's parent is included in the layout
257 QQuickItem *pl = item->parentItem();262 QQuickItem *pl = item->parentItem();
258 marker = 0;263 marker = 0;
264 if (!pl && item->parent()) {
265 // this may be an item instance assigned to a property
266 // like "property var anItem: Item {}"
267 // in which case we must get the parent object of it, not the parent item
268 pl = qobject_cast<QQuickItem*>(item->parent());
269 }
259 while (pl) {270 while (pl) {
260 marker = qobject_cast<ULLayoutsAttached*>(271 marker = qobject_cast<ULLayoutsAttached*>(
261 qmlAttachedPropertiesObject<ULLayouts>(pl, false));272 qmlAttachedPropertiesObject<ULLayouts>(pl, false));
@@ -267,6 +278,9 @@
267 if (!marker || (marker && marker->item().isEmpty())) {278 if (!marker || (marker && marker->item().isEmpty())) {
268 // remember theese so we hide them once we switch away from default layout279 // remember theese so we hide them once we switch away from default layout
269 excludedFromLayout << item;280 excludedFromLayout << item;
281 // and make sure we remove the item from excluded ones in case the item is destroyed
282 QObject::connect(item, SIGNAL(destroyed(QObject*)),
283 q, SLOT(_q_removeExcludedItem(QObject*)));
270 }284 }
271 }285 }
272 }286 }
@@ -632,3 +646,5 @@
632 &ULLayoutsPrivate::at_layout,646 &ULLayoutsPrivate::at_layout,
633 &ULLayoutsPrivate::clear_layouts);647 &ULLayoutsPrivate::clear_layouts);
634}648}
649
650#include "moc_ullayouts.cpp"
635651
=== modified file 'modules/Ubuntu/Layouts/plugin/ullayouts.h'
--- modules/Ubuntu/Layouts/plugin/ullayouts.h 2013-06-24 13:33:31 +0000
+++ modules/Ubuntu/Layouts/plugin/ullayouts.h 2014-02-18 13:41:01 +0000
@@ -77,6 +77,8 @@
77 friend class ULConditionalLayout;77 friend class ULConditionalLayout;
78 Q_DECLARE_PRIVATE(ULLayouts)78 Q_DECLARE_PRIVATE(ULLayouts)
79 QScopedPointer<ULLayoutsPrivate> d_ptr;79 QScopedPointer<ULLayoutsPrivate> d_ptr;
80
81 Q_PRIVATE_SLOT(d_func(), void _q_removeExcludedItem(QObject*))
80};82};
8183
82QML_DECLARE_TYPE(ULLayouts)84QML_DECLARE_TYPE(ULLayouts)
8385
=== modified file 'modules/Ubuntu/Layouts/plugin/ullayouts_p.h'
--- modules/Ubuntu/Layouts/plugin/ullayouts_p.h 2013-06-25 13:42:22 +0000
+++ modules/Ubuntu/Layouts/plugin/ullayouts_p.h 2014-02-18 13:41:01 +0000
@@ -33,6 +33,7 @@
3333
34 ULLayoutsPrivate(ULLayouts *qq);34 ULLayoutsPrivate(ULLayouts *qq);
3535
36 void _q_removeExcludedItem(QObject *excludedItem);
36 void validateConditionalLayouts();37 void validateConditionalLayouts();
37 void getLaidOutItems();38 void getLaidOutItems();
38 void updateLayout();39 void updateLayout();
3940
=== added file 'tests/unit_x11/tst_layouts/DialerCrash.qml'
--- tests/unit_x11/tst_layouts/DialerCrash.qml 1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_layouts/DialerCrash.qml 2014-02-18 13:41:01 +0000
@@ -0,0 +1,55 @@
1/*
2 * Copyright 2014 Canonical Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU Lesser General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import Ubuntu.Components 0.1
19import Ubuntu.Components.Pickers 0.1
20import Ubuntu.Layouts 0.1
21
22MainView {
23 width: units.gu(40)
24 height: units.gu(71)
25 backgroundColor: "#A55263"
26
27 property bool wide: true
28
29 Layouts {
30 objectName: "layoutManager"
31 layouts: [
32 ConditionalLayout {
33 name: "wideAspect"
34 when: wide
35
36 ItemLayout {
37 item: "object1"
38 }
39 },
40
41 ConditionalLayout {
42 name: "regularAspect"
43 when: !wide
44
45 ItemLayout {
46 item: "object2"
47 }
48 }
49 ]
50
51 Dialer {
52 Layouts.item: "object1"
53 }
54 }
55}
056
=== added file 'tests/unit_x11/tst_layouts/ExcludedItemDeleted.qml'
--- tests/unit_x11/tst_layouts/ExcludedItemDeleted.qml 1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_layouts/ExcludedItemDeleted.qml 2014-02-18 13:41:01 +0000
@@ -0,0 +1,67 @@
1/*
2 * Copyright 2014 Canonical Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU Lesser General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import Ubuntu.Components 0.1
19import Ubuntu.Layouts 0.1
20
21Item {
22 property bool wide: true
23 width: units.gu(40)
24 height: units.gu(71)
25
26 function changeLayout() {
27 loader2.sourceComponent = undefined; wide = false
28 }
29
30 Layouts {
31 objectName: "layoutManager"
32 layouts: [
33 ConditionalLayout {
34 name: "wideAspect"
35 when: wide
36
37 ItemLayout {
38 item: "object1"
39 }
40 },
41 ConditionalLayout {
42 name: "regularAspect"
43 when: !wide
44
45 ItemLayout {
46 item: "object1"
47 anchors.left: parent.left
48 }
49 }
50 ]
51
52 Column {
53 Loader {
54 Layouts.item: "object1"
55 id: loader
56 sourceComponent: ShaderEffectSource {
57 sourceItem: Item {
58 }
59 }
60 }
61 Loader {
62 id: loader2
63 sourceComponent: Item{}
64 }
65 }
66 }
67}
068
=== added file 'tests/unit_x11/tst_layouts/ItemInstanceAsProperty.qml'
--- tests/unit_x11/tst_layouts/ItemInstanceAsProperty.qml 1970-01-01 00:00:00 +0000
+++ tests/unit_x11/tst_layouts/ItemInstanceAsProperty.qml 2014-02-18 13:41:01 +0000
@@ -0,0 +1,63 @@
1/*
2 * Copyright 2014 Canonical Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU Lesser General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU Lesser General Public License for more details.
12 *
13 * You should have received a copy of the GNU Lesser General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import Ubuntu.Components 0.1
19import Ubuntu.Layouts 0.1
20
21Item {
22 property bool wide: true
23 width: units.gu(40)
24 height: units.gu(71)
25
26 function changeLayout() {
27 loader.sourceComponent = undefined; wide = false
28 }
29
30 Layouts {
31 objectName: "layoutManager"
32 layouts: [
33 ConditionalLayout {
34 name: "wideAspect"
35 when: wide
36
37 ItemLayout {
38 item: "object1"
39 }
40 },
41 ConditionalLayout {
42 name: "regularAspect"
43 when: !wide
44
45 ItemLayout {
46 item: "object2"
47 anchors.left: parent.left
48 }
49 }
50 ]
51
52 Item {
53 Layouts.item: "object1"
54 Loader {
55 id: loader
56 sourceComponent: ShaderEffectSource {
57 sourceItem: Item {
58 }
59 }
60 }
61 }
62 }
63}
064
=== modified file 'tests/unit_x11/tst_layouts/tst_layouts.cpp'
--- tests/unit_x11/tst_layouts/tst_layouts.cpp 2014-01-27 07:54:14 +0000
+++ tests/unit_x11/tst_layouts/tst_layouts.cpp 2014-02-18 13:41:01 +0000
@@ -897,6 +897,58 @@
897 QCOMPARE(anchors->margins(), 20.0);897 QCOMPARE(anchors->margins(), 20.0);
898 }898 }
899899
900 // the following tests guard bug #1280359
901 void testCase_ItemInstanceAsProperty()
902 {
903 QScopedPointer<QQuickView> view(loadTest("ItemInstanceAsProperty.qml"));
904 QVERIFY(view);
905 QQuickItem *root = view->rootObject();
906 QVERIFY(root);
907
908 QQuickItem *layout = testItem(root, "layoutManager");
909 QVERIFY(layout);
910 QSignalSpy layoutChangeSpy(layout, SIGNAL(currentLayoutChanged()));
911
912 // invoke layout change
913 view->rootObject()->metaObject()->invokeMethod(view->rootObject(), "changeLayout");
914 layoutChangeSpy.wait();
915 QCOMPARE(layoutChangeSpy.count(), 1);
916 }
917
918 void testCase_DialerCrash()
919 {
920 QScopedPointer<QQuickView> view(loadTest("DialerCrash.qml"));
921 QVERIFY(view);
922 QQuickItem *root = view->rootObject();
923 QVERIFY(root);
924
925 QQuickItem *layout = testItem(root, "layoutManager");
926 QVERIFY(layout);
927 QSignalSpy layoutChangeSpy(layout, SIGNAL(currentLayoutChanged()));
928
929 // invoke layout change
930 view->rootObject()->setProperty("wide", false);
931 layoutChangeSpy.wait();
932 QCOMPARE(layoutChangeSpy.count(), 1);
933 }
934
935 void testCase_ExcludedItemDeleted()
936 {
937 QScopedPointer<QQuickView> view(loadTest("ExcludedItemDeleted.qml"));
938 QVERIFY(view);
939 QQuickItem *root = view->rootObject();
940 QVERIFY(root);
941
942 QQuickItem *layout = testItem(root, "layoutManager");
943 QVERIFY(layout);
944 QSignalSpy layoutChangeSpy(layout, SIGNAL(currentLayoutChanged()));
945
946 // invoke layout change
947 view->rootObject()->metaObject()->invokeMethod(view->rootObject(), "changeLayout");
948 layoutChangeSpy.wait();
949 QCOMPARE(layoutChangeSpy.count(), 1);
950 }
951
900};952};
901953
902QTEST_MAIN(tst_Layouts)954QTEST_MAIN(tst_Layouts)
903955
=== modified file 'tests/unit_x11/tst_layouts/tst_layouts.pro'
--- tests/unit_x11/tst_layouts/tst_layouts.pro 2013-12-02 13:48:53 +0000
+++ tests/unit_x11/tst_layouts/tst_layouts.pro 2014-02-18 13:41:01 +0000
@@ -34,4 +34,7 @@
34 AnchorRight.qml \34 AnchorRight.qml \
35 AnchorTop.qml \35 AnchorTop.qml \
36 AnchorBottom.qml \36 AnchorBottom.qml \
37 AnchorAll.qml37 AnchorAll.qml \
38 ItemInstanceAsProperty.qml \
39 DialerCrash.qml \
40 ExcludedItemDeleted.qml

Subscribers

People subscribed via source and target branches

to status/vote changes: