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

[Michael Zanetti]

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.

 * 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

[Albert Astals Cid (aacid)]

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

review: Needs Information

[Unity8 CI Bot (unity8-ci-bot)]

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)

[lp:~mzanetti/unity8/parent-that-launcheritem]

2216. By Michael Zanetti on 2016-03-09

prevent the same mistake from happening again

[Albert Astals Cid (aacid)]

 * 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

[Unity8 CI Bot (unity8-ci-bot)]

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)

[lp:~mzanetti/unity8/parent-that-launcheritem]

2217. By Michael Zanetti on 2016-03-10

merge ~lukas-kde/unity8/dynamicLauncherIcons as prereq

Unmerged revisions

=== 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 @@
     m_alerting(false),
     m_quickList(new QuickListModel(this))
 {
+    Q_ASSERT(parent != nullptr);
     QuickListEntry nameAction;
     nameAction.setActionId(QStringLiteral("launch_item"));
     nameAction.setText(m_name);

=== 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 @@
 {
     Q_OBJECT
 public:
-    LauncherItem(const QString &appId, const QString &name, const QString &icon, QObject *parent = 0);
+    LauncherItem(const QString &appId, const QString &name, const QString &icon, QObject *parent);
 
     QString appId() const override;
     QString name() const override;

=== 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 @@
         LauncherItem *item = m_list.at(index);
         if (!item->focused()) {
             item->setAlerting(alerting);
-            Q_EMIT dataChanged(modelIndex, modelIndex, QVector<int>() << RoleAlerting);
+            Q_EMIT dataChanged(modelIndex, modelIndex, {RoleAlerting});
         }
     }
 }
@@ -389,7 +389,8 @@
         if (countVisible && desktopFile.isValid()) {
             LauncherItem *item = new LauncherItem(appId,
                                                   desktopFile.displayName(),
-                                                  desktopFile.icon());
+                                                  desktopFile.icon(),
+                                                  this);
             item->setCountVisible(true);
             beginInsertRows(QModelIndex(), m_list.count(), m_list.count());
             m_list.append(item);
@@ -414,10 +415,19 @@
         } else {
             int idx = m_list.indexOf(item);
             item->setName(desktopFile.displayName());
-            item->setIcon(desktopFile.icon());
             item->setPinned(item->pinned()); // update pinned text if needed
             item->setRunning(item->running());
-            Q_EMIT dataChanged(index(idx), index(idx), {RoleName, RoleIcon, RoleRunning});
+            Q_EMIT dataChanged(index(idx), index(idx), {RoleName, RoleRunning});
+
+            const QString oldIcon = item->icon();
+            if (oldIcon == desktopFile.icon()) { // same icon file, perhaps different contents, simulate changing the icon name to force reload
+                item->setIcon(QString());
+                Q_EMIT dataChanged(index(idx), index(idx), {RoleIcon});
+            }
+
+            // now set the icon for real
+            item->setIcon(desktopFile.icon());
+            Q_EMIT dataChanged(index(idx), index(idx), {RoleIcon});
         }
     }
 
@@ -489,7 +499,7 @@
     if (idx >= 0) {
         LauncherItem *item = m_list.at(idx);
         setAlerting(item->appId(), true);
-        Q_EMIT dataChanged(index(idx), index(idx), QVector<int>() << RoleAlerting);
+        Q_EMIT dataChanged(index(idx), index(idx), {RoleAlerting});
     }
 }
 

=== 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 @@
    Q_OBJECT
 
 public:
-    LauncherModel(QObject *parent = 0);
+    LauncherModel(QObject *parent = nullptr);
     ~LauncherModel();
 
     int rowCount(const QModelIndex &parent = QModelIndex()) const override;

=== 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 @@
                 sourceSize.width: iconShape.width
                 sourceSize.height: iconShape.height
                 source: root.iconName
+                cache: false // see lpbug#1543290 why no cache
             }
         }