panel-service: don't use the "evil hack" to get the entry value... Going back to HashTable

Using IO could lead to unexpected troubles and is slower, Hashtables are better :) (bzr r1858.11.3)

1 files changed, 102 insertions, 110 deletions

diff --git a/services/panel-service.c b/services/panel-service.c
index 89c8adb45..136509edf 100644
--- a/services/panel-service.c
+++ b/services/panel-service.c
@@ -50,6 +50,7 @@ static PanelService *static_service = NULL;
 struct _PanelServicePrivate
 {
 GSList *indicators;
+ GHashTable *id2entry_hash;
 GHashTable *panel2entries_hash;
 
 guint initial_sync_id;
@@ -120,8 +121,6 @@ static void load_indicators (PanelService *self);
 static void sort_indicators (PanelService *self);
 
 static void notify_object (IndicatorObject *object);
-static IndicatorObjectEntry *get_entry_at (PanelService *self, gint x, gint y);
-static IndicatorObject *get_entry_parent_indicator (IndicatorObjectEntry *entry);
 
 static GdkFilterReturn event_filter (GdkXEvent *ev,
 GdkEvent *gev,
@@ -137,6 +136,7 @@ panel_service_class_dispose (GObject *object)
 PanelServicePrivate *priv = PANEL_SERVICE (object)->priv;
 gint i;
 
+ g_hash_table_destroy (priv->id2entry_hash);
 g_hash_table_destroy (priv->panel2entries_hash);
 
 gdk_window_remove_filter (NULL, (GdkFilterFunc)event_filter, object);
@@ -247,6 +247,82 @@ panel_service_class_init (PanelServiceClass *klass)
 g_type_class_add_private (obj_class, sizeof (PanelServicePrivate));
 }
 
+static IndicatorObjectEntry *
+get_entry_at (PanelService *self, gint x, gint y)
+{
+ GHashTableIter panel_iter, entries_iter;
+ gpointer key, value, k, v;
+
+ g_hash_table_iter_init (&panel_iter, self->priv->panel2entries_hash);
+ while (g_hash_table_iter_next (&panel_iter, &key, &value))
+ {
+ GHashTable *entry2geometry_hash = value;
+ g_hash_table_iter_init (&entries_iter, entry2geometry_hash);
+
+ while (g_hash_table_iter_next (&entries_iter, &k, &v))
+ {
+ IndicatorObjectEntry *entry = k;
+ GdkRectangle *geo = v;
+
+ if (x >= geo->x && x <= (geo->x + geo->width) &&
+ y >= geo->y && y <= (geo->y + geo->height))
+ {
+ return entry;
+ }
+ }
+ }
+
+ return NULL;
+}
+
+static IndicatorObject *
+get_entry_parent_indicator (IndicatorObjectEntry *entry)
+{
+ if (!entry || !INDICATOR_IS_OBJECT (entry->parent_object))
+ return NULL;
+
+ return INDICATOR_OBJECT (entry->parent_object);
+}
+
+static gchar *
+get_indicator_entry_id_by_entry (IndicatorObjectEntry *entry)
+{
+ gchar *entry_id = NULL;
+
+ if (entry)
+ entry_id = g_strdup_printf ("%p", entry);
+
+ return entry_id;
+}
+
+static IndicatorObjectEntry *
+get_indicator_entry_by_id (PanelService *self, const gchar *entry_id)
+{
+ IndicatorObjectEntry *entry;
+
+ entry = g_hash_table_lookup (self->priv->id2entry_hash, entry_id);
+
+ if (entry)
+ {
+ if (g_slist_find (self->priv->indicators, entry->parent_object))
+ {
+ if (!INDICATOR_IS_OBJECT (entry->parent_object))
+ entry = NULL;
+ }
+ else
+ {
+ entry = NULL;
+ }
+
+ if (!entry)
+ {
+ g_warning("The entry id '%s' you're trying to lookup is not a valid IndicatorObjectEntry!", entry_id);
+ }
+ }
+
+ return entry;
+}
+
 static GdkFilterReturn
 event_filter (GdkXEvent *ev, GdkEvent *gev, PanelService *self)
 {
@@ -332,7 +408,7 @@ event_filter (GdkXEvent *ev, GdkEvent *gev, PanelService *self)
 /* If we were navigating over indicators using the keyboard
 * and now we click over the indicator under the mouse, we
 * must force it to show back again, not make it close */
- gchar *entry_id = g_strdup_printf ("%p", entry);
+ gchar *entry_id = get_indicator_entry_id_by_entry (entry);
 g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, entry_id);
 g_free (entry_id);
 }
@@ -346,7 +422,7 @@ event_filter (GdkXEvent *ev, GdkEvent *gev, PanelService *self)
 /* If we're scrolling or middle-clicking over an indicator
 * (which is not an appmenu entry) then we need to send the
 * event to the indicator itself, and avoid it to close */
- gchar *entry_id = g_strdup_printf ("%p", entry);
+ gchar *entry_id = get_indicator_entry_id_by_entry (entry);
 
 if (XIMaskIsSet (event->buttons.mask, 4) || XIMaskIsSet (event->buttons.mask, 5))
 {
@@ -367,100 +443,6 @@ event_filter (GdkXEvent *ev, GdkEvent *gev, PanelService *self)
 return ret;
 }
 
-static IndicatorObjectEntry *
-get_entry_at (PanelService *self, gint x, gint y)
-{
- GHashTableIter panel_iter, entries_iter;
- gpointer key, value, k, v;
-
- g_hash_table_iter_init (&panel_iter, self->priv->panel2entries_hash);
- while (g_hash_table_iter_next (&panel_iter, &key, &value))
- {
- GHashTable *entry2geometry_hash = value;
- g_hash_table_iter_init (&entries_iter, entry2geometry_hash);
-
- while (g_hash_table_iter_next (&entries_iter, &k, &v))
- {
- IndicatorObjectEntry *entry = k;
- GdkRectangle *geo = v;
-
- if (x >= geo->x && x <= (geo->x + geo->width) &&
- y >= geo->y && y <= (geo->y + geo->height))
- {
- return entry;
- }
- }
- }
-
- return NULL;
-}
-
-static IndicatorObject *
-get_entry_parent_indicator (IndicatorObjectEntry *entry)
-{
- if (!entry || !INDICATOR_IS_OBJECT (entry->parent_object))
- return NULL;
-
- return INDICATOR_OBJECT (entry->parent_object);
-}
-
-static IndicatorObjectEntry *
-get_indicator_entry_by_id (PanelService *self, const gchar *entry_id)
-{
- IndicatorObjectEntry *entry;
- entry = NULL;
-
- if (sscanf (entry_id, "%p", &entry) == 1)
- {
- /* Checking that entry is really an IndicatorObjectEntry.
- * To do that, we use an hack that allows to check if the pointer we read is
- * actually a valid pointer without crashing. This can be done using the
- * low-level write function to read from the pointer to a valid fds (we use
- * a pipe for convenience). Write will fail without crashing if we try to
- * read from an invalid pointer, so we can finally be pretty sure if the
- * pointed entry is an IndicatorObjectEntry by checking if it has a valid
- * parent IndicatorObject */
-
- int fds[2];
-
- if (pipe (fds) > -1)
- {
- size_t data_size = sizeof (IndicatorObjectEntry);
-
- if (write (fds[1], entry, data_size) != data_size)
- {
- entry = NULL;
- }
- else
- {
- if (g_slist_find (self->priv->indicators, entry->parent_object))
- {
- if (!INDICATOR_IS_OBJECT (entry->parent_object))
- entry = NULL;
- }
- else
- {
- entry = NULL;
- }
- }
-
- close (fds[0]);
- close (fds[1]);
- }
- else
- {
- entry = NULL;
- }
- }
-
- if (!entry)
- {
- g_warning("The entry id '%s' you're trying to parse is not a valid IndicatorObjectEntry!", entry_id);
- }
-
- return entry;
-}
-
 static gboolean
 initial_resync (PanelService *self)
 {
@@ -484,6 +466,7 @@ panel_service_init (PanelService *self)
 
 gdk_window_add_filter (NULL, (GdkFilterFunc)event_filter, self);
 
+ priv->id2entry_hash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 priv->panel2entries_hash = g_hash_table_new_full (g_str_hash, g_str_equal,
 g_free,
 (GDestroyNotify) g_hash_table_destroy);
@@ -522,15 +505,23 @@ panel_service_actually_remove_indicator (PanelService *self, IndicatorObject *in
 {
 for (l = entries; l; l = l->next)
 {
+ IndicatorObjectEntry *entry;
+ gchar *entry_id;
 GHashTableIter iter;
 gpointer key, value;
+
+ entry = l->data;
+ entry_id = get_indicator_entry_id_by_entry (entry);
+ g_hash_table_remove (self->priv->id2entry_hash, entry_id);
+ g_free (entry_id);
+
 g_hash_table_iter_init (&iter, self->priv->panel2entries_hash);
 while (g_hash_table_iter_next (&iter, &key, &value))
 {
 GHashTable *entry2geometry_hash = value;
 
 if (g_hash_table_size (entry2geometry_hash) > 1)
- g_hash_table_remove (entry2geometry_hash, l->data);
+ g_hash_table_remove (entry2geometry_hash, entry);
 else
 g_hash_table_iter_remove (&iter);
 }
@@ -737,6 +728,9 @@ on_entry_added (IndicatorObject *object,
 g_return_if_fail (PANEL_IS_SERVICE (self));
 g_return_if_fail (entry != NULL);
 
+ gchar *entry_id = get_indicator_entry_id_by_entry (entry);
+ g_hash_table_insert (self->priv->id2entry_hash, entry_id, entry);
+
 if (GTK_IS_LABEL (entry->label))
 {
 g_signal_connect (entry->label, "notify::label",
@@ -785,8 +779,8 @@ on_entry_removed (IndicatorObject *object,
 g_return_if_fail (PANEL_IS_SERVICE (self));
 g_return_if_fail (entry != NULL);
 
- /* Don't remove here the value from priv->panel2entries_hash, this should
- * be done in during the sync, to avoid false positive.
+ /* Don't remove here the value from id2entry_hash or panel2entries_hash,
+ * this should be done in during the sync, to avoid false positive.
 * FIXME this in libappmenu.so to avoid to send an "entry-removed" signal
 * when switching the focus from a window to one of its dialog children */
 
@@ -816,10 +810,8 @@ on_indicator_menu_show (IndicatorObject *object,
 return;
 }
 
- entry_id = g_strdup_printf ("%p", entry);
-
+ entry_id = get_indicator_entry_id_by_entry (entry);
 g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, entry_id);
-
 g_free (entry_id);
 }
 
@@ -838,10 +830,8 @@ on_indicator_menu_show_now_changed (IndicatorObject *object,
 return;
 }
 
- entry_id = g_strdup_printf ("%p", entry);
-
+ entry_id = get_indicator_entry_id_by_entry (entry);
 g_signal_emit (self, _service_signals[ENTRY_SHOW_NOW_CHANGED], 0, entry_id, show_now_changed);
-
 g_free (entry_id);
 }
 
@@ -1109,7 +1099,7 @@ indicator_object_to_variant (IndicatorObject *object, const gchar *indicator_id,
 {
 gint prio = -1;
 IndicatorObjectEntry *entry = e->data;
- gchar *id = g_strdup_printf ("%p", entry);
+ gchar *id = get_indicator_entry_id_by_entry (entry);
 
 if (entry->name_hint)
 {
@@ -1291,6 +1281,8 @@ panel_service_sync_geometry (PanelService *self,
 g_hash_table_remove (priv->panel2entries_hash, panel_id);
 }
 }
+
+ g_hash_table_remove (priv->id2entry_hash, entry_id);
 }
 else
 {
@@ -1433,7 +1425,7 @@ activate_next_prev_menu (PanelService *self,
 
 if (new_entry)
 {
- id = g_strdup_printf ("%p", new_entry);
+ id = get_indicator_entry_id_by_entry (entry);
 g_signal_emit (self, _service_signals[ENTRY_ACTIVATE_REQUEST], 0, id);
 g_free (id);
 }
@@ -1536,7 +1528,7 @@ panel_service_actually_show_entry (PanelService *self,
 
 if (entry != NULL)
 {
- gchar *entry_id = g_strdup_printf ("%p", entry);
+ gchar *entry_id = get_indicator_entry_id_by_entry (entry);
 g_signal_emit (self, _service_signals[ENTRY_ACTIVATED], 0, entry_id);
 g_free (entry_id);