Merge lp:~cjcurran/indicator-sound/protect-against-rogue-property-updates-volume-slider into lp:~indicator-applet-developers/indicator-sound/trunk_3

[Conor Curran]

Fixes the bug attached. I found two issues,

1.I was unpacking a double into a boolean on the service side of the volume slider !! (don't know how this was not noticed until now)

Secondly I was not protecting against property updates on the volume slider which are not of the gdouble type. For some reason rogue property updates were being triggered on the slider item when I player started or closed. When trying to extract a double from variant that is not a double you will get 0 and I was in turn then setting the slider value to 0 which in turn triggered the gtk 'value-changed' callback which was then setting the pulse volume to you guessed it 0.
ah

[Ted Gould (ted)]

  review approve
  merge approve

review: Approve

=== modified file 'src/slider-menu-item.c'
--- src/slider-menu-item.c	2011-02-07 13:03:24 +0000
+++ src/slider-menu-item.c	2011-03-10 19:58:09 +0000
@@ -92,10 +92,11 @@
     input = g_variant_get_variant(value);
   }
 
-  gboolean volume_input = g_variant_get_double(input);
   if (value != NULL){
     if (IS_SLIDER_MENU_ITEM (mi)) {
       SliderMenuItemPrivate* priv = SLIDER_MENU_ITEM_GET_PRIVATE (SLIDER_MENU_ITEM (mi));
+      gdouble volume_input = g_variant_get_double(input);
+      //g_debug ("slider menu item about to update volume %f", volume_input);
       active_sink_update_volume (priv->a_sink, volume_input);
       active_sink_ensure_sink_is_unmuted (priv->a_sink);      
     }    

=== modified file 'src/volume-widget.c'
--- src/volume-widget.c	2011-03-09 01:17:01 +0000
+++ src/volume-widget.c	2011-03-10 19:58:09 +0000
@@ -128,6 +128,7 @@
                                GVariant* value, gpointer userdata)
 { 
   g_return_if_fail (IS_VOLUME_WIDGET (userdata)); 
+  g_return_if_fail (g_variant_is_of_type (value, G_VARIANT_TYPE_DOUBLE) );
   VolumeWidget* mitem = VOLUME_WIDGET(userdata);
   VolumeWidgetPrivate * priv = VOLUME_WIDGET_GET_PRIVATE(mitem);
   //g_debug("scrub-widget::property_update for prop %s", property); 
@@ -136,7 +137,6 @@
       GtkWidget *slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)priv->ido_volume_slider);
       GtkRange *range = (GtkRange*)slider;
       gdouble update = g_variant_get_double (value);
-      //g_debug("volume-widget - update level with value %f", update);
       gtk_range_set_value(range, update);
       update_accessible_desc(priv->indicator);
     }
@@ -163,13 +163,14 @@
 
 static gboolean
 volume_widget_change_value_cb (GtkRange     *range,
-                              GtkScrollType scroll,
-                              gdouble       new_value,
-                              gpointer      user_data)
+                               GtkScrollType scroll,
+                               gdouble       new_value,
+                               gpointer      user_data)
 {
   g_return_val_if_fail (IS_VOLUME_WIDGET (user_data), FALSE);
   VolumeWidget* mitem = VOLUME_WIDGET(user_data);
-  volume_widget_update(mitem, new_value); 
+  //g_debug ("changed value %f", new_value);
+  volume_widget_update(mitem, new_value);
   return FALSE;
 }
 
@@ -178,17 +179,19 @@
  which set the slider to 0 or 100. Ignore all other events.
 */ 
 static gboolean
-volume_widget_value_changed_cb(GtkRange *range, gpointer user_data)
+volume_widget_value_changed_cb (GtkRange *range, gpointer user_data)
 {
+
   g_return_val_if_fail (IS_VOLUME_WIDGET (user_data), FALSE);
   VolumeWidget* mitem = VOLUME_WIDGET(user_data);
   VolumeWidgetPrivate * priv = VOLUME_WIDGET_GET_PRIVATE(mitem);
-  GtkWidget *slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)priv->ido_volume_slider);  
+  GtkWidget *slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)priv->ido_volume_slider);
   gdouble current_value =  CLAMP(gtk_range_get_value(GTK_RANGE(slider)), 0, 100);
-  
+  //g_debug ("value changed %f", gtk_range_get_value(GTK_RANGE(slider)));
   if(current_value == 0 || current_value == 100){
     volume_widget_update(mitem, current_value);
   }
+
   return FALSE;
 }