[Xfce4-commits] [xfce/xfce4-power-manager] 01/01: plugin: Fix crash when devices (dis)connect
noreply at xfce.org
noreply at xfce.org
Tue Sep 2 02:46:33 CEST 2014
This is an automated email from the git hooks/post-receive script.
eric pushed a commit to branch master
in repository xfce/xfce4-power-manager.
commit 2e0de98b0b4322b92096554e3248a15771b3f9e3
Author: Eric Koegel <eric.koegel at gmail.com>
Date: Sun Aug 31 21:11:56 2014 +0300
plugin: Fix crash when devices (dis)connect
There are a couple conditions where the plguin would crash or
emit runtime warnings due a device being connected or
disconnected. Those should be fixed now.
---
.../power-manager-plugin/power-manager-button.c | 254 ++++++++++++++------
1 file changed, 175 insertions(+), 79 deletions(-)
diff --git a/panel-plugins/power-manager-plugin/power-manager-button.c b/panel-plugins/power-manager-plugin/power-manager-button.c
index 216f17f..42ba3f8 100644
--- a/panel-plugins/power-manager-plugin/power-manager-button.c
+++ b/panel-plugins/power-manager-plugin/power-manager-button.c
@@ -97,12 +97,14 @@ struct PowerManagerButtonPrivate
typedef struct
{
- GdkPixbuf *pix; /* Icon */
- gchar *details; /* Description of the device + state */
- gchar *object_path; /* UpDevice object path */
- UpDevice *device; /* Pointer to the UpDevice */
- gulong signal_id; /* device changed callback id */
- GtkWidget *menu_item; /* The device's item on the menu (if shown) */
+ GdkPixbuf *pix; /* Icon */
+ GtkWidget *img; /* Icon image in the menu */
+ gchar *details; /* Description of the device + state */
+ gchar *object_path; /* UpDevice object path */
+ UpDevice *device; /* Pointer to the UpDevice */
+ gulong changed_signal_id; /* device changed callback id */
+ gulong expose_signal_id; /* expose-event callback id */
+ GtkWidget *menu_item; /* The device's item on the menu (if shown) */
} BatteryDevice;
typedef enum
@@ -120,10 +122,10 @@ static gboolean power_manager_button_device_icon_expose (GtkWidget *img, GdkEven
static gboolean power_manager_button_set_icon (PowerManagerButton *button);
static gboolean power_manager_button_press_event (GtkWidget *widget, GdkEventButton *event);
static void power_manager_button_show_menu (PowerManagerButton *button);
-static void power_manager_button_menu_add_device (PowerManagerButton *button, BatteryDevice *battery_device, gboolean append);
+static gboolean power_manager_button_menu_add_device (PowerManagerButton *button, BatteryDevice *battery_device, gboolean append);
static void increase_brightness (PowerManagerButton *button);
static void decrease_brightness (PowerManagerButton *button);
-
+static void battery_device_remove_pix (BatteryDevice *battery_device);
static BatteryDevice*
@@ -133,35 +135,45 @@ get_display_device (PowerManagerButton *button)
gdouble highest_percentage = 0;
BatteryDevice *display_device = NULL;
+ TRACE("entering");
+
+ g_return_val_if_fail ( POWER_MANAGER_IS_BUTTON(button), NULL );
+
if (button->priv->display_device)
{
- item = find_device_in_list (button, up_device_get_object_path (button->priv->display_device));
- if (item)
- return item->data;
+ item = find_device_in_list (button, up_device_get_object_path (button->priv->display_device));
+ if (item)
+ {
+ return item->data;
+ }
}
/* We want to find the battery or ups device with the highest percentage
* and use that to get our tooltip from */
for (item = g_list_first (button->priv->devices); item != NULL; item = g_list_next (item))
{
- BatteryDevice *battery_device = item->data;
- guint type = 0;
- gdouble percentage;
+ BatteryDevice *battery_device = item->data;
+ guint type = 0;
+ gdouble percentage;
- g_object_get (battery_device->device,
- "kind", &type,
- "percentage", &percentage,
- NULL);
+ if (!battery_device->device || !UP_IS_DEVICE(battery_device))
+ {
+ continue;
+ }
- if ( type == UP_DEVICE_KIND_BATTERY || type == UP_DEVICE_KIND_UPS)
- {
- if ( highest_percentage < percentage )
- {
- /* found something better */
- display_device = battery_device;
- highest_percentage = percentage;
- }
- }
+ g_object_get (battery_device->device,
+ "kind", &type,
+ "percentage", &percentage,
+ NULL);
+
+ if ( type == UP_DEVICE_KIND_BATTERY || type == UP_DEVICE_KIND_UPS)
+ {
+ if ( highest_percentage < percentage )
+ {
+ display_device = battery_device;
+ highest_percentage = percentage;
+ }
+ }
}
return display_device;
@@ -172,16 +184,26 @@ power_manager_button_set_tooltip (PowerManagerButton *button)
{
BatteryDevice *display_device = get_display_device (button);
- if ( display_device )
+ TRACE("entering");
+
+ if (!GTK_IS_WIDGET (button))
{
- /* if we have something, display it */
- gtk_widget_set_tooltip_markup (GTK_WIDGET (button), display_device->details);
+ g_critical ("power_manager_button_set_tooltip: !GTK_IS_WIDGET (button)");
+ return;
}
- else
+
+ if ( display_device )
{
- /* how did we get here? */
- gtk_widget_set_tooltip_text (GTK_WIDGET (button), _("Display battery levels for attached devices"));
+ /* if we have something, display it */
+ if( display_device->details )
+ {
+ gtk_widget_set_tooltip_markup (GTK_WIDGET (button), display_device->details);
+ return;
+ }
}
+
+ /* Odds are this is a desktop without any batteries attached */
+ gtk_widget_set_tooltip_text (GTK_WIDGET (button), _("Display battery levels for attached devices"));
}
static GList*
@@ -195,9 +217,15 @@ find_device_in_list (PowerManagerButton *button, const gchar *object_path)
for (item = g_list_first (button->priv->devices); item != NULL; item = g_list_next (item))
{
- BatteryDevice *battery_device = item->data;
- if (g_strcmp0 (battery_device->object_path, object_path) == 0)
- return item;
+ BatteryDevice *battery_device = item->data;
+ if (battery_device == NULL)
+ {
+ DBG("!battery_device");
+ continue;
+ }
+
+ if (g_strcmp0 (battery_device->object_path, object_path) == 0)
+ return item;
}
return NULL;
@@ -207,7 +235,7 @@ static gboolean
power_manager_button_device_icon_expose (GtkWidget *img, GdkEventExpose *event, gpointer userdata)
{
cairo_t *cr;
- UpDevice *device = UP_DEVICE(userdata);
+ UpDevice *device = NULL;
guint type = 0, state = 0;
gdouble percentage;
gint height, width;
@@ -215,17 +243,33 @@ power_manager_button_device_icon_expose (GtkWidget *img, GdkEventExpose *event,
PangoLayout *layout = NULL;
PangoRectangle ink_extent, log_extent;
- TRACE("entering for %s", up_device_get_object_path(device));
- g_object_get (device,
- "kind", &type,
- "state", &state,
- "percentage", &percentage,
- NULL);
+ TRACE("entering");
- /* Don't draw the progressbar for Battery and UPS */
- if (type == UP_DEVICE_KIND_BATTERY || type == UP_DEVICE_KIND_UPS)
+ /* sanity checks */
+ if (!img || !GTK_IS_WIDGET (img))
return FALSE;
+ if (UP_IS_DEVICE (userdata))
+ {
+ device = UP_DEVICE(userdata);
+
+ g_object_get (device,
+ "kind", &type,
+ "state", &state,
+ "percentage", &percentage,
+ NULL);
+
+ /* Don't draw the progressbar for Battery and UPS */
+ if (type == UP_DEVICE_KIND_BATTERY || type == UP_DEVICE_KIND_UPS)
+ return FALSE;
+ }
+ else
+ {
+ /* If the UpDevice hasn't fully updated yet it then we'll want
+ * a question mark for sure. */
+ state = UP_DEVICE_STATE_UNKNOWN;
+ }
+
cr = gdk_cairo_create (img->window);
width = img->allocation.width;
height = img->allocation.height;
@@ -328,8 +372,9 @@ power_manager_button_update_device_icon_and_details (PowerManagerButton *button,
g_free(battery_device->details);
battery_device->details = details;
- if (battery_device->pix)
- g_object_unref (battery_device->pix);
+ /* If we had an image before, remove it and the callback */
+ battery_device_remove_pix(battery_device);
+
battery_device->pix = pix;
/* Get the display device, which may now be this one */
@@ -348,14 +393,17 @@ power_manager_button_update_device_icon_and_details (PowerManagerButton *button,
/* If the menu is being displayed, update it */
if (button->priv->menu && battery_device->menu_item)
{
- GtkWidget *img;
-
- gtk_menu_item_set_label (GTK_MENU_ITEM (battery_device->menu_item), details);
-
- /* update the image */
- img = gtk_image_new_from_pixbuf(battery_device->pix);
- gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(battery_device->menu_item), img);
- g_signal_connect_after (G_OBJECT (img), "expose-event", G_CALLBACK (power_manager_button_device_icon_expose), device);
+ gtk_menu_item_set_label (GTK_MENU_ITEM (battery_device->menu_item), details);
+
+ /* update the image, keep track of the signal ids and the img
+ * so we can disconnect it later */
+ battery_device->img = gtk_image_new_from_pixbuf(battery_device->pix);
+ g_object_ref (battery_device->img);
+ gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(battery_device->menu_item), battery_device->img);
+ battery_device->expose_signal_id = g_signal_connect_after (G_OBJECT (battery_device->img),
+ "expose-event",
+ G_CALLBACK (power_manager_button_device_icon_expose),
+ device);
}
}
@@ -401,8 +449,8 @@ power_manager_button_add_device (UpDevice *device, PowerManagerButton *button)
/* populate the struct */
battery_device->object_path = g_strdup (object_path);
- battery_device->signal_id = signal_id;
- battery_device->device = device;
+ battery_device->changed_signal_id = signal_id;
+ battery_device->device = g_object_ref(device);
/* add it to the list */
button->priv->devices = g_list_append (button->priv->devices, battery_device);
@@ -417,6 +465,33 @@ power_manager_button_add_device (UpDevice *device, PowerManagerButton *button)
}
}
+/* This function unrefs the pix and img from the battery device and
+ * disconnects the expose-event callback on the img.
+ */
+static void
+battery_device_remove_pix (BatteryDevice *battery_device)
+{
+ TRACE("entering");
+
+ if (battery_device == NULL)
+ return;
+
+ if (G_IS_OBJECT(battery_device->pix))
+ {
+ if (GTK_IS_WIDGET(battery_device->img))
+ {
+ if (battery_device->expose_signal_id != 0)
+ {
+ g_signal_handler_disconnect (battery_device->img, battery_device->expose_signal_id);
+ battery_device->expose_signal_id = 0;
+ }
+ g_object_unref (battery_device->img);
+ battery_device->img = NULL;
+ }
+ battery_device->pix = NULL;
+ }
+}
+
static void
power_manager_button_remove_device (PowerManagerButton *button, const gchar *object_path)
{
@@ -436,17 +511,18 @@ power_manager_button_remove_device (PowerManagerButton *button, const gchar *obj
if(battery_device->menu_item && button->priv->menu)
gtk_container_remove(GTK_CONTAINER(button->priv->menu), battery_device->menu_item);
- if (battery_device->pix)
- g_object_unref (battery_device->pix);
-
g_free(battery_device->details);
g_free(battery_device->object_path);
- if (battery_device->device)
+ if (battery_device->device != NULL && UP_IS_DEVICE(battery_device->device))
{
- if (battery_device->signal_id)
- g_signal_handler_disconnect (battery_device->device, battery_device->signal_id);
- g_object_unref (battery_device->device);
+ /* disconnect the signal handler if we were using it */
+ if (battery_device->changed_signal_id != 0)
+ g_signal_handler_disconnect (battery_device->device, battery_device->changed_signal_id);
+ battery_device->changed_signal_id = 0;
+
+ g_object_unref (battery_device->device);
+ battery_device->device = NULL;
}
/* remove it item and free the battery device */
@@ -920,22 +996,30 @@ menu_item_activate_cb(GtkWidget *object, gpointer user_data)
}
}
-static void
+static gboolean
power_manager_button_menu_add_device (PowerManagerButton *button, BatteryDevice *battery_device, gboolean append)
{
- GtkWidget *mi, *label, *img;
+ GtkWidget *mi, *label;
guint type = 0;
+ g_return_val_if_fail (POWER_MANAGER_IS_BUTTON (button), FALSE);
+
/* We need a menu to attach it to */
- g_return_if_fail (button->priv->menu);
+ g_return_val_if_fail (button->priv->menu, FALSE);
- g_object_get (battery_device->device,
- "kind", &type,
- NULL);
+ if (UP_IS_DEVICE(battery_device->device))
+ {
+ g_object_get (battery_device->device,
+ "kind", &type,
+ NULL);
- /* Don't add the display device or line power to the menu */
- if (type == UP_DEVICE_KIND_LINE_POWER || battery_device->device == button->priv->display_device)
- return;
+ /* Don't add the display device or line power to the menu */
+ if (type == UP_DEVICE_KIND_LINE_POWER || battery_device->device == button->priv->display_device)
+ {
+ DBG("filtering device from menu (display or line power device)");
+ return FALSE;
+ }
+ }
mi = gtk_image_menu_item_new_with_label(battery_device->details);
/* Make the menu item be bold and multi-line */
@@ -943,13 +1027,17 @@ power_manager_button_menu_add_device (PowerManagerButton *button, BatteryDevice
gtk_label_set_use_markup(GTK_LABEL(label), TRUE);
/* add the image */
- img = gtk_image_new_from_pixbuf(battery_device->pix);
- gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(mi), img);
+ battery_device->img = gtk_image_new_from_pixbuf(battery_device->pix);
+ g_object_ref (battery_device->img);
+ gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(mi), battery_device->img);
/* keep track of the menu item in the battery_device so we can update it */
battery_device->menu_item = mi;
g_signal_connect(G_OBJECT(mi), "destroy", G_CALLBACK(menu_item_destroyed_cb), button);
- g_signal_connect_after (G_OBJECT (img), "expose-event", G_CALLBACK (power_manager_button_device_icon_expose), battery_device->device);
+ battery_device->expose_signal_id = g_signal_connect_after (G_OBJECT (battery_device->img),
+ "expose-event",
+ G_CALLBACK (power_manager_button_device_icon_expose),
+ battery_device->device);
/* Active calls xfpm settings with the device's id to display details */
g_signal_connect(G_OBJECT(mi), "activate", G_CALLBACK(menu_item_activate_cb), button);
@@ -960,6 +1048,8 @@ power_manager_button_menu_add_device (PowerManagerButton *button, BatteryDevice
gtk_menu_shell_append(GTK_MENU_SHELL(button->priv->menu), mi);
else
gtk_menu_shell_prepend(GTK_MENU_SHELL(button->priv->menu), mi);
+
+ return TRUE;
}
static void
@@ -1076,6 +1166,10 @@ power_manager_button_show_menu (PowerManagerButton *button)
gboolean show_separator_flag = FALSE;
gint32 max_level, current_level = 0;
+ TRACE("entering");
+
+ g_return_if_fail (POWER_MANAGER_IS_BUTTON (button));
+
if(gtk_widget_has_screen(GTK_WIDGET(button)))
gscreen = gtk_widget_get_screen(GTK_WIDGET(button));
else
@@ -1091,9 +1185,11 @@ power_manager_button_show_menu (PowerManagerButton *button)
{
BatteryDevice *battery_device = item->data;
- power_manager_button_menu_add_device (button, battery_device, TRUE);
- /* If we add an item to the menu, show the separator */
- show_separator_flag = TRUE;
+ if (power_manager_button_menu_add_device (button, battery_device, TRUE))
+ {
+ /* If we add an item to the menu, show the separator */
+ show_separator_flag = TRUE;
+ }
}
if (show_separator_flag)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the Xfce4-commits
mailing list