The changes in this branch try to make the panel-service more crash-proof by disallowing casts of random pointers to IndicatorObjectEntry instances, as well as installing a weak notifier on the Indicators (in case they are unreferenced after instantiation).

Please note that this should go also into unity/4.0 branch.. Fixes: https://bugs.launchpad.net/bugs/861144, https://bugs.launchpad.net/bugs/869816. Reviewed by Tim Penhey. (bzr r1748)

3 files changed, 77 insertions, 30 deletions

diff --git a/services/panel-indicator-entry-accessible.c b/services/panel-indicator-entry-accessible.c
index 0ba338622..2b490f72c 100644
--- a/services/panel-indicator-entry-accessible.c
+++ b/services/panel-indicator-entry-accessible.c
@@ -235,9 +235,7 @@ panel_indicator_entry_accessible_initialize (AtkObject *accessible, gpointer dat
 {
 atk_object_set_role (accessible, ATK_ROLE_IMAGE);
 if (piea->priv->entry->accessible_desc != NULL)
- {
- atk_object_set_name (accessible, atk_object_get_name (ATK_OBJECT (piea->priv->entry->image)));
-	}
+ atk_object_set_name (accessible, piea->priv->entry->accessible_desc);
 }
 
 if (piea->priv->entry->accessible_desc != NULL)
diff --git a/services/panel-root-accessible.c b/services/panel-root-accessible.c
index 91b4491f8..d35f4c7dc 100644
--- a/services/panel-root-accessible.c
+++ b/services/panel-root-accessible.c
@@ -88,6 +88,26 @@ panel_root_accessible_new (void)
 return accessible;
 }
 
+static void
+on_indicator_removed (gpointer *data, GObject *removed_indicator)
+{
+ PanelRootAccessible *root = data[0];
+ AtkObject *child = data[1];
+
+ g_return_if_fail (PANEL_IS_ROOT_ACCESSIBLE (root));
+ g_return_if_fail (ATK_IS_OBJECT (child));
+
+ gint index = g_slist_index (root->priv->a11y_children, child);
+ if (index >= 0)
+ {
+ root->priv->a11y_children = g_slist_remove (root->priv->a11y_children, child);
+ g_signal_emit_by_name (root, "children-changed::remove", index, child);
+ g_object_unref (child);
+ }
+
+ g_free (data);
+}
+
 /* Implementation of AtkObject methods */
 
 static void
@@ -111,14 +131,23 @@ panel_root_accessible_initialize (AtkObject *accessible, gpointer data)
 IndicatorObject *indicator;
 
 indicator = panel_service_get_indicator_nth (root->priv->service, i);
- if (indicator != NULL)
+ if (INDICATOR_IS_OBJECT (indicator))
 {
- AtkObject *child;
+ AtkObject *child;
+ gpointer *data;
+
+ child = panel_indicator_accessible_new (indicator);
+ /* FIXME: use proper signals once we support dynamic adding/removing
+ * of indicators */
+ data = g_new0 (gpointer, 2);
+ data[0] = root;
+ data[1] = child;
+ g_object_weak_ref (G_OBJECT (indicator), 
+ (GWeakNotify) on_indicator_removed, data);
 
- child = panel_indicator_accessible_new (indicator);
 atk_object_set_parent (child, accessible);
- root->priv->a11y_children = g_slist_append (root->priv->a11y_children, child);
-	}
+ root->priv->a11y_children = g_slist_append (root->priv->a11y_children, child);
+ }
 }
 }
 
diff --git a/services/panel-service.c b/services/panel-service.c
index 30a7a8fa9..a4e7e4605 100644
--- a/services/panel-service.c
+++ b/services/panel-service.c
@@ -325,15 +325,30 @@ get_entry_at (PanelService *self, gint x, gint y)
 return NULL;
 }
 
