Merge lp:~fboucault/music-app/startup_time into lp:music-app

[Florian Boucault]

Improve startup time (by around 900ms on BQ E4.5):
- load Icons asynchronously.
- load Images asynchronously.
- load Tabs on demand.

Improve startup time (by around 900ms on BQ E4.5):
- load Icons asynchronously.
- load Images asynchronously.
- load Tabs on demand.

[Andrew Hayzen (ahayzen)]

Thank you for looking into this :-)

FYI, we had a branch to do the tabs on demand and async before, but decided against it as it caused a flicker in the GridView and the thumbnailer needed to reload all the cover art (eg going from Artists->Albums->Artists). I wonder if we can make it smoother by fading the items or something? But even then, before, we preferred smoothness of changing tabs over loading performance in that case.

I'll see if I can find the branch, because what we also tried was loading the selected tab in async, and then once that has loaded, load the other tabs in the background async and then not unloading them so that there is no flicker.

Also, we know that the localstorage solution for restoring the queue on startup is slow, I'm currently resolving this upstream by fixing the load and save methods of the new Playlist component within qtubuntu-media and qtmultimedia. So don't worry about trying to speed up that part if you find it is slow :-)

Furthermore, I've found if you go Artists->Albums->Artists and scroll, you seem to have a double GridView in the Artists tab for some reason - I don't remember seeing this before.

[Andrew Hayzen (ahayzen)]

This is the super old branch from before [0], you can see the discussions we had. The preferred solution in that was to load the selected tab and then the others in async, but not unload. As the flicker was a large distraction to the user, note that the branch was on a much older codebase (before moving to GridView etc).

0 - https://code.launchpad.net/~ahayzen/music-app/refactor-async-loader-pages/+merge/248477

[Andrew Hayzen (ahayzen)]

I think the way to progress forwards with this branch is to land the async changes to images and icons. But the loading tabs on demand should be put in a separate branch and changed to load the current tab and then load the others in the background, but with no unloading.

Then when we go through our redesign, which currently changes to the way tabs are used and changes the existing tabs to head sections, we can load (and unload) the head sections on demand - as then the whole of the header won't flicker while loading.

[lp:~fboucault/music-app/startup_time]

1015. By Florian Boucault on 2016-10-19

Merged from trunk

1016. By Florian Boucault on 2016-10-19

Fixed double loading of artists page

1017. By Florian Boucault on 2016-10-19

Reduce flicker by loading tabs synchronously

1018. By Florian Boucault on 2016-10-19

Use image scaling adequately for music-app-cover.png

1019. By Florian Boucault on 2016-10-19

Nicer tabs transition by fading in art

[Andrew Hayzen (ahayzen)]

This is now looking visually much better and the code looks good as well. I just need to run autopilot on device and then it should be good to go :-) Thanks!

[Andrew Hayzen (ahayzen)]

Running this against autopilot on my device I get many failures related to any test that change the tab, this is one of the failures [1].

It appears that it is failing because it tried to check self.autoClose [2] *after* clicking the action, which then fails because the tab has changed and the one autopilot is looking at has been unloaded.

Either, we need to write our own helper, or we could capture StateNotFoundError errors in our switch_to_tab_wrapper() and assume that the next part of the text will fail/check we are in the right place. (Which most do as in the get_songs_page() case it then tried to get the Page, so it would fail then).

So probably the best solution for now is putting a try catch in the switch_to_tab_wrapper() [3].

1 - http://pastebin.ubuntu.com/23374367/
2 - http://bazaar.launchpad.net/~ubuntu-sdk-team/ubuntu-ui-toolkit/trunk/view/head:/tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/popups.py#L108
3 - http://bazaar.launchpad.net/~fboucault/music-app/startup_time/view/head:/tests/autopilot/music_app/__init__.py#L483

review: Needs Fixing

[lp:~fboucault/music-app/startup_time]

1020. By Florian Boucault on 2016-10-25

Fixed autopilot failures due to disappearing popovers

[Andrew Hayzen (ahayzen)]

Thanks for fixing those issues. LGTM, all tests 'pass' (I seem to be a getting one random failure due to the mediascanner mocking failing [also occurs on trunk so my device might just be racey]).

Sorry this took a long time to land.

review: Approve

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

review: Approve (continuous-integration)

=== modified file 'app/components/CoverGrid.qml'
--- app/components/CoverGrid.qml	2016-03-15 02:09:37 +0000
+++ app/components/CoverGrid.qml	2016-10-25 13:44:39 +0000
@@ -63,7 +63,7 @@
                         ? (coverGrid.covers[index].art !== undefined
                            ? coverGrid.covers[index].art
                            : "image://albumart/artist=" + coverGrid.covers[index].author + "&album=" + coverGrid.covers[index].album)
-                        : Qt.resolvedUrl("../graphics/[e-mail]")
+                        : Qt.resolvedUrl("../graphics/music-app-cover.png")
 
                 // TODO: This should be investigated once http://pad.lv/1391368
                 //       is resolved. Once it is, these can either be set to
@@ -89,6 +89,10 @@
                         firstSource = source
                     }
                 }
+                opacity: status == Image.Ready ? 1.0 : 0.0
+                Behavior on opacity {
+                    UbuntuNumberAnimation {}
+                }
             }
         }
     }

=== modified file 'app/components/Delegates/MusicListItem.qml'
--- app/components/Delegates/MusicListItem.qml	2016-02-26 03:16:35 +0000
+++ app/components/Delegates/MusicListItem.qml	2016-10-25 13:44:39 +0000
@@ -105,7 +105,7 @@
 
             onStatusChanged: {
                 if (status === Image.Error) {
-                    source = Qt.resolvedUrl("../../graphics/[e-mail]")
+                    source = Qt.resolvedUrl("../../graphics/music-app-cover.png")
                 }
             }
 

=== modified file 'app/components/ListItemReorderComponent.qml'
--- app/components/ListItemReorderComponent.qml	2015-08-12 23:36:44 +0000
+++ app/components/ListItemReorderComponent.qml	2016-10-25 13:44:39 +0000
@@ -33,6 +33,7 @@
         name: "navigation-menu"  // TODO: use proper image
         height: width
         width: units.gu(3)
+        asynchronous: true
     }
 
     MouseArea {

=== modified file 'app/components/MusicToolbar.qml'
--- app/components/MusicToolbar.qml	2016-01-16 17:46:59 +0000
+++ app/components/MusicToolbar.qml	2016-10-25 13:44:39 +0000
@@ -108,6 +108,7 @@
                           "media-playback-pause" : "media-playback-start"
                 objectName: "disabledSmallPlayShape"
                 width: height
+                asynchronous: true
             }
 
             /* Click to shuffle music */
@@ -203,6 +204,7 @@
                           "media-playback-pause" : "media-playback-start"
                 objectName: "playShape"
                 width: height
+                asynchronous: true
             }
 
             /* Mouse area to jump to now playing */

=== modified file 'app/components/NowPlayingToolbar.qml'
--- app/components/NowPlayingToolbar.qml	2016-03-07 20:01:22 +0000
+++ app/components/NowPlayingToolbar.qml	2016-10-25 13:44:39 +0000
@@ -54,6 +54,7 @@
             name: "media-playlist-repeat"
             objectName: "repeatShape"
             opacity: player.repeat ? 1 : .2
+            asynchronous: true
         }
     }
 
@@ -78,6 +79,7 @@
             name: "media-skip-backward"
             objectName: "previousShape"
             opacity: parent.enabled ? 1 : .2
+            asynchronous: true
         }
     }
 
@@ -98,6 +100,7 @@
             color: parent.pressed ? UbuntuColors.blue : "white"
             name: player.mediaPlayer.playbackState === MediaPlayer.PlayingState ? "media-playback-pause" : "media-playback-start"
             objectName: "playShape"
+            asynchronous: true
         }
     }
 
@@ -122,6 +125,7 @@
             name: "media-skip-forward"
             objectName: "forwardShape"
             opacity: parent.enabled ? 1 : .2
+            asynchronous: true
         }
     }
 
@@ -145,6 +149,7 @@
             name: "media-playlist-shuffle"
             objectName: "shuffleShape"
             opacity: player.shuffle ? 1 : .2
+            asynchronous: true
         }
     }
 

=== modified file 'app/components/Walkthrough/Slide1.qml'
--- app/components/Walkthrough/Slide1.qml	2015-10-18 17:45:48 +0000
+++ app/components/Walkthrough/Slide1.qml	2016-10-25 13:44:39 +0000
@@ -41,6 +41,7 @@
             source:  Image {
                 id: centerImage
                 source: Qt.resolvedUrl("../../graphics/[e-mail]")
+                asynchronous: true
             }
 
             width: height

=== modified file 'app/components/Walkthrough/Slide2.qml'
--- app/components/Walkthrough/Slide2.qml	2015-08-12 23:36:44 +0000
+++ app/components/Walkthrough/Slide2.qml	2016-10-25 13:44:39 +0000
@@ -39,6 +39,7 @@
             height: (parent.height - bodyText.contentHeight - introductionText.height - 4*units.gu(4))/2
             fillMode: Image.PreserveAspectFit
             source: Qt.resolvedUrl("../../graphics/sd_phone_icon.png")
+            asynchronous: true
         }
 
         Label {

=== modified file 'app/components/Walkthrough/Slide3.qml'
--- app/components/Walkthrough/Slide3.qml	2015-08-12 23:36:44 +0000
+++ app/components/Walkthrough/Slide3.qml	2016-10-25 13:44:39 +0000
@@ -39,6 +39,7 @@
             height: (parent.height - introductionText.height - finalMessage.contentHeight - 4.5*units.gu(4))/2
             fillMode: Image.PreserveAspectFit
             source: Qt.resolvedUrl("../../graphics/music_download_icon.png")
+            asynchronous: true
         }
 
         Label {

=== modified file 'app/components/Walkthrough/Walkthrough.qml'
--- app/components/Walkthrough/Walkthrough.qml	2015-10-28 01:05:33 +0000
+++ app/components/Walkthrough/Walkthrough.qml	2016-10-25 13:44:39 +0000
@@ -171,6 +171,7 @@
                 height: width
                 source: listView.currentIndex == index ?  "../../graphics/[e-mail]" : "../../graphics/[e-mail]"
                 width: units.gu(1.5)
+                asynchronous: true
             }
         }
     }
@@ -187,6 +188,7 @@
         name: "chevron"
         visible: listView.currentIndex !== 2
         width: height
+        asynchronous: true
     }
 
     MouseArea {

=== renamed file 'app/graphics/[e-mail]' => 'app/graphics/[e-mail]'
=== modified file 'app/music-app.qml'
--- app/music-app.qml	2016-08-15 22:27:23 +0000
+++ app/music-app.qml	2016-10-25 13:44:39 +0000
@@ -446,10 +446,8 @@
 
                 // TODO: improve in refactoring to be able detect when a track is removed
                 // Update playlists page
-                if (playlistsPage.visible) {
+                if (tabs.selectedTab == playlistsTab) {
                     playlistModel.filterPlaylists()
-                } else {
-                    playlistsPage.changed = true
                 }
             }
         }
@@ -587,7 +585,7 @@
         // Go back up the stack if possible
         function goBack() {
             // Ensure in the case that goBack is called programmatically that any dialogs are closed
-            if (mainPageStack.currentMusicPage.currentDialog !== null) {
+            if (mainPageStack.currentMusicPage && mainPageStack.currentMusicPage.currentDialog !== null) {
                 PopupUtils.close(mainPageStack.currentMusicPage.currentDialog)
             }
 
@@ -677,6 +675,9 @@
                 }
             ]
 
+            property bool completed: false
+            Component.onCompleted: completed = true
+
             onSelectedTabChanged: {
                 // pause loading of the models in the old tab
                 if (lastTab !== null && lastTab !== selectedTab) {
@@ -717,11 +718,14 @@
                         id: recentTab
                         objectName: "recentTab"
                         anchors.fill: parent
-                        title: page.title
+                        title: i18n.tr("Recent")
 
                         // Tab content begins here
-                        page: Recent {
-                            id: recentPage
+                        page: Loader {
+                            width: mainPageStack.width
+                            height: mainPageStack.height
+                            active: tabs.selectedTab == recentTab
+                            source: Qt.resolvedUrl("ui/Recent.qml")
                         }
                     }
                 }
@@ -764,11 +768,16 @@
                 id: artistsTab
                 objectName: "artistsTab"
                 anchors.fill: parent
-                title: page.title
+                title: i18n.tr("Artists")
 
                 // tab content
-                page: Artists {
-                    id: artistsPage
+                page: Loader {
+                    width: mainPageStack.width
+                    height: mainPageStack.height
+                    // condition on tabs.completed necessary to avoid QTBUG 54657
+                    // https://bugreports.qt.io/browse/QTBUG-54657
+                    active: tabs.completed && tabs.selectedTab == artistsTab
+                    source: Qt.resolvedUrl("ui/Artists.qml")
                 }
             }
 
@@ -781,11 +790,16 @@
                 id: albumsTab
                 objectName: "albumsTab"
                 anchors.fill: parent
-                title: page.title
+                title: i18n.tr("Albums")
 
                 // Tab content begins here
-                page: Albums {
-                    id: albumsPage
+                page: Loader {
+                    width: mainPageStack.width
+                    height: mainPageStack.height
+                    // condition on tabs.completed necessary to avoid QTBUG 54657
+                    // https://bugreports.qt.io/browse/QTBUG-54657
+                    active: tabs.completed && tabs.selectedTab == albumsTab
+                    source: Qt.resolvedUrl("ui/Albums.qml")
                 }
             }
 
@@ -798,11 +812,16 @@
                 id: genresTab
                 objectName: "genresTab"
                 anchors.fill: parent
-                title: page.title
+                title: i18n.tr("Genres")
 
                 // Tab content begins here
-                page: Genres {
-                    id: genresPage
+                page: Loader {
+                    width: mainPageStack.width
+                    height: mainPageStack.height
+                    // condition on tabs.completed necessary to avoid QTBUG 54657
+                    // https://bugreports.qt.io/browse/QTBUG-54657
+                    active: tabs.completed && tabs.selectedTab == genresTab
+                    source: Qt.resolvedUrl("ui/Genres.qml")
                 }
             }
 
@@ -815,11 +834,16 @@
                 id: songsTab
                 objectName: "songsTab"
                 anchors.fill: parent
-                title: page.title
+                title: i18n.tr("Tracks")
 
                 // Tab content begins here
-                page: Songs {
-                    id: tracksPage
+                page: Loader {
+                    width: mainPageStack.width
+                    height: mainPageStack.height
+                    // condition on tabs.completed necessary to avoid QTBUG 54657
+                    // https://bugreports.qt.io/browse/QTBUG-54657
+                    active: tabs.completed && tabs.selectedTab == songsTab
+                    source: Qt.resolvedUrl("ui/Songs.qml")
                 }
             }
 
@@ -832,11 +856,16 @@
                 id: playlistsTab
                 objectName: "playlistsTab"
                 anchors.fill: parent
-                title: page.title
+                title: i18n.tr("Playlists")
 
                 // Tab content begins here
-                page: Playlists {
-                    id: playlistsPage
+                page: Loader {
+                    width: mainPageStack.width
+                    height: mainPageStack.height
+                    // condition on tabs.completed necessary to avoid QTBUG 54657
+                    // https://bugreports.qt.io/browse/QTBUG-54657
+                    active: tabs.completed && tabs.selectedTab == playlistsTab
+                    source: Qt.resolvedUrl("ui/Playlists.qml")
                 }
             }
 

=== modified file 'app/ui/Albums.qml'
--- app/ui/Albums.qml	2016-03-07 20:01:22 +0000
+++ app/ui/Albums.qml	2016-10-25 13:44:39 +0000
@@ -44,16 +44,6 @@
         }
     ]
 
-    // FIXME: workaround for pad.lv/1531016 (gridview juddery)
-    anchors {
-        bottom: parent.bottom
-        fill: undefined
-        left: parent.left
-        top: parent.top
-    }
-    height: mainPageStack.height
-    width: mainPageStack.width
-
     // Hack for autopilot otherwise Albums appears as MusicPage
     // due to bug 1341671 it is required that there is a property so that
     // qml doesn't optimise using the parent type

=== modified file 'app/ui/Artists.qml'
--- app/ui/Artists.qml	2016-03-07 20:01:22 +0000
+++ app/ui/Artists.qml	2016-10-25 13:44:39 +0000
@@ -48,16 +48,6 @@
         }
     ]
 
-    // FIXME: workaround for pad.lv/1531016 (gridview juddery)
-    anchors {
-        bottom: parent.bottom
-        fill: undefined
-        left: parent.left
-        top: parent.top
-    }
-    height: mainPageStack.height
-    width: mainPageStack.width
-
     // Hack for autopilot otherwise Artists appears as MusicPage
     // due to bug 1341671 it is required that there is a property so that
     // qml doesn't optimise using the parent type

=== modified file 'app/ui/Genres.qml'
--- app/ui/Genres.qml	2016-03-07 20:01:22 +0000
+++ app/ui/Genres.qml	2016-10-25 13:44:39 +0000
@@ -44,16 +44,6 @@
         }
     ]
 
-    // FIXME: workaround for pad.lv/1531016 (gridview juddery)
-    anchors {
-        bottom: parent.bottom
-        fill: undefined
-        left: parent.left
-        top: parent.top
-    }
-    height: mainPageStack.height
-    width: mainPageStack.width
-
     // Hack for autopilot otherwise Albums appears as MusicPage
     // due to bug 1341671 it is required that there is a property so that
     // qml doesn't optimise using the parent type

=== modified file 'app/ui/LibraryEmptyState.qml'
--- app/ui/LibraryEmptyState.qml	2015-12-20 00:10:13 +0000
+++ app/ui/LibraryEmptyState.qml	2016-10-25 13:44:39 +0000
@@ -75,6 +75,7 @@
                         height: units.gu(10)
                         smooth: true
                         source: "../graphics/music_download_icon.png"
+                        asynchronous: true
                     }
                 }
 
@@ -93,6 +94,7 @@
                         height: units.gu(6)
                         smooth: true
                         source: "../graphics/div.png"
+                        asynchronous: true
                     }
                 }
 
@@ -106,6 +108,7 @@
                     height: units.gu(7)
                     smooth: true
                     source: "../graphics/sd_phone_icon.png"
+                    asynchronous: true
                 }
             }
 

=== modified file 'app/ui/Playlists.qml'
--- app/ui/Playlists.qml	2016-03-07 20:01:22 +0000
+++ app/ui/Playlists.qml	2016-10-25 13:44:39 +0000
@@ -49,16 +49,6 @@
         }
     ]
 
-    // FIXME: workaround for pad.lv/1531016 (gridview juddery)
-    anchors {
-        bottom: parent.bottom
-        fill: undefined
-        left: parent.left
-        top: parent.top
-    }
-    height: mainPageStack.height
-    width: mainPageStack.width
-
     property bool changed: false
     property bool childrenChanged: false
 

=== modified file 'app/ui/Recent.qml'
--- app/ui/Recent.qml	2016-08-15 23:15:01 +0000
+++ app/ui/Recent.qml	2016-10-25 13:44:39 +0000
@@ -73,16 +73,6 @@
     }
     title: i18n.tr("Recent")
 
-    // FIXME: workaround for pad.lv/1531016 (gridview juddery)
-    anchors {
-        bottom: parent.bottom
-        fill: undefined
-        left: parent.left
-        top: parent.top
-    }
-    height: mainPageStack.height
-    width: mainPageStack.width
-
     property bool changed: false
     property bool childrenChanged: false
 

=== modified file 'tests/autopilot/music_app/__init__.py'
--- tests/autopilot/music_app/__init__.py	2016-08-15 19:56:11 +0000
+++ tests/autopilot/music_app/__init__.py	2016-10-25 13:44:39 +0000
@@ -9,6 +9,7 @@
 from ubuntuuitoolkit import (
     MainView, UbuntuUIToolkitCustomProxyObjectBase, UCListItem
 )
+from autopilot.introspection import dbus
 
 
 class MusicAppException(Exception):
@@ -482,6 +483,11 @@
 
     def switch_to_tab_wrapper(self, objectName):
         # We use leadingActionBar instead of Tabs so create wrapper
-        self.wait_select_single(
+        action_bar = self.wait_select_single(
             "ActionBar", objectName="tabsLeadingActionBar", visible=True,
-        ).click_action_button(objectName)
+        )
+        try:
+            action_bar.click_action_button(objectName)
+        except dbus.StateNotFoundError:
+            # the popover was deleted when clicking the button
+            pass