9 files changed, 571 insertions, 126 deletions

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 280e89a..f2c6cec 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -60,13 +60,32 @@ vala_add(indicator-sound-service
     mpris2-interfaces
 )
 vala_add(indicator-sound-service
+  media-player-user.vala
+  DEPENDS
+    media-player
+	accounts-service-sound-settings
+)
+vala_add(indicator-sound-service
   media-player-list.vala
   DEPENDS
     media-player
+)
+vala_add(indicator-sound-service
+  media-player-list-mpris.vala
+  DEPENDS
+    media-player-list
+    media-player
     media-player-mpris
     mpris2-interfaces
 )
 vala_add(indicator-sound-service
+  media-player-list-greeter.vala
+  DEPENDS
+    media-player-list
+    media-player-user
+    media-player
+)
+vala_add(indicator-sound-service
   mpris2-interfaces.vala
 )
 vala_add(indicator-sound-service
@@ -76,7 +95,6 @@ vala_add(indicator-sound-service
   sound-menu.vala
   DEPENDS
     media-player
-    mpris2-interfaces
 )
 vala_add(indicator-sound-service
   accounts-service-user.vala
diff --git a/src/accounts-service-user.vala b/src/accounts-service-user.vala
index f021764..052c7a0 100644
--- a/src/accounts-service-user.vala
+++ b/src/accounts-service-user.vala
@@ -101,6 +101,11 @@ public class AccountsServiceUser : Object {
 
 	~AccountsServiceUser () {
 		this.player = null;
+
+		if (this.timer != 0) {
+			GLib.Source.remove(this.timer);
+			this.timer = 0;
+		}
 	}
 
 	void new_proxy (GLib.Object? obj, AsyncResult res) {
diff --git a/src/main.c b/src/main.c
index f8635c8..e9a148e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -21,9 +21,18 @@ main (int argc, char ** argv) {
 	/* Initialize libnotify */
 	notify_init ("indicator-sound");
 
+	MediaPlayerList * playerlist = NULL;
 
-	service = indicator_sound_service_new ();
+	if (g_strcmp0("lightdm", g_get_user_name()) == 0) {
+		playerlist = MEDIA_PLAYER_LIST(media_player_list_greeter_new());
+	} else {
+		playerlist = MEDIA_PLAYER_LIST(media_player_list_mpris_new());
+	}
+
+	service = indicator_sound_service_new (playerlist);
 	result = indicator_sound_service_run (service);
+
+	g_object_unref(playerlist);
 	g_object_unref(service);
 
 	return result;
diff --git a/src/media-player-list-greeter.vala b/src/media-player-list-greeter.vala
new file mode 100644
index 0000000..15e4c55
--- /dev/null
+++ b/src/media-player-list-greeter.vala
@@ -0,0 +1,123 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Ted Gould <ted@canonical.com>
+ */
+
+[DBus (name="com.canonical.UnityGreeter.List")]
+public interface UnityGreeterList : Object {
+	public abstract async string get_active_entry () throws IOError;
+	public signal void entry_selected (string entry_name);
+}
+
+public class MediaPlayerListGreeter : MediaPlayerList {
+	string? selected_user = null;
+	UnityGreeterList? proxy = null;
+	HashTable<string, MediaPlayerUser> players = new HashTable<string, MediaPlayerUser>(str_hash, str_equal);
+
+	public MediaPlayerListGreeter () {
+		Bus.get_proxy.begin<UnityGreeterList> (
+			BusType.SESSION,
+			"com.canonical.Unity",
+			"/list",
+			DBusProxyFlags.NONE,
+			null,
+			new_proxy);
+	}
+
+	void new_proxy (GLib.Object? obj, AsyncResult res) {
+		try {
+			this.proxy = Bus.get_proxy.end(res);
+
+			this.proxy.entry_selected.connect(active_user_changed);
+			this.proxy.get_active_entry.begin ((obj, res) => {
+				try {
+					var value = (obj as UnityGreeterList).get_active_entry.end(res);
+					active_user_changed(value);
+				} catch (Error e) {
+					warning("Unable to get active entry: %s", e.message);
+				}
+			});
+		} catch (Error e) {
+			this.proxy = null;
+			warning("Unable to create proxy to the greeter: %s", e.message);
+		}
+	}
+
+	void active_user_changed (string active_user) {
+		/* No change, move along */
+		if (selected_user == active_user) {
+			return;
+		}
+
+		debug(@"Active user changed to: $active_user");
+
+		var old_user = selected_user;
+
+		/* Protect against a null user */
+		if (active_user != "" && active_user[0] != '*') {
+			selected_user = active_user;
+		} else {
+			debug(@"Blocking active user change for '$active_user'");
+			selected_user = null;
+		}
+
+		if (selected_user != null && !players.contains(selected_user)) {
+			players.insert(selected_user, new MediaPlayerUser(selected_user));
+		}
+
+		if (old_user != null) {
+			var old_player = players.lookup(old_user);
+			debug("Removing player for user: %s", old_user);
+			player_removed(old_player);
+		}
+
+		if (selected_user != null) {
+			var new_player = players.lookup(selected_user);
+
+			if (new_player != null) {
+				debug("Adding player for user: %s", selected_user);
+				player_added(new_player);
+			}
+		}
+	}
+
+	/* We need to have an iterator for the interface, but eh, we can
+	   only ever have one player for the current user */
+	public class Iterator : MediaPlayerList.Iterator {
+		int i = 0;
+		MediaPlayerListGreeter list;
+
+		public Iterator (MediaPlayerListGreeter in_list) {
+			list = in_list;
+		}
+
+		public override MediaPlayer? next_value () {
+			MediaPlayer? retval = null;
+
+			if (i == 0) {
+				retval = list.players.lookup(list.selected_user);
+			}
+			i++;
+
+			return retval;
+		}
+	}
+
+	public override MediaPlayerList.Iterator iterator() {
+		return new Iterator(this) as MediaPlayerList.Iterator;
+	}
+}
diff --git a/src/media-player-list-mpris.vala b/src/media-player-list-mpris.vala
new file mode 100644
index 0000000..65fb886
--- /dev/null
+++ b/src/media-player-list-mpris.vala
@@ -0,0 +1,135 @@
+/*
+ * Copyright 2013 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Lars Uebernickel <lars.uebernickel@canonical.com>
+ */
+
+/**
+ * MediaPlayerList is a list of media players that should appear in the sound menu.  Its main responsibility is
+ * to listen for MPRIS players on the bus and attach them to the corresponding %Player objects.
+ */
+public class MediaPlayerListMpris : MediaPlayerList {
+
+	public MediaPlayerListMpris () {
+		this._players = new HashTable<string, MediaPlayerMpris> (str_hash, str_equal);
+
+		BusWatcher.watch_namespace (BusType.SESSION, "org.mpris.MediaPlayer2", this.player_appeared, this.player_disappeared);
+	}
+
+	/* only valid while the list is not changed */
+	public class Iterator : MediaPlayerList.Iterator {
+		HashTableIter<string, MediaPlayerMpris> iter;
+
+		public Iterator (MediaPlayerListMpris list) {
+			this.iter = HashTableIter<string, MediaPlayerMpris> (list._players);
+		}
+
+		public override MediaPlayer? next_value () {
+			MediaPlayerMpris? player;
+
+			if (this.iter.next (null, out player))
+				return player as MediaPlayer;
+			else
+				return null;
+		}
+	}
+
+	public override MediaPlayerList.Iterator iterator () {
+		return new Iterator (this) as MediaPlayerList.Iterator;
+	}
+
+	/**
+	 * Adds the player associated with @desktop_id.  Does nothing if such a player already exists.
+	 */
+	MediaPlayerMpris? insert (string desktop_id) {
+		debug("Inserting player: %s", desktop_id);
+
+		var id = desktop_id.has_suffix (".desktop") ? desktop_id : desktop_id + ".desktop";
+		MediaPlayerMpris? player = this._players.lookup (id);
+
+		if (player == null) {
+			var appinfo = new DesktopAppInfo (id);
+			if (appinfo == null) {
+				warning ("unable to find application '%s'", id);
+				return null;
+			}
+
+			player = new MediaPlayerMpris (appinfo);
+			this._players.insert (player.id, player);
+			this.player_added (player);
+		}
+
+		return player;
+	}
+
+	/**
+	 * Removes the player associated with @desktop_id, unless it is currently running.
+	 */
+	void remove (string desktop_id) {
+		MediaPlayer? player = this._players.lookup (desktop_id);
+
+		if (player != null && !player.is_running) {
+			this._players.remove (desktop_id);
+			this.player_removed (player);
+		}
+	}
+
+	/**
+	 * Synchronizes the player list with @desktop_ids.  After this call, this list will only contain the players
+	 * in @desktop_ids.  Players that were running but are not in @desktop_ids will remain in the list.
+	 */
+	public override void sync (string[] desktop_ids) {
+
+		/* hash desktop_ids for faster lookup */
+		var hash = new HashTable<string, unowned string> (str_hash, str_equal);
+		foreach (var id in desktop_ids)
+			hash.add (id);
+
+		/* remove players that are not desktop_ids */
+		foreach (var id in this._players.get_keys ()) {
+			if (!hash.contains (id))
+				this.remove (id);
+		}
+
+		/* insert all players (insert() takes care of not adding a player twice */
+		foreach (var id in desktop_ids)
+			this.insert (id);
+	}
+
+	HashTable<string, MediaPlayerMpris> _players;
+
+	void player_appeared (DBusConnection connection, string name, string owner) {
+		try {
+			MprisRoot mpris2_root = Bus.get_proxy_sync (BusType.SESSION, name, MPRIS_MEDIA_PLAYER_PATH);
+
+			var player = this.insert (mpris2_root.DesktopEntry);
+			if (player != null)
+				player.attach (mpris2_root, name);
+		}
+		catch (Error e) {
+			warning ("unable to create mpris proxy for '%s': %s", name, e.message);
+		}
+	}
+
+	void player_disappeared (DBusConnection connection, string dbus_name) {
+		MediaPlayerMpris? player = this._players.find ( (name, player) => {
+			return player.dbus_name == dbus_name;
+		});
+
+		if (player != null)
+			player.detach ();
+	}
+}
diff --git a/src/media-player-list.vala b/src/media-player-list.vala
index 87ca1f0..fadbf63 100644
--- a/src/media-player-list.vala
+++ b/src/media-player-list.vala
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 Canonical Ltd.
+ * Copyright © 2014 Canonical Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -14,123 +14,20 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authors:
- *      Lars Uebernickel <lars.uebernickel@canonical.com>
+ *      Ted Gould <ted@canonical.com>
  */
 
-/**
- * MediaPlayerList is a list of media players that should appear in the sound menu.  Its main responsibility is
- * to listen for MPRIS players on the bus and attach them to the corresponding %Player objects.
- */
 public class MediaPlayerList {
-
-	public MediaPlayerList () {
-		this._players = new HashTable<string, MediaPlayerMpris> (str_hash, str_equal);
-
-		BusWatcher.watch_namespace (BusType.SESSION, "org.mpris.MediaPlayer2", this.player_appeared, this.player_disappeared);
-	}
-
-	/* only valid while the list is not changed */
 	public class Iterator {
-		HashTableIter<string, MediaPlayerMpris> iter;
-
-		public Iterator (MediaPlayerList list) {
-			this.iter = HashTableIter<string, MediaPlayerMpris> (list._players);
-		}
-
-		public MediaPlayer? next_value () {
-			MediaPlayerMpris? player;
-
-			if (this.iter.next (null, out player))
-				return player as MediaPlayer;
-			else
-				return null;
-		}
-	}
-
-	public Iterator iterator () {
-		return new Iterator (this);
-	}
-
-	/**
-	 * Adds the player associated with @desktop_id.  Does nothing if such a player already exists.
-	 */
-	MediaPlayerMpris? insert (string desktop_id) {
-		var id = desktop_id.has_suffix (".desktop") ? desktop_id : desktop_id + ".desktop";
-		MediaPlayerMpris? player = this._players.lookup (id);
-
-		if (player == null) {
-			var appinfo = new DesktopAppInfo (id);
-			if (appinfo == null) {
-				warning ("unable to find application '%s'", id);
-				return null;
-			}
-
-			player = new MediaPlayerMpris (appinfo);
-			this._players.insert (player.id, player);
-			this.player_added (player);
-		}
-
-		return player;
-	}
-
-	/**
-	 * Removes the player associated with @desktop_id, unless it is currently running.
-	 */
-	void remove (string desktop_id) {
-		MediaPlayer? player = this._players.lookup (desktop_id);
-
-		if (player != null && !player.is_running) {
-			this._players.remove (desktop_id);
-			this.player_removed (player);
+		public virtual MediaPlayer? next_value() {
+			return null;
 		}
 	}
+	public virtual Iterator iterator () { return new Iterator(); }
 
-	/**
-	 * Synchronizes the player list with @desktop_ids.  After this call, this list will only contain the players
-	 * in @desktop_ids.  Players that were running but are not in @desktop_ids will remain in the list.
-	 */
-	public void sync (string[] desktop_ids) {
-
-		/* hash desktop_ids for faster lookup */
-		var hash = new HashTable<string, unowned string> (str_hash, str_equal);
-		foreach (var id in desktop_ids)
-			hash.add (id);
-
-		/* remove players that are not desktop_ids */
-		foreach (var id in this._players.get_keys ()) {
-			if (!hash.contains (id))
-				this.remove (id);
-		}
-
-		/* insert all players (insert() takes care of not adding a player twice */
-		foreach (var id in desktop_ids)
-			this.insert (id);
-	}
+	public virtual void sync (string[] ids) { return; }
 
 	public signal void player_added (MediaPlayer player);
 	public signal void player_removed (MediaPlayer player);
-
-	HashTable<string, MediaPlayerMpris> _players;
-
-	void player_appeared (DBusConnection connection, string name, string owner) {
-		try {
-			MprisRoot mpris2_root = Bus.get_proxy_sync (BusType.SESSION, name, MPRIS_MEDIA_PLAYER_PATH);
-
-			var player = this.insert (mpris2_root.DesktopEntry);
-			if (player != null)
-				player.attach (mpris2_root, name);
-		}
-		catch (Error e) {
-			warning ("unable to create mpris proxy for '%s': %s", name, e.message);
-		}
-	}
-
-	void player_disappeared (DBusConnection connection, string dbus_name) {
-		MediaPlayerMpris? player = this._players.find ( (name, player) => {
-			return player.dbus_name == dbus_name;
-		});
-
-		if (player != null)
-			player.detach ();
-	}
 }
+
diff --git a/src/media-player-user.vala b/src/media-player-user.vala
new file mode 100644
index 0000000..dfe6229
--- /dev/null
+++ b/src/media-player-user.vala
@@ -0,0 +1,239 @@
+/*
+ * Copyright © 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *      Ted Gould <ted@canonical.com>
+ */
+
+public class MediaPlayerUser : MediaPlayer {
+	Act.UserManager accounts_manager = Act.UserManager.get_default();
+	string username;
+	Act.User? actuser = null;
+	AccountsServiceSoundSettings? proxy = null;
+
+	HashTable<string, bool> properties_queued = new HashTable<string, bool>(str_hash, str_equal);
+	uint properties_timeout = 0;
+
+	/* Grab the user from the Accounts service and, when it is loaded then
+	   set up a proxy to its sound settings */
+	public MediaPlayerUser(string user) {
+		username = user;
+
+		actuser = accounts_manager.get_user(user);
+		actuser.notify["is-loaded"].connect(() => {
+			debug("User loaded");
+
+			this.proxy = null;
+
+			Bus.get_proxy.begin<AccountsServiceSoundSettings> (
+				BusType.SYSTEM,
+				"org.freedesktop.Accounts",
+				actuser.get_object_path(),
+				DBusProxyFlags.GET_INVALIDATED_PROPERTIES,
+				null,
+				new_proxy);
+		});
+	}
+
+	~MediaPlayerUser () {
+		if (properties_timeout != 0) {
+			Source.remove(properties_timeout);
+			properties_timeout = 0;
+		}
+	}
+
+	/* Ensure that we've collected all the changes so that we only signal
+	   once for variables like 'track' */
+	bool properties_idle () {
+		properties_timeout = 0;
+
+		properties_queued.@foreach((key, value) => {
+			debug("Notifying '%s' changed", key);
+			this.notify_property(key);
+		});
+
+		properties_queued.remove_all();
+
+		/* Remove source */
+		return false;
+	}
+
+	/* Turns the DBus names into the object properties */
+	void queue_property_notification (string dbus_property_name) {
+		if (properties_timeout == 0) {
+			properties_timeout = Idle.add(properties_idle);
+		}
+
+		switch (dbus_property_name) {
+		case "Timestamp":
+			properties_queued.insert("name", true);
+			properties_queued.insert("icon", true);
+			properties_queued.insert("state", true);
+			properties_queued.insert("current-track", true);
+			properties_queued.insert("is-running", true);
+			break;
+		case "PlayerName":
+			properties_queued.insert("name", true);
+			break;
+		case "PlayerIcon":
+			properties_queued.insert("icon", true);
+			break;
+		case "State":
+			properties_queued.insert("state", true);
+			break;
+		case "Title":
+		case "Artist":
+		case "Album":
+		case "ArtUrl":
+			properties_queued.insert("current-track", true);
+			break;
+		}
+	}
+
+	void new_proxy (GLib.Object? obj, AsyncResult res) {
+		try {
+			this.proxy = Bus.get_proxy.end (res);
+
+			var gproxy = this.proxy as DBusProxy;
+			gproxy.g_properties_changed.connect ((proxy, changed, invalidated) => {
+				string key = "";
+				Variant value;
+				VariantIter iter = new VariantIter(changed);
+
+				while (iter.next("{sv}", &key, &value)) {
+					queue_property_notification(key);
+				}
+
+				foreach (var invalid in invalidated) {
+					queue_property_notification(invalid);
+				}
+			});
+
+			debug("Notifying player is ready for user: %s", this.username);
+			this.notify_property("is-running");
+		} catch (Error e) {
+			this.proxy = null;
+			warning("Unable to get proxy to user '%s' sound settings: %s", username, e.message);
+		}
+	}
+
+	bool proxy_is_valid () {
+		if (this.proxy == null) {
+			return false;
+		}
+
+		/* More than 10 minutes old */
+		if (this.proxy.timestamp < GLib.get_monotonic_time() - 10 * 60 * 1000 * 1000) {
+			return false;
+		}
+
+		return true;
+	}
+
+	public override string id {
+		get { return username; }
+	}
+
+	/* These values come from the proxy */
+	string name_cache;
+	public override string name { 
+		get {
+			if (proxy_is_valid()) {
+				name_cache = this.proxy.player_name;
+				return name_cache;
+			} else {
+				return "";
+			}
+		}
+	}
+	string state_cache;
+	public override string state {
+		get {
+			if (proxy_is_valid()) {
+				state_cache = this.proxy.state;
+				return state_cache;
+			} else {
+				return "";
+			}
+		}
+		set { }
+	}
+	Icon icon_cache;
+	public override Icon? icon { 
+		get { 
+			if (proxy_is_valid()) {
+				icon_cache = Icon.deserialize(this.proxy.player_icon);
+				return icon_cache;
+			} else {
+				return null;
+			}
+		}
+	}
+
+	/* Placeholder */
+	public override string dbus_name { get { return ""; } }
+
+	/* If it's shown externally it's running */
+	public override bool is_running { get { return proxy_is_valid(); } }
+	/* A bit weird.  Not sure how we should handle this. */
+	public override bool can_raise { get { return true; } }
+
+	/* Fill out the track based on the values in the proxy */
+	MediaPlayer.Track track_cache;
+	public override MediaPlayer.Track? current_track {
+		get { 
+			if (proxy_is_valid()) {
+				track_cache = new MediaPlayer.Track(
+					this.proxy.artist,
+					this.proxy.title,
+					this.proxy.album,
+					this.proxy.art_url
+				);
+				return track_cache;
+			} else {
+				return null;
+			}
+		}
+		set { }
+	}
+
+	/* Control functions through unity-greeter-session-broadcast */
+	public override void activate () {
+		/* TODO: */
+	}
+	public override void play_pause () {
+		/* TODO: */
+	}
+	public override void next () {
+		/* TODO: */
+	}
+	public override void previous () {
+		/* TODO: */
+	}
+
+	/* Play list functions are all null as we don't support the
+	   playlist feature on the greeter */
+	public override uint get_n_playlists() {
+		return 0;
+	}
+	public override string get_playlist_id (int index) {
+		return "";
+	}
+	public override string get_playlist_name (int index) {
+		return "";
+	}
+	public override void activate_playlist_by_name (string playlist) {
+	}
+}
diff --git a/src/service.vala b/src/service.vala
index 6835896..793a91a 100644
--- a/src/service.vala
+++ b/src/service.vala
@@ -18,7 +18,7 @@
  */
 
 public class IndicatorSound.Service: Object {
-	public Service () {
+	public Service (MediaPlayerList playerlist) {
 		this.settings = new Settings ("com.canonical.indicator.sound");
 		this.sharedsettings = new Settings ("com.ubuntu.sound");
 
@@ -27,7 +27,7 @@ public class IndicatorSound.Service: Object {
 
 		this.volume_control = new VolumeControl ();
 
-		this.players = new MediaPlayerList ();
+		this.players = playerlist;
 		this.players.player_added.connect (this.player_added);
 		this.players.player_removed.connect (this.player_removed);
 
@@ -39,6 +39,7 @@ public class IndicatorSound.Service: Object {
 
 		this.menus = new HashTable<string, SoundMenu> (str_hash, str_equal);
 		this.menus.insert ("desktop_greeter", new SoundMenu (null, SoundMenu.DisplayFlags.SHOW_MUTE | SoundMenu.DisplayFlags.HIDE_PLAYERS));
+		this.menus.insert ("phone_greeter", new SoundMenu (null, SoundMenu.DisplayFlags.SHOW_MUTE | SoundMenu.DisplayFlags.HIDE_INACTIVE_PLAYERS));
 		this.menus.insert ("desktop", new SoundMenu ("indicator.desktop-settings", SoundMenu.DisplayFlags.SHOW_MUTE));
 		this.menus.insert ("phone", new SoundMenu ("indicator.phone-settings", SoundMenu.DisplayFlags.HIDE_INACTIVE_PLAYERS));
 
@@ -371,15 +372,15 @@ public class IndicatorSound.Service: Object {
 		this.menus.@foreach ( (profile, menu) => menu.add_player (player));
 
 		SimpleAction action = new SimpleAction.stateful (player.id, null, this.action_state_for_player (player));
+		action.set_enabled (player.can_raise);
 		action.activate.connect ( () => { player.activate (); });
 		this.actions.add_action (action);
 
 		var play_action = new SimpleAction.stateful ("play." + player.id, null, player.state);
 		play_action.activate.connect ( () => player.play_pause () );
 		this.actions.add_action (play_action);
-		player.notify.connect ( (object, pspec) => {
-			if (pspec.name == "state")
-				play_action.set_state (player.state);
+		player.notify["state"].connect ( (object, pspec) => {
+			play_action.set_state (player.state);
 		});
 
 		var next_action = new SimpleAction ("next." + player.id, null);
@@ -406,6 +407,8 @@ public class IndicatorSound.Service: Object {
 		this.actions.remove_action ("previous." + player.id);
 		this.actions.remove_action ("play-playlist." + player.id);
 
+		player.notify.disconnect (this.eventually_update_player_actions);
+
 		this.menus.@foreach ( (profile, menu) => menu.remove_player (player));
 
 		this.update_preferred_players ();
diff --git a/src/sound-menu.vala b/src/sound-menu.vala
index 3fdfc36..e37c4e9 100644
--- a/src/sound-menu.vala
+++ b/src/sound-menu.vala
@@ -17,7 +17,7 @@
  *      Lars Uebernickel <lars.uebernickel@canonical.com>
  */
 
-class SoundMenu: Object
+public class SoundMenu: Object
 {
 	public enum DisplayFlags {
 		NONE = 0,
@@ -97,12 +97,13 @@ class SoundMenu: Object
 		this.update_playlists (player);
 
 		var handler_id = player.notify["is-running"].connect ( () => {
-			if (this.hide_inactive) {
-				if (player.is_running)
+			if (player.is_running)
+				if (this.find_player_section(player) == -1)
 					this.insert_player_section (player);
-				else
+			else
+				if (this.hide_inactive)
 					this.remove_player_section (player);
-			}
+
 			this.update_playlists (player);
 		});
 		this.notify_handlers.insert (player, handler_id);
@@ -112,11 +113,20 @@ class SoundMenu: Object
 
 	public void remove_player (MediaPlayer player) {
 		this.remove_player_section (player);
+
+		var id = this.notify_handlers.lookup(player);
+		if (id != 0) {
+			player.disconnect(id);
+		}
+
+		player.playlists_changed.disconnect (this.update_playlists);
+
+		/* this'll drop our ref to it */
 		this.notify_handlers.remove (player);
 	}
 
-	Menu root;
-	Menu menu;
+	public Menu root;
+	public Menu menu;
 	Menu volume_section;
 	bool mic_volume_shown;
 	bool settings_shown = false;
@@ -126,16 +136,20 @@ class SoundMenu: Object
 
 	/* returns the position in this.menu of the section that's associated with @player */
 	int find_player_section (MediaPlayer player) {
+		debug("Looking for player: %s", player.id);
 		string action_name = @"indicator.$(player.id)";
-		int n = this.menu.get_n_items () -1;
-		for (int i = 1; i < n; i++) {
+		int n = this.menu.get_n_items ();
+		for (int i = 0; i < n; i++) {
 			var section = this.menu.get_item_link (i, Menu.LINK_SECTION);
+			if (section == null) continue;
+
 			string action;
 			section.get_item_attribute (0, "action", "s", out action);
 			if (action == action_name)
 				return i;
 		}
 
+		debug("Unable to find section for player: %s", player.id);
 		return -1;
 	}
 
@@ -146,6 +160,8 @@ class SoundMenu: Object
 		var section = new Menu ();
 		Icon icon;
 
+		debug("Adding section for player: %s (%s)", player.id, player.is_running ? "running" : "not running");
+
 		icon = player.icon;
 		if (icon == null)
 			icon = new ThemedIcon.with_default_fallbacks ("application-default-icon");