Merge lp:~timo-jyrinki/libunity/blacklist-crash-1029949-6.0 into lp:libunity/6.0

[Timo Jyrinki]

Backport a fix to libunity 6.0: Fix crash when adding or removing players from the blacklist. (LP: #1029949)

Backport a fix to libunity 6.0: Fix crash when adding or removing players from the blacklist. (LP: #1029949)

[Łukasz Zemczak (sil2100)]

Works for me. Approving!

review: Approve

=== modified file 'src/unity-sound-menu-mpris.vala'
--- src/unity-sound-menu-mpris.vala	2012-09-20 19:52:53 +0000
+++ src/unity-sound-menu-mpris.vala	2012-11-07 08:48:20 +0000
@@ -376,30 +376,29 @@
     }
     
     private void add_to_blacklist ()
-    {      
-      var already_bl = this.settings.get_strv ("blacklisted-media-players");
-      var new_bl = new ArrayList<string>();
-
-      foreach (var s in already_bl){
-        if ( s == get_blacklist_name()) return;
-        Trace.log_object (this, "add_to_blacklist: %s", s);        
-        new_bl.add(s);
+    {
+      var blacklist = new GLib.VariantBuilder (new VariantType("as"));
+      foreach (var already_blacklisted in this.settings.get_strv("blacklisted-media-players"))
+      {
+        if (already_blacklisted == get_blacklist_name ())
+          return;
+        blacklist.add("s", already_blacklisted);
       }
-      new_bl.add (get_blacklist_name());
-      this.settings.set_strv ("blacklisted-media-players",
-                               new_bl.to_array());
+      blacklist.add("s", get_blacklist_name());
+      
+      this.settings.set_value("blacklisted-media-players", blacklist.end());
     }
 
-    private void remove_from_blacklist ()
+    private void remove_from_blacklist()
     {
-      var already_bl = this.settings.get_strv ("blacklisted-media-players");
-      var new_bl = new ArrayList<string>();
-      foreach (string s in already_bl){
-        if ( s == get_blacklist_name() )continue;
-        new_bl.add(s);
+      var blacklist = new GLib.VariantBuilder(new VariantType("as"));
+      foreach (var already_blacklisted in this.settings.get_strv("blacklisted-media-players"))
+      {
+        if (already_blacklisted == get_blacklist_name())
+          continue;
+        blacklist.add(already_blacklisted);
       }
-      this.settings.set_strv( "blacklisted-media-players",
-                               new_bl.to_array());
+      this.settings.set_value("blacklisted-media-players", blacklist.end());
     }
   }
   /****************************************************************************************/

=== modified file 'test/vala/Makefile.am'
--- test/vala/Makefile.am	2012-09-26 08:45:52 +0000
+++ test/vala/Makefile.am	2012-11-07 08:48:20 +0000
@@ -2,57 +2,57 @@
 
 DISTCHECK_CONFIGURE_FLAGS = --enable-integration-tests=no
 
-check_PROGRAMS = test-vala test-lens test-remote-scope
+check_PROGRAMS = test-vala test-lens test-remote-scope test-blacklist-crash
 
 AM_CPPFLAGS = \
-  -I$(top_srcdir) \
-  -I$(top_builddir)/protocol \
-  -I$(top_builddir)/src \
-  -DBUILDDIR=\"$(top_builddir)\" \
-  -DTESTDIR=\"$(top_srcdir)/test\" \
-  -DTESTVALADIR=\"$(top_srcdir)/test/vala\" \
-  -DG_SETTINGS_ENABLE_BACKEND \
-  $(LIBUNITY_CFLAGS) \
-  $(LIBUNITY_LIBS)
+	-I$(top_srcdir) \
+	-I$(top_builddir)/protocol \
+	-I$(top_builddir)/src \
+	-DBUILDDIR=\"$(top_builddir)\" \
+	-DTESTDIR=\"$(top_srcdir)/test\" \
+	-DTESTVALADIR=\"$(top_srcdir)/test/vala\" \
+	-DG_SETTINGS_ENABLE_BACKEND \
+	$(LIBUNITY_CFLAGS) \
+	$(LIBUNITY_LIBS)
 
-if !ENABLE_C_WARNINGS
-  AM_CPPFLAGS += -w
+if ENABLE_C_WARNINGS
+	AM_CPPFLAGS += -w
 endif
 
 if ENABLE_TRACE_LOG
-  AM_CPPFLAGS += -DENABLE_UNITY_TRACE_LOG
+	AM_CPPFLAGS += -DENABLE_UNITY_TRACE_LOG
 endif
 
 AM_VALAFLAGS = \
-  --vapidir=$(top_builddir)/src \
-  --vapidir=$(top_builddir)/protocol \
-  --vapidir=$(top_srcdir)/test/vala \
-  --pkg unity-protocol \
-  --pkg unity-internal \
-  --pkg config \
-  $(LIBUNITY_PACKAGES) \
-  $(MAINTAINER_VALAFLAGS) \
-  $(NULL)
+	--vapidir=$(top_builddir)/src \
+	--vapidir=$(top_builddir)/protocol \
+	--vapidir=$(top_srcdir)/test/vala \
+	--pkg unity-protocol \
+	--pkg unity-internal \
+	--pkg config \
+	$(LIBUNITY_PACKAGES) \
+	$(MAINTAINER_VALAFLAGS) \
+	$(NULL)
 
 test_libs = \
-  $(top_builddir)/src/libunity.la \
-  $(top_builddir)/protocol/libunity-protocol-private.la \
-  $(LIBUNITY_LIBS)
+	$(top_builddir)/src/libunity.la \
+	$(top_builddir)/protocol/libunity-protocol-private.la \
+	$(LIBUNITY_LIBS)
 
-TEST_PROGS += test-vala test-lens
+TEST_PROGS += test-vala test-lens test-blacklist-crash
 
 test_vala_LDADD = $(test_libs)
 
 test_vala_VALASOURCES = \
-  test-appinfo-manager.vala \
-  test-filters.vala \
-  test-io.vala \
-  test-launcher.vala \
-  test-preferences.vala \
-  test-previews.vala \
-  test-scope-signals.vala \
-  test-vala.vala \
-  $(NULL)
+	test-appinfo-manager.vala \
+	test-filters.vala \
+	test-io.vala \
+	test-launcher.vala \
+	test-preferences.vala \
+	test-previews.vala \
+	test-scope-signals.vala \
+	test-vala.vala \
+	$(NULL)
 nodist_test_vala_SOURCES = $(test_vala_VALASOURCES:.vala=.c)
 
 test_lens_LDADD = $(test_libs)
@@ -63,11 +63,16 @@
 test_remote_scope_VALASOURCES = test-remote-scope.vala
 nodist_test_remote_scope_SOURCES = $(test_remote_scope_VALASOURCES:.vala=.c)
 
+test_blacklist_crash_LDADD = $(test_libs)
+test_blacklist_crash_VALASOURCES = blacklist-crash-1029949-test-case.vala
+nodist_test_blacklist_crash_SOURCES = $(test_blacklist_crash_VALASOURCES:.vala=.c)
+
 BUILT_SOURCES = \
-  test-vala.vala.stamp \
-  test-lens.vala.stamp \
-  test-remote-scope.vala.stamp \
-  $(NULL)
+	test-vala.vala.stamp \
+	test-lens.vala.stamp \
+	test-remote-scope.vala.stamp \
+	test-blacklist-crash.vala.stamp \
+	$(NULL)
 
 test-vala.vala.stamp: $(test_vala_VALASOURCES)
 	$(AM_V_GEN)$(VALAC) -C $(AM_VALAFLAGS) $(VALAFLAGS) $^
@@ -81,21 +86,27 @@
 	$(AM_V_GEN)$(VALAC) -C $(AM_VALAFLAGS) $(VALAFLAGS) $^
 	@touch $@
 
+test-blacklist-crash.vala.stamp: $(test_blacklist_crash_VALASOURCES)
+	$(AM_V_GEN)$(VALAC) -C $(AM_VALAFLAGS) $(VALAFLAGS) $^
+	@touch $@
+
 include Makefile.integration_tests
 
 EXTRA_DIST += \
-  $(test_vala_VALASOURCES) \
-  $(test_lens_VALASOURCES) \
-  $(test_remote_scope_VALASOURCES) \
-  config.vapi \
-  $(NULL)
+	$(test_vala_VALASOURCES) \
+	$(test_lens_VALASOURCES) \
+	$(test_remote_scope_VALASOURCES) \
+	$(test_blacklist_crash_VALASOURCES) \
+	config.vapi \
+	$(NULL)
 
 CLEANFILES = \
-  *.stamp \
-  $(test_vala_VALASOURCES:.vala=.c) \
-  $(test_lens_VALASOURCES:.vala=.c) \
-  $(test_remote_scope_VALASOURCES:.vala=.c) \
-  $(NULL)
+	*.stamp \
+	$(test_vala_VALASOURCES:.vala=.c) \
+	$(test_lens_VALASOURCES:.vala=.c) \
+	$(test_remote_scope_VALASOURCES:.vala=.c) \
+	$(test_blacklist_crash_VALASOURCES:.vala=.c) \
+	$(NULL)
 
 # START HEADLESS TESTS
 if ENABLE_HEADLESS_TESTS

=== modified file 'test/vala/Makefile.integration_tests'
--- test/vala/Makefile.integration_tests	2012-08-07 12:31:44 +0000
+++ test/vala/Makefile.integration_tests	2012-11-07 08:48:20 +0000
@@ -139,4 +139,3 @@
 test-mpris-backend-prop-updates-server.vala.stamp: $(test_mpris_backend_prop_updates_server_VALASOURCES)
 	$(AM_V_GEN)$(VALAC) -C $(AM_VALAFLAGS) $(VALAFLAGS) $^
 	@touch $@
-

=== added file 'test/vala/blacklist-crash-1029949-test-case.vala'
--- test/vala/blacklist-crash-1029949-test-case.vala	1970-01-01 00:00:00 +0000
+++ test/vala/blacklist-crash-1029949-test-case.vala	2012-11-07 08:48:20 +0000
@@ -0,0 +1,44 @@
+using Gee;
+using Unity;
+
+
+
+int main(string[] args)
+{
+	int i = 0;
+	var loop = new MainLoop();
+	var time = new TimeoutSource(10);
+	MusicPlayer test_player = null;
+
+	time.set_callback(() => {
+		const string desktop_file_name = "rhythmbox.desktop";
+		test_player = new MusicPlayer(desktop_file_name);
+		test_player.export();
+		
+		return false;	
+	});
+	time.attach(loop.get_context());
+
+	var thread_timer = new TimeoutSource(100);
+	thread_timer.set_callback(() => {
+		set_blacklisted(test_player);
+		if (++i < 100)
+			return true;
+		test_player.unexport();
+		loop.quit();
+		return false;
+	});
+	thread_timer.attach(loop.get_context());
+
+    loop.run();
+	
+	return 0;
+}
+
+void* set_blacklisted(MusicPlayer test_player)
+{
+	int raw_blacklist = Random.int_range(0, 2);
+	bool blacklist = (bool) raw_blacklist;	
+	test_player.is_blacklisted = blacklist;
+	return null;
+}

=== modified file 'test/vala/test-mpris-backend-server.vala'
--- test/vala/test-mpris-backend-server.vala	2011-11-25 12:13:04 +0000
+++ test/vala/test-mpris-backend-server.vala	2012-11-07 08:48:20 +0000
@@ -16,6 +16,9 @@
 
   public bool escape ()
   {
+    // this is some cleanup
+    this.player.menu_player.is_blacklisted = false;
+
     this.mainloop.quit ();
     return false;
   }
@@ -73,6 +76,8 @@
     player.menu_player.player_menu = player_root;
 
     player.menu_player.add_playlist (pl2);
+
+    Timeout.add (5, blacklist_self);
     
     //The manual test of the playlistchanged signal, please leave commented
     //Timeout.add (5, change_playlist_name);                                          
@@ -81,6 +86,13 @@
     return false;
   }
 
+  // Regression test to ensure that setting blacklist does not cause a segfault
+  public bool blacklist_self()
+  {
+    this.player.menu_player.is_blacklisted = true;
+    return false;
+  }
+
   // Manual test of the playlist name change signal
   // Can't test this from the other side of dbus since there is no api to change
   // or edit playlists yet ...