Create players on the phone greeter based on the accounts service data

14 files changed, 948 insertions, 130 deletions

diff --git a/data/com.canonical.indicator.sound b/data/com.canonical.indicator.sound
index fca15be..ae31770 100644
--- a/data/com.canonical.indicator.sound
+++ b/data/com.canonical.indicator.sound
@@ -19,5 +19,5 @@ ObjectPath=/com/canonical/indicator/sound/desktop_greeter
 ObjectPath=/com/canonical/indicator/sound/desktop_greeter
 
 [phone_greeter]
-ObjectPath=/com/canonical/indicator/sound/desktop_greeter
+ObjectPath=/com/canonical/indicator/sound/phone_greeter
 
diff --git a/debian/control b/debian/control
index 4278f7a..f10d9b0 100644
--- a/debian/control
+++ b/debian/control
@@ -13,7 +13,7 @@ Build-Depends: debhelper (>= 9.0),
                autotools-dev,
                valac (>= 0.20),
                libaccountsservice-dev,
-               libdbustest1-dev,
+               libdbustest1-dev (>= 14.04.1),
                libgirepository1.0-dev,
                libglib2.0-dev (>= 2.22.3),
                libgtest-dev,
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/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 f6c5f01..0e32bfe 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));
 
@@ -441,15 +442,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);
@@ -476,6 +477,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");
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 1556fc7..9b0586a 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -114,3 +114,47 @@ add_test(accounts-service-user-test-basic
 add_test(accounts-service-user-test-player
 	accounts-service-user-test  --gtest_filter=AccountsServiceUserTest.SetMediaPlayer
 )
+
+###########################
+# Sound Menu
+###########################
+
+include_directories(${CMAKE_SOURCE_DIR}/src)
+add_executable (sound-menu-test sound-menu.cc)
+target_link_libraries (
+	sound-menu-test
+	indicator-sound-service-lib
+	vala-mocks-lib
+	gtest
+	${SOUNDSERVICE_LIBRARIES}
+	${TEST_LIBRARIES}
+)
+
+add_test(sound-menu-test sound-menu-test)
+
+###########################
+# Accounts Service User
+###########################
+
+include_directories(${CMAKE_SOURCE_DIR}/src)
+add_executable (media-player-user-test media-player-user.cc)
+target_link_libraries (
+	media-player-user-test
+	indicator-sound-service-lib
+	vala-mocks-lib
+	gtest
+	${SOUNDSERVICE_LIBRARIES}
+	${TEST_LIBRARIES}
+)
+
+# Split tests to work around libaccountservice sucking
+add_test(media-player-user-test-basic
+	media-player-user-test  --gtest_filter=MediaPlayerUserTest.BasicObject
+)
+add_test(media-player-user-test-dataset
+	media-player-user-test  --gtest_filter=MediaPlayerUserTest.DataSet
+)
+add_test(media-player-user-test-timeout
+	media-player-user-test  --gtest_filter=MediaPlayerUserTest.TimeoutTest
+)
+
diff --git a/tests/accounts-service-mock.h b/tests/accounts-service-mock.h
index 225d7b5..d4dae7e 100644
--- a/tests/accounts-service-mock.h
+++ b/tests/accounts-service-mock.h
@@ -22,6 +22,8 @@
 class AccountsServiceMock
 {
 		DbusTestDbusMock * mock = nullptr;
+		DbusTestDbusMockObject * soundobj = nullptr;
+		DbusTestDbusMockObject * userobj = nullptr;
 
 	public:
 		AccountsServiceMock () {
@@ -45,12 +47,12 @@ class AccountsServiceMock
 				"UncacheUser", G_VARIANT_TYPE_STRING, NULL,
 				"", NULL);
 
-			DbusTestDbusMockObject * userobj = dbus_test_dbus_mock_get_object(mock, "/user", "org.freedesktop.Accounts.User", NULL);
+			userobj = dbus_test_dbus_mock_get_object(mock, "/user", "org.freedesktop.Accounts.User", NULL);
 			dbus_test_dbus_mock_object_add_property(mock, userobj,
 				"UserName", G_VARIANT_TYPE_STRING,
 				g_variant_new_string(g_get_user_name()), NULL);
 
-			DbusTestDbusMockObject * soundobj = dbus_test_dbus_mock_get_object(mock, "/user", "com.canonical.indicator.sound.AccountsService", NULL);
+			soundobj = dbus_test_dbus_mock_get_object(mock, "/user", "com.canonical.indicator.sound.AccountsService", NULL);
 			dbus_test_dbus_mock_object_add_property(mock, soundobj,
 				"Timestamp", G_VARIANT_TYPE_UINT64,
 				g_variant_new_uint64(0), NULL);
@@ -81,10 +83,19 @@ class AccountsServiceMock
 		}
 
 		~AccountsServiceMock () {
+			g_debug("Destroying the Accounts Service Mock");
 			g_clear_object(&mock);
 		}
 
 		operator DbusTestTask* () {
 			return DBUS_TEST_TASK(mock);
 		}
+
+		operator DbusTestDbusMock* () {
+			return mock;
+		}
+
+		DbusTestDbusMockObject * get_sound () {
+			return soundobj;
+		}
 };
diff --git a/tests/media-player-user.cc b/tests/media-player-user.cc
new file mode 100644
index 0000000..2132e14
--- /dev/null
+++ b/tests/media-player-user.cc
@@ -0,0 +1,210 @@
+/*
+ * 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>
+ */
+
+#include <gtest/gtest.h>
+#include <gio/gio.h>
+#include <libdbustest/dbus-test.h>
+
+#include "accounts-service-mock.h"
+
+extern "C" {
+#include "indicator-sound-service.h"
+}
+
+class MediaPlayerUserTest : public ::testing::Test
+{
+
+	protected:
+		DbusTestService * service = NULL;
+		AccountsServiceMock service_mock;
+
+		GDBusConnection * session = NULL;
+		GDBusConnection * system = NULL;
+		GDBusProxy * proxy = NULL;
+
+		virtual void SetUp() {
+			service = dbus_test_service_new(NULL);
+
+
+			dbus_test_service_add_task(service, (DbusTestTask*)service_mock);
+			dbus_test_service_start_tasks(service);
+
+			g_setenv("DBUS_SYSTEM_BUS_ADDRESS", g_getenv("DBUS_SESSION_BUS_ADDRESS"), TRUE);
+
+			session = g_bus_get_sync(G_BUS_TYPE_SESSION, NULL, NULL);
+			ASSERT_NE(nullptr, session);
+			g_dbus_connection_set_exit_on_close(session, FALSE);
+			g_object_add_weak_pointer(G_OBJECT(session), (gpointer *)&session);
+
+			system = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, NULL);
+			ASSERT_NE(nullptr, system);
+			g_dbus_connection_set_exit_on_close(system, FALSE);
+			g_object_add_weak_pointer(G_OBJECT(system), (gpointer *)&system);
+
+			proxy = g_dbus_proxy_new_sync(session,
+				G_DBUS_PROXY_FLAGS_NONE,
+				NULL,
+				"org.freedesktop.Accounts",
+				"/user",
+				"org.freedesktop.DBus.Properties",
+				NULL, NULL);
+			ASSERT_NE(nullptr, proxy);
+		}
+
+		virtual void TearDown() {
+			g_clear_object(&proxy);
+			g_clear_object(&service);
+
+			g_object_unref(session);
+			g_object_unref(system);
+
+			#if 0
+			/* Accounts Service keeps a bunch of references around so we
+			   have to split the tests and can't check this :-( */
+			unsigned int cleartry = 0;
+			while ((session != NULL || system != NULL) && cleartry < 100) {
+				loop(100);
+				cleartry++;
+			}
+
+			ASSERT_EQ(nullptr, session);
+			ASSERT_EQ(nullptr, system);
+			#endif
+		}
+
+		static gboolean timeout_cb (gpointer user_data) {
+			GMainLoop * loop = static_cast<GMainLoop *>(user_data);
+			g_main_loop_quit(loop);
+			return G_SOURCE_REMOVE;
+		}
+
+		void loop (unsigned int ms) {
+			GMainLoop * loop = g_main_loop_new(NULL, FALSE);
+			g_timeout_add(ms, timeout_cb, loop);
+			g_main_loop_run(loop);
+			g_main_loop_unref(loop);
+		}
+
+		void set_property (const gchar * name, GVariant * value) {
+			dbus_test_dbus_mock_object_update_property((DbusTestDbusMock *)service_mock, service_mock.get_sound(), name, value, NULL);
+		}
+};
+
+TEST_F(MediaPlayerUserTest, BasicObject) {
+	MediaPlayerUser * player = media_player_user_new("user");
+	ASSERT_NE(nullptr, player);
+
+	/* Protected, but no useful data */
+	EXPECT_FALSE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	EXPECT_TRUE(media_player_get_can_raise(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("user", media_player_get_id(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("", media_player_get_name(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("", media_player_get_state(MEDIA_PLAYER(player)));
+	EXPECT_EQ(nullptr, media_player_get_icon(MEDIA_PLAYER(player)));
+	EXPECT_EQ(nullptr, media_player_get_current_track(MEDIA_PLAYER(player)));
+
+	/* Get the proxy -- but no good data */
+	loop(100);
+
+	/* Ensure even with the proxy we don't have anything */
+	EXPECT_FALSE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	EXPECT_TRUE(media_player_get_can_raise(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("user", media_player_get_id(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("", media_player_get_name(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("", media_player_get_state(MEDIA_PLAYER(player)));
+	EXPECT_EQ(nullptr, media_player_get_icon(MEDIA_PLAYER(player)));
+	EXPECT_EQ(nullptr, media_player_get_current_track(MEDIA_PLAYER(player)));
+
+	g_clear_object(&player);
+}
+
+TEST_F(MediaPlayerUserTest, DataSet) {
+	/* Put data into Acts */
+	set_property("Timestamp", g_variant_new_uint64(g_get_monotonic_time()));
+	set_property("PlayerName", g_variant_new_string("The Player Formerly Known as Prince"));
+	GIcon * in_icon = g_themed_icon_new_with_default_fallbacks("foo-bar-fallback");
+	set_property("PlayerIcon", g_variant_new_variant(g_icon_serialize(in_icon)));
+	set_property("State", g_variant_new_string("Chillin'"));
+	set_property("Title", g_variant_new_string("Dictator"));
+	set_property("Artist", g_variant_new_string("Bansky"));
+	set_property("Album", g_variant_new_string("Vinyl is dead"));
+	set_property("ArtUrl", g_variant_new_string("http://art.url"));
+
+	/* Build our media player */
+	MediaPlayerUser * player = media_player_user_new("user");
+	ASSERT_NE(nullptr, player);
+
+	/* Get the proxy -- and it's precious precious data -- oh, my, precious! */
+	loop(100);
+
+	/* Ensure even with the proxy we don't have anything */
+	EXPECT_TRUE(media_player_get_is_running(MEDIA_PLAYER(player)));
+	EXPECT_TRUE(media_player_get_can_raise(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("user", media_player_get_id(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("The Player Formerly Known as Prince", media_player_get_name(MEDIA_PLAYER(player)));
+	EXPECT_STREQ("Chillin'", media_player_get_state(MEDIA_PLAYER(player)));
+
+	GIcon * out_icon = media_player_get_icon(MEDIA_PLAYER(player));
+	EXPECT_NE(nullptr, out_icon);
+	EXPECT_TRUE(g_icon_equal(in_icon, out_icon));
+	g_clear_object(&out_icon);
+
+	MediaPlayerTrack * track = media_player_get_current_track(MEDIA_PLAYER(player));
+	EXPECT_NE(nullptr, track);
+	EXPECT_STREQ("Dictator", media_player_track_get_title(track));
+	EXPECT_STREQ("Bansky", media_player_track_get_artist(track));
+	EXPECT_STREQ("Vinyl is dead", media_player_track_get_album(track));
+	EXPECT_STREQ("http://art.url", media_player_track_get_art_url(track));
+	g_clear_object(&track);
+
+	g_clear_object(&in_icon);
+	g_clear_object(&player);
+}
+
+TEST_F(MediaPlayerUserTest, TimeoutTest) {
+	/* Put data into Acts -- but 15 minutes ago */
+	set_property("Timestamp", g_variant_new_uint64(g_get_monotonic_time() - 15 * 60 * 1000 * 1000));
+	set_property("PlayerName", g_variant_new_string("The Player Formerly Known as Prince"));
+	GIcon * in_icon = g_themed_icon_new_with_default_fallbacks("foo-bar-fallback");
+	set_property("PlayerIcon", g_variant_new_variant(g_icon_serialize(in_icon)));
+	set_property("State", g_variant_new_string("Chillin'"));
+	set_property("Title", g_variant_new_string("Dictator"));
+	set_property("Artist", g_variant_new_string("Bansky"));
+	set_property("Album", g_variant_new_string("Vinyl is dead"));
+	set_property("ArtUrl", g_variant_new_string("http://art.url"));
+
+	/* Build our media player */
+	MediaPlayerUser * player = media_player_user_new("user");
+	ASSERT_NE(nullptr, player);
+
+	/* Get the proxy -- and the old data, so old, like forever */
+	loop(100);
+
+	/* Ensure that we show up as not running */
+	EXPECT_FALSE(media_player_get_is_running(MEDIA_PLAYER(player)));
+
+	/* Update to make running */
+	set_property("Timestamp", g_variant_new_uint64(g_get_monotonic_time()));
+	loop(100);
+
+	EXPECT_TRUE(media_player_get_is_running(MEDIA_PLAYER(player)));
+
+	g_clear_object(&in_icon);
+	g_clear_object(&player);
+}
diff --git a/tests/sound-menu.cc b/tests/sound-menu.cc
new file mode 100644
index 0000000..10c0cb9
--- /dev/null
+++ b/tests/sound-menu.cc
@@ -0,0 +1,113 @@
+/*
+ * 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>
+ */
+
+#include <gtest/gtest.h>
+#include <gio/gio.h>
+
+extern "C" {
+#include "indicator-sound-service.h"
+#include "vala-mocks.h"
+}
+
+class SoundMenuTest : public ::testing::Test
+{
+	protected:
+		GTestDBus * bus = nullptr;
+
+		virtual void SetUp() {
+			bus = g_test_dbus_new(G_TEST_DBUS_NONE);
+			g_test_dbus_up(bus);
+		}
+
+		virtual void TearDown() {
+			g_test_dbus_down(bus);
+			g_clear_object(&bus);
+		}
+
+		void verify_item_attribute (GMenuModel * mm, guint index, const gchar * name, GVariant * value) {
+			g_variant_ref_sink(value);
+
+			gchar * variantstr = g_variant_print(value, TRUE);
+			g_debug("Expecting item %d to have a '%s' attribute: %s", index, name, variantstr);
+
+			const GVariantType * type = g_variant_get_type(value);
+			GVariant * itemval = g_menu_model_get_item_attribute_value(mm, index, name, type);
+
+			ASSERT_NE(nullptr, itemval);
+			EXPECT_TRUE(g_variant_equal(itemval, value));
+
+			g_variant_unref(value);
+		}
+};
+
+TEST_F(SoundMenuTest, BasicObject) {
+	SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
+
+	ASSERT_NE(nullptr, menu);
+
+	g_clear_object(&menu);
+	return;
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayer) {
+	SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
+
+	MediaPlayerTrack * track = media_player_track_new("Artist", "Title", "Album", "http://art.url");
+
+	MediaPlayerMock * media = MEDIA_PLAYER_MOCK(
+		g_object_new(TYPE_MEDIA_PLAYER_MOCK,
+			"mock-id", "player-id",
+			"mock-name", "Test Player",
+			"mock-state", "Playing",
+			"mock-is-running", TRUE,
+			"mock-can-raise", FALSE,
+			"mock-current-track", track,
+			NULL)
+	);
+	g_clear_object(&track);
+
+	sound_menu_add_player(menu, MEDIA_PLAYER(media));
+
+	ASSERT_NE(nullptr, menu->menu);
+	EXPECT_EQ(2, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
+
+	GMenuModel * section = g_menu_model_get_item_link(G_MENU_MODEL(menu->menu), 1, G_MENU_LINK_SECTION);
+	ASSERT_NE(nullptr, section);
+	EXPECT_EQ(2, g_menu_model_get_n_items(section)); /* No playlists, so two items */
+
+	/* Player display */
+	verify_item_attribute(section, 0, "action", g_variant_new_string("indicator.player-id"));
+	verify_item_attribute(section, 0, "x-canonical-type", g_variant_new_string("com.canonical.unity.media-player"));
+
+	/* Player control */
+	verify_item_attribute(section, 1, "x-canonical-type", g_variant_new_string("com.canonical.unity.playback-item"));
+	verify_item_attribute(section, 1, "x-canonical-play-action", g_variant_new_string("indicator.play.player-id"));
+	verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string("indicator.next.player-id"));
+	verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string("indicator.previous.player-id"));
+
+	g_clear_object(&section);
+
+	sound_menu_remove_player(menu, MEDIA_PLAYER(media));
+
+	EXPECT_EQ(1, g_menu_model_get_n_items(G_MENU_MODEL(menu->menu)));
+
+	g_clear_object(&media);
+	g_clear_object(&menu);
+	return;
+}