Merge lp:~lukas-kde/unity8/dynamicLauncherIcons into lp:unity8

[Lukáš Tinkl]

Watch for launcher item icon changes

Watch for launcher item icon changes

Fixes #1543290 - Icons of apps pinned in the launcher don't update if app got updated and has a new icon

Checklist:

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

Yes

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

Yes

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

N/A

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

FAILED: Continuous integration, rev:2175
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/295/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/416/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/439
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/457
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/457
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/453/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/453/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/453/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/453/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/453/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/453/console

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

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

hmm... We already have a file system watcher for launcher stuff... Check out data/unity8-filewatcher.conf

The reason for not using a QFileSystemWatcher directly for that was there is a limited number of watches that a process can set up. Can we update the icons on that UpdateLauncher call too?

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

The existing watcher is observing changes in the desktop files only (applications being added/removed) using:

data/unity8-filewatcher.conf:

start on (file FILE=/home/phablet/.local/share/applications/*.desktop) or (file FILE=/usr/share/applications/*.desktop)

Here we have to watch icons' actual contents changes which is doable only from the model where we have the path to the icon (the .desktop file doesn't have the full icon path).

[Michael Zanetti (mzanetti)]

I don't think an app icon can change without it's desktop file being touched too. (Except, obviously, if the user replaces the file manually in the file system). But during app updates the .desktop file is always overwritten (even if sometimes with the same content) which then would always call the UpdateLauncher D-Bus method.

If possible, I'd like to try hooking into that mechanism instead of setting up more FSWatchers.

[Lukáš Tinkl (lukas-kde)]

OK done, reused the the existing upstart watcher. I have to reset the whole model when the refresh() method is called tho... otherwise QML won't notice the change and won't redraw the icon because the "icon" property stays the same (unlike its contents).

[Michael Zanetti (mzanetti)]

resetting the model will mess up scrolling positions, it would destroy opened quicklists and what not.

Which I understand that some sort of hack will be needed, I'd suggest something easier for the view. Perhaps like this?

QString tmp = m_icon;
m_icon.clear();
emit dataChanged({RoleIcon});
m_icon = tmp;
emit dataChanged({RoleIcon});

review: Needs Information

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

FAILED: Continuous integration, rev:2176
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/298/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/419/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/442
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/460
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/460
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/456/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/456/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/456/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/456/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/456/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/456/console

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

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

Done, still works if I simulate the model reset by setting the icon twice

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

FAILED: Continuous integration, rev:2177
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/299/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/420/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/443
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/461
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/461
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/457/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/457/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/457/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/457/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/457/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/457/console

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2175
http://jenkins.qa.ubuntu.com/job/unity8-ci/7237/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6384/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/652/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1942/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/645/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1837/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1837/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/644/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/643/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6395/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/650/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7237/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2177
http://jenkins.qa.ubuntu.com/job/unity8-ci/7243/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6390/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/658/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1948/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/651/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1843/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1843/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/650/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/649/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6401/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/656/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7243/rebuild

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

two small inline comments

[Michael Zanetti (mzanetti)]

something's wrong with merging... has some conflicts

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

Submitted on top of trunk, should be good now

[Michael Zanetti (mzanetti)]

hmm... for some reason it doesn't seem to work for me...

I've tested with manually installing this app [1] and then upgrading to the latest version in the systemsettings:

[1] http://notyetthere.org/data/com.ubuntu.developer.majster-pl.utorch_2.7.5_all.click

review: Needs Fixing

[lp:~lukas-kde/unity8/dynamicLauncherIcons]

2210. By Lukáš Tinkl on 2016-03-02

no cache, more fun

[Lukáš Tinkl (lukas-kde)]

> hmm... for some reason it doesn't seem to work for me...
>
> I've tested with manually installing this app [1] and then upgrading to the
> latest version in the systemsettings:
>
> [1] http://notyetthere.org/data/com.ubuntu.developer.majster-
> pl.utorch_2.7.5_all.click

OK, it was the cache after all :) Fixed

[Michael Zanetti (mzanetti)]

I'm still not a big fan of generally disabling the cache, but ok... it seems to be required in order to reload the picture and everything else (e.g. a Loader around the shape or similar) actually seems overkill too for the fact that we only have a limited number of items in the launcher.

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

yes

 * Did CI run pass? If not, please explain why.

yes

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

yes

review: Approve

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

PASSED: Continuous integration, rev:2210
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/550/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/308
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/308
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/308
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/739
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/758
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/758
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/754/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== 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-02 13:12:05 +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 = nullptr);
 
     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-02 13:12:05 +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});
         }
     }
 }
@@ -414,10 +414,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 +498,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-02 13:12:05 +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-02 13:12:05 +0000
@@ -135,6 +135,7 @@
                 sourceSize.width: iconShape.width
                 sourceSize.height: iconShape.height
                 source: root.iconName
+                cache: false // see lpbug#1543290 why no cache
             }
         }