Ubuntu UI Toolkit

Merge lp:~zsombi/ubuntu-ui-toolkit/fix-style-lookup-crash into lp:ubuntu-ui-toolkit

  1. fix-style-lookup-crash
  2. Merge into trunk
Proposed by Zsombor Egri
Status: Merged
Approved by: Florian Boucault
Approved revision: 491
Merged at revision: 493
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/fix-style-lookup-crash
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 106 lines (+45/-3)
6 files modified
modules/Ubuntu/Components/plugin/itemstyleattached.cpp (+6/-1)
modules/Ubuntu/Components/plugin/ucstyle.cpp (+1/-1)
tests/unit/tst_theme_engine/StyleLookupCrash.qml (+26/-0)
tests/unit/tst_theme_engine/StyleLookupCrash.qmltheme (+1/-0)
tests/unit/tst_theme_engine/tst_theme_engine.pro (+3/-1)
tests/unit/tst_theme_engine/tst_theme_enginetest.cpp (+8/-0)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/fix-style-lookup-crash
Reviewer Review Type Date Requested Status
Zsombor Egri Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+163735@code.launchpad.net

Commit message

Fix for crash in theme engine when loading application specific theme file in an app having a Popover instantiated.

Description of the change

Fix for crash in theme engine when loading application specific theme file in an app having a Popover instantiated.

To post a comment you must log in.
Revision history for this message
Zsombor Egri (zsombi) wrote :

Not ready yet, posted for testing purpose.

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

PASSED: Continuous integration, rev:488
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/53/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/53
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/53

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

review: Approve (continuous-integration)
lp:~zsombi/ubuntu-ui-toolkit/fix-style-lookup-crash updated
489. By Zsombor Egri

test case extended to reproduce the crash on nexus 10

490. By Zsombor Egri

fix style cleanup

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

FAILED: Continuous integration, rev:489
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/54/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/54
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/54

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:490
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/55/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/55
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/55

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

review: Approve (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote :

Tested with reverted media-player app workarounds.

lp:~zsombi/ubuntu-ui-toolkit/fix-style-lookup-crash updated
491. By Zsombor Egri

link to the bug added

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

PASSED: Continuous integration, rev:491
http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-ci/56/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-amd64-ci/56
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-ui-toolkit-raring-armhf-ci/56

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

review: Approve (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote :

Ready for review. Tested with rolled back workaround on media-player application on Nexus 10.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'modules/Ubuntu/Components/plugin/itemstyleattached.cpp'
--- modules/Ubuntu/Components/plugin/itemstyleattached.cpp 2013-05-07 10:55:24 +0000
+++ modules/Ubuntu/Components/plugin/itemstyleattached.cpp 2013-05-15 06:49:24 +0000
@@ -358,8 +358,13 @@
358358
359 // delete style also if there is an owner set to it359 // delete style also if there is an owner set to it
360 if (!customStyle || style->owner()) {360 if (!customStyle || style->owner()) {
361 delete style;361 // reset style object before we delete it, themed animations may get changed
362 // during the style deletion which will cause invalid pointer operations
363 // in style bindings cleanup
364 // https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1175394
365 QObject *object = style;
362 style = 0;366 style = 0;
367 delete object;
363 }368 }
364}369}
365370
366371
=== modified file 'modules/Ubuntu/Components/plugin/ucstyle.cpp'
--- modules/Ubuntu/Components/plugin/ucstyle.cpp 2013-05-01 19:19:22 +0000
+++ modules/Ubuntu/Components/plugin/ucstyle.cpp 2013-05-15 06:49:24 +0000
@@ -46,7 +46,7 @@
46UCStyle::~UCStyle()46UCStyle::~UCStyle()
47{47{
48 // we shouldn't have bindings by this time, but just in case...48 // we shouldn't have bindings by this time, but just in case...
49 QHashIterator<QString, QQmlProperty> i(m_bindings);49 QMutableHashIterator<QString, QQmlProperty> i(m_bindings);
50 while (i.hasNext()) {50 while (i.hasNext()) {
51 i.next();51 i.next();
52 unbind(i.key());52 unbind(i.key());
5353
=== added file 'tests/unit/tst_theme_engine/StyleLookupCrash.qml'
--- tests/unit/tst_theme_engine/StyleLookupCrash.qml 1970-01-01 00:00:00 +0000
+++ tests/unit/tst_theme_engine/StyleLookupCrash.qml 2013-05-15 06:49:24 +0000
@@ -0,0 +1,26 @@
1/*
2 * Copyright 2012 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 */
16import QtQuick 2.0
17import Ubuntu.Components 0.1
18import Ubuntu.Components.Popups 0.1
19
20Item {
21 Popover {
22 }
23 Slider{
24 }
25 Component.onCompleted: Theme.loadTheme(Qt.resolvedUrl("StyleLookupCrash.qmltheme"))
26}
027
=== added file 'tests/unit/tst_theme_engine/StyleLookupCrash.qmltheme'
--- tests/unit/tst_theme_engine/StyleLookupCrash.qmltheme 1970-01-01 00:00:00 +0000
+++ tests/unit/tst_theme_engine/StyleLookupCrash.qmltheme 2013-05-15 06:49:24 +0000
@@ -0,0 +1,1 @@
1@import url(system:/Ambiance/qmltheme/default.qmltheme);
02
=== modified file 'tests/unit/tst_theme_engine/tst_theme_engine.pro'
--- tests/unit/tst_theme_engine/tst_theme_engine.pro 2013-04-30 22:38:24 +0000
+++ tests/unit/tst_theme_engine/tst_theme_engine.pro 2013-05-15 06:49:24 +0000
@@ -9,4 +9,6 @@
9 SelectorTest.qml \9 SelectorTest.qml \
10 InheritanceTest.qml \10 InheritanceTest.qml \
11 MemoryCleanup.qml \11 MemoryCleanup.qml \
12 CustomStyles.qml12 CustomStyles.qml \
13 StyleLookupCrash.qml \
14 StyleLookupCrash.qmltheme
1315
=== modified file 'tests/unit/tst_theme_engine/tst_theme_enginetest.cpp'
--- tests/unit/tst_theme_engine/tst_theme_enginetest.cpp 2013-05-07 10:55:24 +0000
+++ tests/unit/tst_theme_engine/tst_theme_enginetest.cpp 2013-05-15 06:49:24 +0000
@@ -67,6 +67,7 @@
67 void testCase_inheritance();67 void testCase_inheritance();
68 void testCase_MemoryCleanup();68 void testCase_MemoryCleanup();
69 void testCase_CustomTheme();69 void testCase_CustomTheme();
70 void testCase_styleLookupCrash();
70private:71private:
71 bool check_properties(QObject *style, const QString &properties, bool xfail);72 bool check_properties(QObject *style, const QString &properties, bool xfail);
72private:73private:
@@ -370,6 +371,13 @@
370 delete root;371 delete root;
371}372}
372373
374void tst_ThemeEngine::testCase_styleLookupCrash()
375{
376 QQuickItem *root = loadTest("StyleLookupCrash.qml");
377
378 delete root;
379}
380
373QTEST_MAIN(tst_ThemeEngine)381QTEST_MAIN(tst_ThemeEngine)
374382
375#include "tst_theme_enginetest.moc"383#include "tst_theme_enginetest.moc"

Subscribers

People subscribed via source and target branches

to status/vote changes: