[gnome-music/wip/jfelder/model-remove-active-playlist] coremodel: Simplify active_playlist usage

[Jean Felder <jfelder src gnome org>]

commit 81278d18ddb9d17c786ed22044ebe5d4b3853205
Author: Jean Felder <jfelder src gnome org>
Date:   Tue Jan 21 12:05:41 2020 +0100

    coremodel: Simplify active_playlist usage
    
    There are two ways to start playing a playlist:
    - from PlaylistsView, by clicking on a song
    - from an MPRIS client which handles the MediaPlayer2.Playlists
    interface
    
    The active_playlist property from CoreModel is used to keep track of
    the changes and notify PlaylistsView when an MPRIS client requests to
    play a new playlist. In that case, PlaylistsView displays the
    requested playlist and starts playing it.
    However, this approach is too complicated. While starting to play a
    playlist, PlaylistsView needs to block the "notify::active-playlist"
    notification to prevent an infinite recursive call. In fact, MPRIS
    could load and start playing the playlist itself.
    
    With this change, when an MPRIS client requests a new playlist, MPRIS
    loads and starts playing it. Then, PlaylistsView only needs to display
    it.

 gnomemusic/mpris.py               | 25 +++++++++++++++++++++++--
 gnomemusic/views/playlistsview.py | 38 +++++++++-----------------------------
 2 files changed, 32 insertions(+), 31 deletions(-)
---
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index dc9822a4..4d495d38 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -769,6 +769,17 @@ class MPRIS(DBusInterface):
             "CurrentTrack": self._get_song_dbus_path()}
         self._dbus_emit_signal("TrackListReplaced", parameters)
 
+    def _load_player_playlist(self, playlist):
+        def _on_playlist_loaded(klass, playlist_type):
+            self._coremodel.props.active_playlist = playlist
+            self._player.play()
+            self._coremodel.disconnect(loaded_id)
+
+        loaded_id = self._coremodel.connect(
+            "playlist-loaded", _on_playlist_loaded)
+        self._coremodel.set_player_model(
+            PlayerPlaylist.Type.PLAYLIST, playlist.props.model)
+
     def _activate_playlist(self, playlist_path):
         """Starts playing the given playlist (MPRIS Method).
 
@@ -780,8 +791,18 @@ class MPRIS(DBusInterface):
                 selected_playlist = playlist
                 break
 
-        if selected_playlist is not None:
-            self._coremodel.props.active_playlist = selected_playlist
+        if selected_playlist is None:
+            return
+
+        def _on_playlist_model_loaded(playlist):
+            playlist.disconnect(signal_id)
+            self._load_player_playlist(playlist)
+
+        if selected_playlist.props.model.get_n_items() > 0:
+            self._load_player_playlist(selected_playlist)
+        else:
+            signal_id = selected_playlist.connect(
+                "playlist-loaded", _on_playlist_model_loaded)
 
     def _get_playlists(self, index, max_count, order, reverse):
         """Gets a set of playlists (MPRIS Method).
diff --git a/gnomemusic/views/playlistsview.py b/gnomemusic/views/playlistsview.py
index ec3d2541..c82c9be7 100644
--- a/gnomemusic/views/playlistsview.py
+++ b/gnomemusic/views/playlistsview.py
@@ -93,7 +93,7 @@ class PlaylistsView(BaseView):
 
         self._loaded_id = self._coremodel.connect(
             "playlists-loaded", self._on_playlists_loaded)
-        self._active_playlist_id = self._coremodel.connect(
+        self._coremodel.connect(
             "notify::active-playlist", self._on_active_playlist_changed)
 
         # Selection is only possible from the context menu
@@ -211,23 +211,15 @@ class PlaylistsView(BaseView):
         self._remove_song_action.set_enabled(not playlist.props.is_smart)
 
     def _on_active_playlist_changed(self, klass, val):
-        """Selects and starts playing a playlist.
-
-        If the view has not been populated yet, populate it and then
-        select the requested playlist. Otherwise, directly select the
-        requested playlist and start playing.
-
-        :param CoreModel klass: Main CoreModel
-        :param GParamObject val: value
+        """Selects the active playlist when an MPRIS client
+           has changed it.
         """
         playlist = self._coremodel.props.active_playlist
-        if playlist is None:
+        selection = self._sidebar.get_selected_row()
+        if (playlist is None
+                or playlist == selection.props.playlist):
             return
 
-        def _on_playlist_loaded(playlist):
-            playlist.disconnect(playlist_ready_id)
-            self._song_activated(None)
-
         playlist_row = None
         for row in self._sidebar:
             if row.props.playlist == playlist:
@@ -237,18 +229,8 @@ class PlaylistsView(BaseView):
         if not playlist_row:
             return
 
-        selection = self._sidebar.get_selected_row()
-        if selection.get_index() == playlist_row.get_index():
-            self._song_activated(None)
-            return
-
-        self._sidebar.select_row(row)
-        self._on_playlist_activated(self._sidebar, row)
-        if playlist.props.model.get_n_items() > 0:
-            self._song_activated(None)
-        else:
-            playlist_ready_id = playlist.connect(
-                "playlist-loaded", _on_playlist_loaded)
+        self._sidebar.select_row(playlist_row)
+        self._on_playlist_activated(self._sidebar, playlist_row)
 
     def _create_song_widget(self, coresong, playlist):
         can_dnd = not playlist.props.is_smart
@@ -269,16 +251,14 @@ class PlaylistsView(BaseView):
         signal_id = None
 
         def _on_playlist_loaded(klass, playlist_type):
+            self._coremodel.props.active_playlist = current_playlist
             self._player.play(coresong)
-            self._coremodel.handler_unblock(self._active_playlist_id)
             self._coremodel.disconnect(signal_id)
 
         selection = self._sidebar.get_selected_row()
         current_playlist = selection.props.playlist
         signal_id = self._coremodel.connect(
             "playlist-loaded", _on_playlist_loaded)
-        self._coremodel.handler_block(self._active_playlist_id)
-        self._coremodel.props.active_playlist = current_playlist
         self._coremodel.set_player_model(
             PlayerPlaylist.Type.PLAYLIST, current_playlist.props.model)