[gnome-music/wip/mschraal/remove-playlists-loaded-signal-3-34: 2/2] Let consumers handle playlists availability

[Marinus Schraal <mschraal src gnome org>]

commit e282aab5d84967e9d8adc4ec320cc2981ee3b5b0
Author: Marinus Schraal <mschraal gnome org>
Date:   Thu Jan 9 16:06:39 2020 +0100

    Let consumers handle playlists availability
    
    PlaylistsView and MPRIS want to know when playlists become available to
    setup some actions. This was previously done by emitting a signal after
    the initial load of playlists. However, in certain  conditions this
    signal could already have fired before the consumer was even read, so no
    initial action would be taken.
    
    For example, PlaylistsView by default selects a row for intial display
    when the list is filled. However, without a signal being received, no
    row is selected to display.
    
    Now PlaylistsView instead directly listens to the playlists model for
    changes and uses an initial state variable to check if it is needed to
    activate a row.
    
    Closes: #348

 gnomemusic/coremodel.py                         |  1 -
 gnomemusic/grilowrappers/grltrackerplaylists.py |  1 -
 gnomemusic/mpris.py                             | 38 +++++++++----------------
 gnomemusic/views/playlistsview.py               | 35 ++++++++++++++---------
 4 files changed, 35 insertions(+), 40 deletions(-)
---
diff --git a/gnomemusic/coremodel.py b/gnomemusic/coremodel.py
index 13a8bc0e..09d5ac78 100644
--- a/gnomemusic/coremodel.py
+++ b/gnomemusic/coremodel.py
@@ -67,7 +67,6 @@ class CoreModel(GObject.GObject):
     __gsignals__ = {
         "artists-loaded": (GObject.SignalFlags.RUN_FIRST, None, ()),
         "playlist-loaded": (GObject.SignalFlags.RUN_FIRST, None, (int,)),
-        "playlists-loaded": (GObject.SignalFlags.RUN_FIRST, None, ()),
     }
 
     active_playlist = GObject.Property(type=Playlist, default=None)
diff --git a/gnomemusic/grilowrappers/grltrackerplaylists.py b/gnomemusic/grilowrappers/grltrackerplaylists.py
index 57ed1c42..dd32baef 100644
--- a/gnomemusic/grilowrappers/grltrackerplaylists.py
+++ b/gnomemusic/grilowrappers/grltrackerplaylists.py
@@ -145,7 +145,6 @@ class GrlTrackerPlaylists(GObject.GObject):
             self._window.notifications_popup.pop_loading()
             return
         if not media:
-            self._coremodel.emit("playlists-loaded")
             self._window.notifications_popup.pop_loading()
             return
 
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index f3ae494e..dc29f664 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -303,8 +303,11 @@ class MPRIS(DBusInterface):
             "playlist-loaded", self._on_player_playlist_changed)
 
         self._playlists_model = self._coremodel.props.playlists_sort
-        self._playlists_loaded_id = self._coremodel.connect(
-            "playlists-loaded", self._on_playlists_loaded)
+        n_items = self._playlists_model.get_n_items()
+        self._on_playlists_items_changed(
+            self._playlists_model, 0, n_items, n_items)
+        self._playlists_model.connect(
+            "items-changed", self._on_playlists_items_changed)
 
         self._player_playlist_type = None
         self._path_list = []
@@ -316,7 +319,6 @@ class MPRIS(DBusInterface):
         self._previous_loop_status = ""
         self._previous_mpris_playlist = self._get_active_playlist()
         self._previous_playback_status = "Stopped"
-        self._previous_playlist_count = 0
 
     @log
     def _get_playback_status(self):
@@ -607,33 +609,19 @@ class MPRIS(DBusInterface):
         self._properties_changed(
             MPRIS.MEDIA_PLAYER2_PLAYLISTS_IFACE, properties, [])
 
-    def _on_playlists_loaded(self, klass):
-        self._coremodel.disconnect(self._playlists_loaded_id)
-        for playlist in self._playlists_model:
-            playlist.connect("notify::title", self._on_playlist_renamed)
-            playlist.connect(
-                "notify::active", self._on_player_playlist_changed)
-
-        self._playlists_model.connect(
-            "items-changed", self._on_playlists_count_changed)
-        self._on_playlists_count_changed(None, None, 0, 0)
-
-    @log
-    def _on_playlists_count_changed(self, model, position, removed, added):
-        playlist_count = self._playlists_model.get_n_items()
-        if playlist_count == self._previous_playlist_count:
-            return
+    def _on_playlists_items_changed(self, model, position, removed, added):
+        if added > 0:
+            for i in range(added):
+                playlist = model[position + i]
+                playlist.connect("notify::title", self._on_playlist_renamed)
+                playlist.connect(
+                    "notify::active", self._on_player_playlist_changed)
 
-        self._previous_playlist_count = playlist_count
+        playlist_count = model.get_n_items()
         properties = {"PlaylistCount": GLib.Variant("u", playlist_count)}
         self._properties_changed(
             MPRIS.MEDIA_PLAYER2_PLAYLISTS_IFACE, properties, [])
 
-        if added == 0:
-            return
-
-        model[position].connect("notify::title", self._on_playlist_renamed)
-
     @log
     def _on_playlist_renamed(self, playlist, param):
         mpris_playlist = self._get_mpris_playlist_from_playlist(playlist)
diff --git a/gnomemusic/views/playlistsview.py b/gnomemusic/views/playlistsview.py
index e7c60755..8a38d565 100644
--- a/gnomemusic/views/playlistsview.py
+++ b/gnomemusic/views/playlistsview.py
@@ -62,6 +62,10 @@ class PlaylistsView(BaseView):
         self._window = application.props.window
         self._player = player
 
+        # This indicates if the current list has been empty and has
+        # had no user interaction since.
+        self._untouched_list = True
+
         self._song_popover = PlaylistContextMenu(self._view)
 
         play_song = Gio.SimpleAction.new('play_song', None)
@@ -108,11 +112,12 @@ class PlaylistsView(BaseView):
 
         self._sidebar.bind_model(self._model, self._add_playlist_to_sidebar)
 
-        self._loaded_id = self._coremodel.connect(
-            "playlists-loaded", self._on_playlists_loaded)
         self._active_playlist_id = self._coremodel.connect(
             "notify::active-playlist", self._on_active_playlist_changed)
 
+        self._model.connect("items-changed", self._on_playlists_model_changed)
+        self._on_playlists_model_changed(self._model, 0, 0, 0)
+
         # Selection is only possible from the context menu
         self.disconnect(self._selection_mode_id)
 
@@ -147,15 +152,16 @@ class PlaylistsView(BaseView):
         row = PlaylistTile(playlist)
         return row
 
-    def _on_playlists_loaded(self, klass):
-        self._coremodel.disconnect(self._loaded_id)
-        self._model.connect("items-changed", self._on_playlists_model_changed)
-
-        first_row = self._sidebar.get_row_at_index(0)
-        self._sidebar.select_row(first_row)
-        first_row.emit("activate")
-
     def _on_playlists_model_changed(self, model, position, removed, added):
+        if model.get_n_items() == 0:
+            self._untouched_list = True
+            return
+        elif self._untouched_list is True:
+            first_row = self._sidebar.get_row_at_index(0)
+            self._sidebar.select_row(first_row)
+            self._on_playlist_activated(self._sidebar, first_row, True)
+            return
+
         if removed == 0:
             return
 
@@ -163,7 +169,7 @@ class PlaylistsView(BaseView):
                     or self._sidebar.get_row_at_index(position - 1))
         if row_next:
             self._sidebar.select_row(row_next)
-            row_next.emit("activate")
+            self._on_playlist_activated(self._sidebar, row_next, True)
 
     @log
     def _on_view_right_clicked(self, gesture, n_press, x, y):
@@ -216,8 +222,11 @@ class PlaylistsView(BaseView):
             coresong)
 
     @log
-    def _on_playlist_activated(self, sidebar, row, data=None):
+    def _on_playlist_activated(self, sidebar, row, untouched=False):
         """Update view with content from selected playlist"""
+        if untouched is False:
+            self._untouched_list = False
+
         playlist = row.props.playlist
 
         if self.rename_active:
@@ -265,7 +274,7 @@ class PlaylistsView(BaseView):
             return
 
         self._sidebar.select_row(row)
-        row.emit('activate')
+        self._on_playlist_activated(self._sidebar, row, True)
         if playlist.props.model.get_n_items() > 0:
             self._song_activated(None)
         else: