Merge lp:~xavi-garcia-mena/indicator-sound/next-play-prev-buttons-bug-1373313 into lp:indicator-sound/15.04

[Xavi Garcia]

Synchronize the state of the Previous/Play/Next buttons with the state obtained from MPRIS.

The state is updated every time the properties are modified.

Synchronize the state of the Previous/Play/Next buttons with the state obtained from MPRIS.

The state is updated every time the properties are modified.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:507
http://jenkins.qa.ubuntu.com/job/indicator-sound-15.04-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-15.04-vivid-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-sound-15.04-vivid-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/indicator-sound-15.04-vivid-armhf-ci/2/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-sound-15.04-ci/2/rebuild

review: Approve (continuous-integration)

[Xavi Garcia (xavi-garcia-mena)]

Approving as this code change was already reviewed and approved by Charles.
This MP is only for vivid.

review: Approve

[lp:~xavi-garcia-mena/indicator-sound/next-play-prev-buttons-bug-1373313]

508. By Xavi Garcia on 2015-10-13

Fixed version in changelog

509. By Xavi Garcia on 2015-10-14

Updating MPRIS control in the menu not only when next and previous are updated

=== modified file 'debian/changelog'
--- debian/changelog	2015-10-14 14:58:09 +0000
+++ debian/changelog	2015-10-14 14:58:09 +0000
@@ -1,3 +1,21 @@
+indicator-sound (12.10.2+15.04.20150917-0ubuntu1) vivid; urgency=medium
+
+  [ Xavi Garcia Mena ]
+  * Merged lp:~xavi-garcia-mena/indicator-sound/icon-volume-zero
+
+ -- Sebastien Bacher <[e-mail]>  Tue, 15 Sep 2015 08:19:51 +0000
+
+indicator-sound (12.10.2+15.04.20150915-0ubuntu1) vivid; urgency=medium
+
+  [ Sebastien Bacher ]
+  * under unity8 start system-settings instead unity-control-center (LP:
+    #1489427)
+
+  [ Xavi Garcia Mena ]
+  * Merged lp:~xavi-garcia-mena/indicator-sound/icon-volume-zero
+
+ -- Sebastien Bacher <[e-mail]>  Tue, 15 Sep 2015 08:19:51 +0000
+
 indicator-sound (12.10.2+15.04.20150812.3-0ubuntu1) vivid; urgency=medium
 
   [ CI Train Bot ]

=== modified file 'src/media-player-mpris.vala'
--- src/media-player-mpris.vala	2015-03-26 15:56:49 +0000
+++ src/media-player-mpris.vala	2015-10-14 14:58:09 +0000
@@ -79,6 +79,24 @@
 		}
 	}
 
+	public override bool can_do_play {
+		get {
+			return this.proxy.CanPlay;
+		}
+	}
+	
+	public override bool can_do_prev {
+		get {
+			return this.proxy.CanGoPrevious;
+		}
+	}
+
+	public override bool can_do_next {
+		get {
+			return this.proxy.CanGoNext;
+		}
+	}
+
 	/**
 	 * Attach this object to a process of the associated media player.  The player must own @dbus_name and
 	 * implement the org.mpris.MediaPlayer2.Player interface.
@@ -272,6 +290,10 @@
 		if (changed_properties.lookup ("PlaybackStatus", "s", null)) {
 			this.state = this.proxy.PlaybackStatus != null ? this.proxy.PlaybackStatus : "Unknown";
 		}
+		if (changed_properties.lookup ("CanGoNext", "b", null) || changed_properties.lookup ("CanGoPrevious", "b", null) ||
+                    changed_properties.lookup ("CanPlay", "b", null) || changed_properties.lookup ("CanPause", "b", null)) {
+			this.playbackstatus_changed ();
+		}
 
 		var metadata = changed_properties.lookup_value ("Metadata", new VariantType ("a{sv}"));
 		if (metadata != null)

=== modified file 'src/media-player.vala'
--- src/media-player.vala	2014-02-12 16:21:32 +0000
+++ src/media-player.vala	2015-10-14 14:58:09 +0000
@@ -26,6 +26,9 @@
 
 	public virtual bool is_running { get { not_implemented(); return false; } }
 	public virtual bool can_raise { get { not_implemented(); return false; } }
+	public virtual bool can_do_next { get { not_implemented(); return false; } }
+	public virtual bool can_do_prev { get { not_implemented(); return false; } }
+	public virtual bool can_do_play { get { not_implemented(); return false; } }
 
 	public class Track : Object {
 		public string artist { get; construct; }
@@ -44,6 +47,7 @@
 	}
 
 	public signal void playlists_changed ();
+	public signal void playbackstatus_changed ();
 
 	public abstract void activate ();
 	public abstract void play_pause ();

=== modified file 'src/mpris2-interfaces.vala'
--- src/mpris2-interfaces.vala	2015-03-26 15:56:49 +0000
+++ src/mpris2-interfaces.vala	2015-10-14 14:58:09 +0000
@@ -37,7 +37,10 @@
   // properties
   public abstract HashTable<string, Variant?> Metadata{owned get; set;}
   public abstract int32 Position{owned get; set;}
-  public abstract string? PlaybackStatus{owned get; set;}  
+  public abstract string? PlaybackStatus{owned get; set;} 
+  public abstract bool CanPlay{owned get; set;}
+  public abstract bool CanGoNext{owned get; set;}
+  public abstract bool CanGoPrevious{owned get; set;}
   // methods
   public abstract async void PlayPause() throws IOError;
   public abstract async void Next() throws IOError;

=== modified file 'src/sound-menu.vala'
--- src/sound-menu.vala	2015-02-12 00:44:34 +0000
+++ src/sound-menu.vala	2015-10-14 14:58:09 +0000
@@ -157,18 +157,26 @@
 		this.update_playlists (player);
 
 		var handler_id = player.notify["is-running"].connect ( () => {
-			if (player.is_running)
-				if (this.find_player_section(player) == -1)
+			if (player.is_running) {
+				int index = this.find_player_section(player);
+				if (index == -1) {
 					this.insert_player_section (player);
-			else
+				}
+				else {
+					update_player_section (player, index);
+				}
+			}
+			else {
 				if (this.hide_inactive)
 					this.remove_player_section (player);
+			}
 
 			this.update_playlists (player);
 		});
 		this.notify_handlers.insert (player, handler_id);
 
 		player.playlists_changed.connect (this.update_playlists);
+		player.playbackstatus_changed.connect (this.update_playbackstatus);
 	}
 
 	public void remove_player (MediaPlayer player) {
@@ -215,6 +223,23 @@
 		return -1;
 	}
 
+	MenuItem create_playback_menu_item (MediaPlayer player) {
+		var playback_item = new MenuItem (null, null);
+		playback_item.set_attribute ("x-canonical-type", "s", "com.canonical.unity.playback-item");
+		if (player.is_running) {
+			if (player.can_do_play) {
+				playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
+			}
+			if (player.can_do_next) {
+				playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
+			}
+			if (player.can_do_prev) {
+				playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
+			}
+		}
+		return playback_item;
+	}
+
 	void insert_player_section (MediaPlayer player) {
 		if (this.hide_players)
 			return;
@@ -240,9 +265,21 @@
 
 		var playback_item = new MenuItem (null, null);
 		playback_item.set_attribute ("x-canonical-type", "s", "com.canonical.unity.playback-item");
-		playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
-		playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
-		playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
+		playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id + ".disabled");
+		playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id + ".disabled");
+		playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id + ".disabled");
+
+		if (player.is_running) {
+			if (player.can_do_play) {
+				playback_item.set_attribute ("x-canonical-play-action", "s", "indicator.play." + player.id);
+			}
+			if (player.can_do_next) {
+				playback_item.set_attribute ("x-canonical-next-action", "s", "indicator.next." + player.id);
+			}
+			if (player.can_do_prev) {
+				playback_item.set_attribute ("x-canonical-previous-action", "s", "indicator.previous." + player.id);
+			}
+		}
 		section.append_item (playback_item);
 
 		/* Add new players to the end of the player sections, just before the settings */
@@ -262,6 +299,17 @@
 			this.menu.remove (index);
 	}
 
+	void update_player_section (MediaPlayer player, int index) {
+		var player_section = this.menu.get_item_link(index, Menu.LINK_SECTION) as Menu;
+		if (player_section.get_n_items () == 2) {
+			// we have 2 items, the second one is the playback item
+			// remove it first
+			player_section.remove (1);
+			MenuItem playback_item = create_playback_menu_item (player);
+			player_section.append_item (playback_item);
+		}
+	}
+
 	void update_playlists (MediaPlayer player) {
 		int index = find_player_section (player);
 		if (index < 0)
@@ -292,6 +340,13 @@
 		submenu.append_section (null, playlists_section);
 		player_section.append_submenu (_("Choose Playlist"), submenu);
 	}
+	
+	void update_playbackstatus (MediaPlayer player) {
+		int index = find_player_section (player);
+		if (index != -1) {
+			update_player_section (player, index);	
+		}
+	}
 
 	MenuItem create_slider_menu_item (string label, string action, double min, double max, double step, string min_icon_name, string max_icon_name) {
 		var min_icon = new ThemedIcon.with_default_fallbacks (min_icon_name);

=== modified file 'tests/media-player-mock.vala'
--- tests/media-player-mock.vala	2014-02-12 19:54:59 +0000
+++ tests/media-player-mock.vala	2015-10-14 14:58:09 +0000
@@ -28,6 +28,9 @@
 
 	public override bool is_running { get { return mock_is_running; } }
 	public override bool can_raise { get {  return mock_can_raise; } }
+	public override bool can_do_next { get { return mock_can_do_next; } }
+	public override bool can_do_prev { get { return mock_can_do_prev; } }
+	public override bool can_do_play { get { return mock_can_do_play; } }
 
 	public override MediaPlayer.Track? current_track { get { return mock_current_track; } set { this.mock_current_track = value; } }
 
@@ -40,6 +43,9 @@
 
 	public bool mock_is_running { get; set; }
 	public bool mock_can_raise { get; set; }
+	public bool mock_can_do_next { get; set; }
+	public bool mock_can_do_prev { get; set; }
+	public bool mock_can_do_play { get; set; }
 
 	public MediaPlayer.Track? mock_current_track { get; set; } 
 

=== modified file 'tests/sound-menu.cc'
--- tests/sound-menu.cc	2014-03-03 21:38:38 +0000
+++ tests/sound-menu.cc	2015-10-14 14:58:09 +0000
@@ -27,87 +27,163 @@
 
 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);
-		}
+    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);
+        }
+
+        void verify_item_attribute_is_not_set(GMenuModel * mm, guint index, const gchar * name, const GVariantType * type) {
+            GVariant * itemval = g_menu_model_get_item_attribute_value(mm, index, name, type);
+            EXPECT_EQ(itemval, nullptr);
+        }
+
+        void check_player_control_buttons(bool canPlay, bool canNext, bool canPrev)
+        {
+            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,
+                    "mock-can-do-play", canPlay,
+                    "mock-can-do-next", canNext,
+                    "mock-can-do-prev", canPrev,
+                    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(canPlay ? "indicator.play.player-id" : "indicator.play.player-id.disabled"));
+            verify_item_attribute(section, 1, "x-canonical-next-action", g_variant_new_string(canNext ? "indicator.next.player-id" : "indicator.next.player-id.disabled"));
+            verify_item_attribute(section, 1, "x-canonical-previous-action", g_variant_new_string(canPrev ? "indicator.previous.player-id" : "indicator.previous.player-id.disabled"));
+
+            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);
+        }
 };
 
 TEST_F(SoundMenuTest, BasicObject) {
-	SoundMenu * menu = sound_menu_new (nullptr, SOUND_MENU_DISPLAY_FLAGS_NONE);
-
-	ASSERT_NE(nullptr, menu);
-
-	g_clear_object(&menu);
-	return;
+    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;
-}
+    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,
+            "mock-can-do-play", TRUE,
+            "mock-can-do-next", TRUE,
+            "mock-can-do-prev", TRUE,
+            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;
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPlayNextPrev) {
+    check_player_control_buttons(false, false, false);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoNext) {
+    check_player_control_buttons(true, false, true);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPrev) {
+    check_player_control_buttons(true, true, false);
+}
+
+TEST_F(SoundMenuTest, AddRemovePlayerNoPlay) {
+    check_player_control_buttons(false, true, true);
+}
+
+//