-static IndicatorObjectEntry *
-get_entry_by_id (const gchar *entry_id)
+static void
+panel_service_get_indicator_entry_by_id (PanelService *self, 
+ const gchar *entry_id,
+ IndicatorObjectEntry **entry,
+ IndicatorObject **object)
 {
- IndicatorObjectEntry *entry;
- 
- if (sscanf (entry_id, "%p", &entry) == 1)
- return entry;
+ IndicatorObject *indicator;
+ IndicatorObjectEntry *probably_entry;
+ PanelServicePrivate *priv = self->priv;
 
- return NULL;
+ /* FIXME: eeek, why do we even do this? */
+ if (sscanf (entry_id, "%p", &probably_entry) == 1)
+ {
+ /* check that there really is such IndicatorObjectEntry */
+ indicator = g_hash_table_lookup (priv->entry2indicator_hash,
+ probably_entry);
+ if (object) *object = indicator;
+ if (entry) *entry = indicator != NULL ? probably_entry : NULL;
+ }
+ else
+ {
+ if (object) *object = NULL;
+ if (entry) *entry = NULL;
+ }
 }
 
 static gboolean
@@ -1132,9 +1147,11 @@ panel_service_sync_geometry (PanelService *self,
 gint width,
 gint height)
 {
- PanelServicePrivate *priv = self->priv;
- IndicatorObjectEntry *entry = get_entry_by_id (entry_id);
- IndicatorObject *object = g_hash_table_lookup (priv->entry2indicator_hash, entry);
+ IndicatorObject *object;
+ IndicatorObjectEntry *entry;
+ PanelServicePrivate *priv = self->priv;
+
+ panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
 
 if (entry)
 {
@@ -1184,9 +1201,9 @@ panel_service_sync_geometry (PanelService *self,
 geo->width = width;
 geo->height = height;
 }
- }
 
- g_signal_emit (self, _service_signals[GEOMETRIES_CHANGED], 0, object, entry, x, y, width, height);
+ g_signal_emit (self, _service_signals[GEOMETRIES_CHANGED], 0, object, entry, x, y, width, height);
+ }
 }
 
 static gboolean
@@ -1357,10 +1374,12 @@ panel_service_show_entry (PanelService *self,
 gint32 y,
 gint32 button)
 {
- PanelServicePrivate *priv = self->priv;
- IndicatorObjectEntry *entry = get_entry_by_id (entry_id);
- IndicatorObject *object = g_hash_table_lookup (priv->entry2indicator_hash, entry);
+ IndicatorObject *object;
+ IndicatorObjectEntry *entry;
 GtkWidget *last_menu;
+ PanelServicePrivate *priv = self->priv;
+
+ panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
 
 g_return_if_fail (entry);
 
@@ -1450,11 +1469,11 @@ panel_service_secondary_activate_entry (PanelService *self,
 const gchar *entry_id,
 guint32 timestamp)
 {
- PanelServicePrivate *priv = self->priv;
- IndicatorObjectEntry *entry = get_entry_by_id (entry_id);
- g_return_if_fail (entry);
+ IndicatorObject *object;
+ IndicatorObjectEntry *entry;
 
- IndicatorObject *object = g_hash_table_lookup (priv->entry2indicator_hash, entry);
+ panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
+ g_return_if_fail (entry);
 
 g_signal_emit_by_name(object, INDICATOR_OBJECT_SIGNAL_SECONDARY_ACTIVATE, entry,
 timestamp);
@@ -1465,11 +1484,12 @@ panel_service_scroll_entry (PanelService *self,
 const gchar *entry_id,
 gint32 delta)
 {
- PanelServicePrivate *priv = self->priv;
- IndicatorObjectEntry *entry = get_entry_by_id (entry_id);
+ IndicatorObject *object;
+ IndicatorObjectEntry *entry;
+
+ panel_service_get_indicator_entry_by_id (self, entry_id, &entry, &object);
 g_return_if_fail (entry);
 
- IndicatorObject *object = g_hash_table_lookup (priv->entry2indicator_hash, entry);
 GdkScrollDirection direction = delta < 0 ? GDK_SCROLL_DOWN : GDK_SCROLL_UP;
 
 g_signal_emit_by_name(object, INDICATOR_OBJECT_SIGNAL_ENTRY_SCROLLED, entry,