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

[Marinus Schraal <mschraal src gnome org>]

commit 8e9e5e541f2798d5d974a467800afa385e850fa4
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 a8579159..37c8f230 100644
--- a/gnomemusic/coremodel.py
+++ b/gnomemusic/coremodel.py
@@ -68,7 +68,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 99ea61f2..5b147aa8 100644
--- a/gnomemusic/grilowrappers/grltrackerplaylists.py
+++ b/gnomemusic/grilowrappers/grltrackerplaylists.py
@@ -148,7 +148,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 03408fa8..7daf5a56 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 ec3d2541..aca16934 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(application, self._view)
 
         play_song = self._window.lookup_action("play_song")
@@ -91,11 +95,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._window.disconnect(self._selection_mode_id)
 
@@ -130,15 +135,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)
-        self._on_playlist_activated(self._sidebar, first_row)
-
     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
 
@@ -146,7 +152,7 @@ class PlaylistsView(BaseView):
                     or self._sidebar.get_row_at_index(position - 1))
         if row_next:
             self._sidebar.select_row(row_next)
-            self._on_playlist_activated(self._sidebar, row_next)
+            self._on_playlist_activated(self._sidebar, row_next, True)
 
     @log
     def _on_view_right_clicked(self, gesture, n_press, x, y):
@@ -199,8 +205,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
 
         self._view.bind_model(
@@ -243,7 +252,7 @@ class PlaylistsView(BaseView):
             return
 
         self._sidebar.select_row(row)
-        self._on_playlist_activated(self._sidebar, row)
+        self._on_playlist_activated(self._sidebar, row, True)
         if playlist.props.model.get_n_items() > 0:
             self._song_activated(None)
         else: