Merge lp:~mzanetti/unity8/parent-that-launcheritem into lp:unity8

Proposed by Michael Zanetti
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 2217
Merged at revision: 2267
Proposed branch: lp:~mzanetti/unity8/parent-that-launcheritem
Merge into: lp:unity8
Prerequisite: lp:~lukas-kde/unity8/dynamicLauncherIcons
Diff against target: 38 lines (+4/-2)
3 files modified
plugins/Unity/Launcher/launcheritem.cpp (+1/-0)
plugins/Unity/Launcher/launcheritem.h (+1/-1)
plugins/Unity/Launcher/launchermodel.cpp (+2/-1)
To merge this branch: bzr merge lp:~mzanetti/unity8/parent-that-launcheritem
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Needs Fixing
Albert Astals Cid (community) Approve
Review via email: mp+288636@code.launchpad.net

This proposal supersedes a proposal from 2016-03-08.

Commit message

Properly parent launcher items in all cases

It is required to keep object ownership within C++ or QML would delete items in some circumstances which leads to crashes.

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.

no

 * Did you perform an exploratory manual test run of your code change and any related functionality?

no.. there's no change in behaviour. I believe this fixes a crash

https://errors.ubuntu.com/problem/dd7cdb19e961e18e58939b8af5c9324afbafc573

however, still need to figure how to reproduce this.

 * Did you make sure that your branch does not contain spurious tags?

yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

n/a

 * If you changed the UI, has there been a design review?

n/a

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

Should we also remove the = 0 from the LauncherItem constructor for the parent argument then?

review: Needs Information
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:2215
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/623/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/353
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/353
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/353
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/825
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/841
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/841
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/839/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/839/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/839/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/839/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/839/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/839
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/839/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/623/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Not really but the code is quite self explanatory

 * Did CI run pass?
Yes

 * Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote : Posted in a previous version of this proposal

FAILED: Continuous integration, rev:2216
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/632/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/358
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/358
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/358
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/836
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/852
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/852
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/850/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/850/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/850/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/850/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/850/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/850
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/850/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/632/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) :
review: Approve
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2217
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/654/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/372
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/372
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/372
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/860
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/876
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/876
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/874/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/874
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/874/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/654/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/Unity/Launcher/launcheritem.cpp'
--- plugins/Unity/Launcher/launcheritem.cpp 2015-09-14 09:11:08 +0000
+++ plugins/Unity/Launcher/launcheritem.cpp 2016-03-10 13:07:40 +0000
@@ -37,6 +37,7 @@
37 m_alerting(false),37 m_alerting(false),
38 m_quickList(new QuickListModel(this))38 m_quickList(new QuickListModel(this))
39{39{
40 Q_ASSERT(parent != nullptr);
40 QuickListEntry nameAction;41 QuickListEntry nameAction;
41 nameAction.setActionId(QStringLiteral("launch_item"));42 nameAction.setActionId(QStringLiteral("launch_item"));
42 nameAction.setText(m_name);43 nameAction.setText(m_name);
4344
=== modified file 'plugins/Unity/Launcher/launcheritem.h'
--- plugins/Unity/Launcher/launcheritem.h 2016-03-10 13:07:40 +0000
+++ plugins/Unity/Launcher/launcheritem.h 2016-03-10 13:07:40 +0000
@@ -32,7 +32,7 @@
32{32{
33 Q_OBJECT33 Q_OBJECT
34public:34public:
35 LauncherItem(const QString &appId, const QString &name, const QString &icon, QObject *parent = nullptr);35 LauncherItem(const QString &appId, const QString &name, const QString &icon, QObject *parent);
3636
37 QString appId() const override;37 QString appId() const override;
38 QString name() const override;38 QString name() const override;
3939
=== modified file 'plugins/Unity/Launcher/launchermodel.cpp'
--- plugins/Unity/Launcher/launchermodel.cpp 2016-03-10 13:07:40 +0000
+++ plugins/Unity/Launcher/launchermodel.cpp 2016-03-10 13:07:40 +0000
@@ -389,7 +389,8 @@
389 if (countVisible && desktopFile.isValid()) {389 if (countVisible && desktopFile.isValid()) {
390 LauncherItem *item = new LauncherItem(appId,390 LauncherItem *item = new LauncherItem(appId,
391 desktopFile.displayName(),391 desktopFile.displayName(),
392 desktopFile.icon());392 desktopFile.icon(),
393 this);
393 item->setCountVisible(true);394 item->setCountVisible(true);
394 beginInsertRows(QModelIndex(), m_list.count(), m_list.count());395 beginInsertRows(QModelIndex(), m_list.count(), m_list.count());
395 m_list.append(item);396 m_list.append(item);

Subscribers

People subscribed via source and target branches