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

Proposed by Michael Zanetti
Status: Superseded
Proposed branch: lp:~mzanetti/unity8/parent-that-launcheritem
Merge into: lp:unity8
Diff against target: 102 lines (+19/-7)
5 files modified
plugins/Unity/Launcher/launcheritem.cpp (+1/-0)
plugins/Unity/Launcher/launcheritem.h (+1/-1)
plugins/Unity/Launcher/launchermodel.cpp (+15/-5)
plugins/Unity/Launcher/launchermodel.h (+1/-1)
qml/Launcher/LauncherDelegate.qml (+1/-0)
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+288412@code.launchpad.net

This proposal has been superseded by a proposal from 2016-03-10.

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 :

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 :

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)
2216. By Michael Zanetti

prevent the same mistake from happening again

Revision history for this message
Albert Astals Cid (aacid) wrote :

 * 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 :

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)
2217. By Michael Zanetti

merge ~lukas-kde/unity8/dynamicLauncherIcons as prereq

Unmerged revisions

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:25 +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 2015-07-23 14:13:57 +0000
+++ plugins/Unity/Launcher/launcheritem.h 2016-03-10 13:07:25 +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 = 0);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 2015-09-14 09:11:08 +0000
+++ plugins/Unity/Launcher/launchermodel.cpp 2016-03-10 13:07:25 +0000
@@ -103,7 +103,7 @@
103 LauncherItem *item = m_list.at(index);103 LauncherItem *item = m_list.at(index);
104 if (!item->focused()) {104 if (!item->focused()) {
105 item->setAlerting(alerting);105 item->setAlerting(alerting);
106 Q_EMIT dataChanged(modelIndex, modelIndex, QVector<int>() << RoleAlerting);106 Q_EMIT dataChanged(modelIndex, modelIndex, {RoleAlerting});
107 }107 }
108 }108 }
109}109}
@@ -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);
@@ -414,10 +415,19 @@
414 } else {415 } else {
415 int idx = m_list.indexOf(item);416 int idx = m_list.indexOf(item);
416 item->setName(desktopFile.displayName());417 item->setName(desktopFile.displayName());
417 item->setIcon(desktopFile.icon());
418 item->setPinned(item->pinned()); // update pinned text if needed418 item->setPinned(item->pinned()); // update pinned text if needed
419 item->setRunning(item->running());419 item->setRunning(item->running());
420 Q_EMIT dataChanged(index(idx), index(idx), {RoleName, RoleIcon, RoleRunning});420 Q_EMIT dataChanged(index(idx), index(idx), {RoleName, RoleRunning});
421
422 const QString oldIcon = item->icon();
423 if (oldIcon == desktopFile.icon()) { // same icon file, perhaps different contents, simulate changing the icon name to force reload
424 item->setIcon(QString());
425 Q_EMIT dataChanged(index(idx), index(idx), {RoleIcon});
426 }
427
428 // now set the icon for real
429 item->setIcon(desktopFile.icon());
430 Q_EMIT dataChanged(index(idx), index(idx), {RoleIcon});
421 }431 }
422 }432 }
423433
@@ -489,7 +499,7 @@
489 if (idx >= 0) {499 if (idx >= 0) {
490 LauncherItem *item = m_list.at(idx);500 LauncherItem *item = m_list.at(idx);
491 setAlerting(item->appId(), true);501 setAlerting(item->appId(), true);
492 Q_EMIT dataChanged(index(idx), index(idx), QVector<int>() << RoleAlerting);502 Q_EMIT dataChanged(index(idx), index(idx), {RoleAlerting});
493 }503 }
494}504}
495505
496506
=== modified file 'plugins/Unity/Launcher/launchermodel.h'
--- plugins/Unity/Launcher/launchermodel.h 2015-07-29 12:32:57 +0000
+++ plugins/Unity/Launcher/launchermodel.h 2016-03-10 13:07:25 +0000
@@ -38,7 +38,7 @@
38 Q_OBJECT38 Q_OBJECT
3939
40public:40public:
41 LauncherModel(QObject *parent = 0);41 LauncherModel(QObject *parent = nullptr);
42 ~LauncherModel();42 ~LauncherModel();
4343
44 int rowCount(const QModelIndex &parent = QModelIndex()) const override;44 int rowCount(const QModelIndex &parent = QModelIndex()) const override;
4545
=== modified file 'qml/Launcher/LauncherDelegate.qml'
--- qml/Launcher/LauncherDelegate.qml 2015-11-19 16:55:31 +0000
+++ qml/Launcher/LauncherDelegate.qml 2016-03-10 13:07:25 +0000
@@ -135,6 +135,7 @@
135 sourceSize.width: iconShape.width135 sourceSize.width: iconShape.width
136 sourceSize.height: iconShape.height136 sourceSize.height: iconShape.height
137 source: root.iconName137 source: root.iconName
138 cache: false // see lpbug#1543290 why no cache
138 }139 }
139 }140 }
140141

Subscribers

People subscribed via source and target branches