Merge lp:~ted/libdbusmenu/event-grouping into lp:libdbusmenu/0.6
- event-grouping
- Merge into trunk.0.6
Status: | Merged |
---|---|
Approved by: | Charles Kerr |
Approved revision: | 428 |
Merged at revision: | 400 |
Proposed branch: | lp:~ted/libdbusmenu/event-grouping |
Merge into: | lp:libdbusmenu/0.6 |
Diff against target: |
1250 lines (+843/-62) 8 files modified
.bzrignore (+2/-0) docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt (+1/-0) libdbusmenu-glib/client.c (+446/-45) libdbusmenu-glib/client.h (+6/-0) libdbusmenu-glib/dbus-menu.xml (+46/-0) libdbusmenu-glib/server.c (+176/-17) tests/Makefile.am (+24/-0) tests/test-glib-events-nogroup-client.c (+142/-0) |
To merge this branch: | bzr merge lp:~ted/libdbusmenu/event-grouping |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Charles Kerr (community) | Approve | ||
Review via email: mp+100690@code.launchpad.net |
Commit message
Description of the change
Takes all of the events on a client and puts them into single dbus messages to be sent to the server. It also does the same thing for about-to-show messages. Both sets of messages are reserialized on either side to make callers unable to notice the difference.
Allison Karlitskaya (desrt) wrote : | # |
Allison Karlitskaya (desrt) wrote : | # |
Totally crazy outside-the-box type of idea (which I regret only having thought of just now): what if we just had a D-Bus API for "show all of the submenus that you know about" and the service side figured out which menus to deliver that to locally...
Allison Karlitskaya (desrt) wrote : | # |
The potential race caused by a "new submenu has appeared" signal crossing the bus at the same time as the "show all the submenus" method call presents an annoying problem. The API would rather have to be more like "enter into the always-showing mode" and "leave the always-showing mode" so that new items that got added on the client side would automatically have 'open' delivered to them without additional intervention from the HUD.
Ted Gould (ted) wrote : | # |
On Wed, 2012-04-04 at 00:41 +0000, Ryan Lortie wrote:
> - use a GQueue instead of prepend-
Eh, okay. r427
> - the flag to enable/disable event-grouping is awkward. By my read,
> it gets set according to the remote version but can be manually
> changed by the user but may change again depending on when we get the
> version number from the remote?
The flag is only there so that the test suite can test both cases to
ensure there are no regressions. Since it is possible someone would
want to use it, I saw no reason to keep it private. There's really no
use-case to send singular events if the grouping is available.
> - when doing mass-dispatching of events I'm unlikely to care about errors.
> I wouldn't bother reporting them. In fact, the only user of this API (the
> HUD) will have set the "don't send reply" flag.
Sure, but we need to maintain API compatibility no matter how someone
called the API it needs to work the same. It should be transparent to
upper level applications whether there is grouping or not. It's a
technical detail that can, and is, hidden in the library.
> - it's very possible that I might send some events and then unref the
> client before returning to the mainloop (or by returning to the
> mainloop to find an event source with a higher priority than your idle
> that causes the unref).
I'm unsure of what you're saying here. We do keep a ref to the client
so I think that case is taken care of... do you see a case where that's
not true?
> - the /* Get the icon theme directories if available */ comment is
> intensely confusing (copy/paste error?)
Fixed. r428
> - some of your use of GVariant is a bit odd (although you could argue
> a style of prefering to avoid varargs use):
Exactly. And I will :-) I like it when the compiler tells me when I've
done thing wrong not bug reports from apport :-)
> - the receiving side looks good except for the questionable use of an
> idle/timeout(0). Why did you do that? You're already in a rather
> direct dispatch from the mainloop -- punting to another idle here
> gains you nothing except for additional complexity and may also
> introduce ordering issues (ie: the next dbus message to arrive may end
> up being processed before the idle for your group-of-events that
> arrived first).
The problem that we found is that in some cases we weren't giving enough
time for the mainloop to process and this would make application appear
"hung" to some users. Effectively appmenu-gtk is a leach on mainloop
time and we're trying to be as light a pull on that as possible. I
believe this will be especially the case with the number of events the
HUD is sending.
> Totally crazy outside-the-box type of idea (which I regret only having
> thought of just now): what if we just had a D-Bus API for "show all of
> the submenus that you know about" and the service side figured out
> which menus to deliver that to locally...
We kinda need to stay with as much as possible the events that already
exist. Adding a new program flow through all the various dbusmenu
implementations would be difficult and risky.
> The potential race caused by a "new submenu has appeared" signa...
Allison Karlitskaya (desrt) wrote : | # |
fwiw, It's possible to use GQueue embedded in the parent struct. This is when it's at its best. I can understand your lack of enthusiasm for switching to it when you have to manually g_queue_
About the API for grouping: I guess I was falsely considering that the user would be responsible for setting the grouping flag for themselves to opt-in to the feature. I didn't conisder that it would be enabled for everyone on the basis of version (which I only discovered after more reading). It sort of makes more sense now that I consider that.
Ted Gould (ted) wrote : | # |
On Wed, 2012-04-04 at 12:56 +0000, Ryan Lortie wrote:
> fwiw, It's possible to use GQueue embedded in the parent struct. This
> is when it's at its best. I can understand your lack of enthusiasm
> for switching to it when you have to manually
> g_queue_
The problem there is that a new queue always needs to be created when
the dbus message is sent and the old queue attached to the callback. So
we have to allocate the queues.
Charles Kerr (charlesk) wrote : | # |
I agree with some of the discussion here but the remaining refinements aren't blockers for today's release.
Preview Diff
1 | === modified file '.bzrignore' |
2 | --- .bzrignore 2011-08-22 19:31:31 +0000 |
3 | +++ .bzrignore 2012-04-04 04:28:21 +0000 |
4 | @@ -257,3 +257,5 @@ |
5 | genericmenuitem-enum-types.c |
6 | genericmenuitem-enum-types.h |
7 | libdbusmenu_gtk3_la-genericmenuitem-enum-types.lo |
8 | +test-glib-events-nogroup |
9 | +test-glib-events-nogroup-client |
10 | |
11 | === modified file 'docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt' |
12 | --- docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt 2012-01-31 04:56:12 +0000 |
13 | +++ docs/libdbusmenu-glib/reference/libdbusmenu-glib-sections.txt 2012-04-04 04:28:21 +0000 |
14 | @@ -9,6 +9,7 @@ |
15 | DBUSMENU_CLIENT_SIGNAL_ICON_THEME_DIRS_CHANGED |
16 | DBUSMENU_CLIENT_PROP_DBUS_NAME |
17 | DBUSMENU_CLIENT_PROP_DBUS_OBJECT |
18 | +DBUSMENU_CLIENT_PROP_GROUP_EVENTS |
19 | DBUSMENU_CLIENT_PROP_STATUS |
20 | DBUSMENU_CLIENT_PROP_TEXT_DIRECTION |
21 | DBUSMENU_CLIENT_TYPES_DEFAULT |
22 | |
23 | === modified file 'libdbusmenu-glib/client.c' |
24 | --- libdbusmenu-glib/client.c 2012-03-16 17:26:58 +0000 |
25 | +++ libdbusmenu-glib/client.c 2012-04-04 04:28:21 +0000 |
26 | @@ -52,7 +52,8 @@ |
27 | PROP_DBUSOBJECT, |
28 | PROP_DBUSNAME, |
29 | PROP_STATUS, |
30 | - PROP_TEXT_DIRECTION |
31 | + PROP_TEXT_DIRECTION, |
32 | + PROP_GROUP_EVENTS |
33 | }; |
34 | |
35 | /* Signals */ |
36 | @@ -66,6 +67,12 @@ |
37 | LAST_SIGNAL |
38 | }; |
39 | |
40 | +/* Errors */ |
41 | +enum { |
42 | + ERROR_DISPOSAL, |
43 | + ERROR_ID_NOT_FOUND |
44 | +}; |
45 | + |
46 | typedef void (*properties_func) (GVariant * properties, GError * error, gpointer user_data); |
47 | |
48 | static guint signals[LAST_SIGNAL] = { 0 }; |
49 | @@ -100,6 +107,13 @@ |
50 | DbusmenuTextDirection text_direction; |
51 | DbusmenuStatus status; |
52 | GStrv icon_dirs; |
53 | + |
54 | + gboolean group_events; |
55 | + guint event_idle; |
56 | + GQueue * events_to_go; /* type: event_data_t * */ |
57 | + |
58 | + guint about_to_show_idle; |
59 | + GQueue * about_to_show_to_go; /* type: about_to_show_t * */ |
60 | }; |
61 | |
62 | typedef struct _newItemPropData newItemPropData; |
63 | @@ -120,6 +134,7 @@ |
64 | |
65 | typedef struct _event_data_t event_data_t; |
66 | struct _event_data_t { |
67 | + gint id; |
68 | DbusmenuClient * client; |
69 | DbusmenuMenuitem * menuitem; |
70 | gchar * event; |
71 | @@ -171,6 +186,8 @@ |
72 | static void menuproxy_name_changed_cb (GObject * object, GParamSpec * pspec, gpointer user_data); |
73 | static void menuproxy_signal_cb (GDBusProxy * proxy, gchar * sender, gchar * signal, GVariant * params, gpointer user_data); |
74 | static void type_handler_destroy (gpointer user_data); |
75 | +static void event_data_end (event_data_t * eventd, GError * error); |
76 | +static void about_to_show_finish_pntr (gpointer data, gpointer user_data); |
77 | |
78 | /* Globals */ |
79 | static GDBusNodeInfo * dbusmenu_node_info = NULL; |
80 | @@ -309,6 +326,10 @@ |
81 | "Signals which direction the default text direction is for the menus", |
82 | DBUSMENU_TYPE_TEXT_DIRECTION, DBUSMENU_TEXT_DIRECTION_NONE, |
83 | G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); |
84 | + g_object_class_install_property (object_class, PROP_GROUP_EVENTS, |
85 | + g_param_spec_boolean(DBUSMENU_CLIENT_PROP_GROUP_EVENTS, "Whether or not multiple events should be grouped", |
86 | + "Event grouping lowers the number of messages on DBus and will be set automatically based on the version to optimize traffic. It can be disabled for testing or other purposes.", |
87 | + FALSE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); |
88 | |
89 | if (dbusmenu_node_info == NULL) { |
90 | GError * error = NULL; |
91 | @@ -380,6 +401,13 @@ |
92 | priv->status = DBUSMENU_STATUS_NORMAL; |
93 | priv->icon_dirs = NULL; |
94 | |
95 | + priv->group_events = FALSE; |
96 | + priv->event_idle = 0; |
97 | + priv->events_to_go = NULL; |
98 | + |
99 | + priv->about_to_show_idle = 0; |
100 | + priv->about_to_show_to_go = NULL; |
101 | + |
102 | return; |
103 | } |
104 | |
105 | @@ -393,6 +421,32 @@ |
106 | priv->delayed_idle = 0; |
107 | } |
108 | |
109 | + if (priv->event_idle != 0) { |
110 | + g_source_remove(priv->event_idle); |
111 | + priv->event_idle = 0; |
112 | + } |
113 | + |
114 | + if (priv->about_to_show_idle != 0) { |
115 | + g_source_remove(priv->about_to_show_idle); |
116 | + priv->about_to_show_idle = 0; |
117 | + } |
118 | + |
119 | + if (priv->events_to_go != NULL) { |
120 | + g_warning("Getting to client dispose with events pending. This is odd. Probably there's a ref count problem somewhere, but we're going to be cool about it now and clean up. But there's probably a bug."); |
121 | + GError * error = g_error_new_literal(error_domain(), ERROR_DISPOSAL, "Client disposed before event signal returned"); |
122 | + g_queue_foreach(priv->events_to_go, (GFunc)event_data_end, error); |
123 | + g_queue_free(priv->events_to_go); |
124 | + priv->events_to_go = NULL; |
125 | + g_error_free(error); |
126 | + } |
127 | + |
128 | + if (priv->about_to_show_to_go != NULL) { |
129 | + g_warning("Getting to client dispose with about_to_show's pending. This is odd. Probably there's a ref count problem somewhere, but we're going to be cool about it now and clean up. But there's probably a bug."); |
130 | + g_queue_foreach(priv->about_to_show_to_go, about_to_show_finish_pntr, GINT_TO_POINTER(FALSE)); |
131 | + g_queue_free(priv->about_to_show_to_go); |
132 | + priv->about_to_show_to_go = NULL; |
133 | + } |
134 | + |
135 | /* Only used for queueing up a new command, so we can |
136 | just drop this array. */ |
137 | if (priv->delayed_property_list != NULL) { |
138 | @@ -517,6 +571,9 @@ |
139 | build_proxies(DBUSMENU_CLIENT(obj)); |
140 | } |
141 | break; |
142 | + case PROP_GROUP_EVENTS: |
143 | + priv->group_events = g_value_get_boolean(value); |
144 | + break; |
145 | default: |
146 | g_warning("Unknown property %d.", id); |
147 | return; |
148 | @@ -543,6 +600,9 @@ |
149 | case PROP_TEXT_DIRECTION: |
150 | g_value_set_enum(value, priv->text_direction); |
151 | break; |
152 | + case PROP_GROUP_EVENTS: |
153 | + g_value_set_boolean(value, priv->group_events); |
154 | + break; |
155 | default: |
156 | g_warning("Unknown property %d.", id); |
157 | return; |
158 | @@ -649,7 +709,7 @@ |
159 | for (i = 0; i < listeners->len; i++) { |
160 | properties_listener_t * listener = &g_array_index(listeners, properties_listener_t, i); |
161 | if (!listener->replied) { |
162 | - g_warning("Generating properties error for: %d", listener->id); |
163 | + g_debug("Generating properties error for: %d", listener->id); |
164 | if (localerror == NULL) { |
165 | g_set_error_literal(&localerror, error_domain(), 0, "Error getting properties for ID"); |
166 | } |
167 | @@ -1086,6 +1146,7 @@ |
168 | g_object_notify(G_OBJECT(user_data), DBUSMENU_CLIENT_PROP_TEXT_DIRECTION); |
169 | |
170 | g_variant_unref(textdir); |
171 | + textdir = NULL; |
172 | } |
173 | |
174 | /* Check the status if available */ |
175 | @@ -1101,6 +1162,7 @@ |
176 | g_object_notify(G_OBJECT(user_data), DBUSMENU_CLIENT_PROP_STATUS); |
177 | |
178 | g_variant_unref(status); |
179 | + status = NULL; |
180 | } |
181 | |
182 | /* Get the icon theme directories if available */ |
183 | @@ -1115,6 +1177,33 @@ |
184 | g_signal_emit(G_OBJECT(client), signals[ICON_THEME_DIRS], 0, priv->icon_dirs, TRUE); |
185 | |
186 | g_variant_unref(icon_dirs); |
187 | + icon_dirs = NULL; |
188 | + } |
189 | + |
190 | + /* Get the dbusmenu protocol version if available */ |
191 | + GVariant * version = g_dbus_proxy_get_cached_property(priv->menuproxy, "Version"); |
192 | + if (version != NULL) { |
193 | + guint32 remote_version = 0; |
194 | + |
195 | + if (g_variant_is_of_type(version, G_VARIANT_TYPE_UINT32)) { |
196 | + remote_version = g_variant_get_uint32(version); |
197 | + } |
198 | + |
199 | + gboolean old_group = priv->group_events; |
200 | + /* Figure out if we can group the events or not */ |
201 | + if (remote_version >= 3) { |
202 | + priv->group_events = TRUE; |
203 | + } else { |
204 | + priv->group_events = FALSE; |
205 | + } |
206 | + |
207 | + /* Notify listeners if we changed the value */ |
208 | + if (old_group != priv->group_events) { |
209 | + g_object_notify(G_OBJECT(client), DBUSMENU_CLIENT_PROP_GROUP_EVENTS); |
210 | + } |
211 | + |
212 | + g_variant_unref(version); |
213 | + version = NULL; |
214 | } |
215 | |
216 | /* If we get here, we don't need the DBus proxy */ |
217 | @@ -1196,6 +1285,19 @@ |
218 | priv->icon_dirs = g_variant_dup_strv(value, NULL); |
219 | dirs_changed = TRUE; |
220 | } |
221 | + if (g_strcmp0(key, "Version") == 0) { |
222 | + guint32 remote_version = 0; |
223 | + |
224 | + if (g_variant_is_of_type(value, G_VARIANT_TYPE_UINT32)) { |
225 | + remote_version = g_variant_get_uint32(value); |
226 | + } |
227 | + |
228 | + if (remote_version >= 3) { |
229 | + priv->group_events = TRUE; |
230 | + } else { |
231 | + priv->group_events = FALSE; |
232 | + } |
233 | + } |
234 | } |
235 | |
236 | if (olddir != priv->text_direction) { |
237 | @@ -1441,7 +1543,7 @@ |
238 | newItemPropData * propdata = (newItemPropData *)data; |
239 | |
240 | if (error != NULL) { |
241 | - g_warning("Error getting properties on a new menuitem: %s", error->message); |
242 | + g_debug("Error getting properties on a new menuitem: %s", error->message); |
243 | goto out; |
244 | } |
245 | |
246 | @@ -1488,21 +1590,11 @@ |
247 | return; |
248 | } |
249 | |
250 | -/* Respond to the call function to make sure that the other side |
251 | - got it, or print a warning. */ |
252 | +/* A function to work with an event_data_t and make sure it gets |
253 | + free'd and in a terminal state. */ |
254 | static void |
255 | -menuitem_call_cb (GObject * proxy, GAsyncResult * res, gpointer userdata) |
256 | +event_data_end (event_data_t * edata, GError * error) |
257 | { |
258 | - GError * error = NULL; |
259 | - event_data_t * edata = (event_data_t *)userdata; |
260 | - GVariant * params; |
261 | - |
262 | - params = g_dbus_proxy_call_finish(G_DBUS_PROXY(proxy), res, &error); |
263 | - |
264 | - if (error != NULL) { |
265 | - g_warning("Unable to call event '%s' on menu item %d: %s", edata->event, dbusmenu_menuitem_get_id(edata->menuitem), error->message); |
266 | - } |
267 | - |
268 | g_signal_emit(edata->client, signals[EVENT_RESULT], 0, edata->menuitem, edata->event, edata->variant, edata->timestamp, error, TRUE); |
269 | |
270 | g_variant_unref(edata->variant); |
271 | @@ -1511,6 +1603,26 @@ |
272 | g_object_unref(edata->client); |
273 | g_free(edata); |
274 | |
275 | + return; |
276 | +} |
277 | + |
278 | +/* Respond to the call function to make sure that the other side |
279 | + got it, or print a warning. */ |
280 | +static void |
281 | +menuitem_call_cb (GObject * proxy, GAsyncResult * res, gpointer userdata) |
282 | +{ |
283 | + GError * error = NULL; |
284 | + event_data_t * edata = (event_data_t *)userdata; |
285 | + GVariant * params; |
286 | + |
287 | + params = g_dbus_proxy_call_finish(G_DBUS_PROXY(proxy), res, &error); |
288 | + |
289 | + if (error != NULL) { |
290 | + g_warning("Unable to call event '%s' on menu item %d: %s", edata->event, dbusmenu_menuitem_get_id(edata->menuitem), error->message); |
291 | + } |
292 | + |
293 | + event_data_end(edata, error); |
294 | + |
295 | if (G_UNLIKELY(error != NULL)) { |
296 | g_error_free(error); |
297 | } |
298 | @@ -1521,6 +1633,128 @@ |
299 | return; |
300 | } |
301 | |
302 | +/* Looks at event_data_t structs to match an ID */ |
303 | +gint |
304 | +event_data_find (gconstpointer data, gconstpointer user_data) |
305 | +{ |
306 | + event_data_t * edata = (event_data_t *)data; |
307 | + gint id = GPOINTER_TO_INT(user_data); |
308 | + |
309 | + if (edata->id == id) { |
310 | + return 0; |
311 | + } else { |
312 | + return -1; |
313 | + } |
314 | +} |
315 | + |
316 | +/* The callback from the dbus message to pass events to the |
317 | + to the server en masse */ |
318 | +static void |
319 | +event_group_cb (GObject * proxy, GAsyncResult * res, gpointer user_data) |
320 | +{ |
321 | + GQueue * events = (GQueue *)user_data; |
322 | + |
323 | + GError * error = NULL; |
324 | + GVariant * params; |
325 | + params = g_dbus_proxy_call_finish(G_DBUS_PROXY(proxy), res, &error); |
326 | + |
327 | + if (error != NULL) { |
328 | + /* If we got an actual DBus error, we should just pass that |
329 | + along and finish up */ |
330 | + g_queue_foreach(events, (GFunc)event_data_end, error); |
331 | + g_queue_free(events); |
332 | + events = NULL; |
333 | + return; |
334 | + } |
335 | + |
336 | + gint id = 0; |
337 | + GVariant * array = g_variant_get_child_value(params, 0); |
338 | + GVariantIter iter; |
339 | + g_variant_iter_init(&iter, array); |
340 | + |
341 | + while (g_variant_iter_loop(&iter, "i", &id)) { |
342 | + GList * item = g_queue_find_custom(events, GINT_TO_POINTER(id), event_data_find); |
343 | + |
344 | + if (item != NULL) { |
345 | + GError * iderror = g_error_new(error_domain(), ERROR_ID_NOT_FOUND, "Unable to find ID: %d", id); |
346 | + event_data_end((event_data_t *)item->data, iderror); |
347 | + g_queue_delete_link(events, item); |
348 | + g_error_free(iderror); |
349 | + } |
350 | + } |
351 | + |
352 | + g_variant_unref(array); |
353 | + g_variant_unref(params); |
354 | + |
355 | + /* If we have any left send non-error responses */ |
356 | + g_queue_foreach(events, (GFunc)event_data_end, NULL); |
357 | + g_queue_free(events); |
358 | + return; |
359 | +} |
360 | + |
361 | +/* Turn an event structure into the variant builder form */ |
362 | +static void |
363 | +events_to_builder (gpointer data, gpointer user_data) |
364 | +{ |
365 | + event_data_t * edata = (event_data_t *)data; |
366 | + GVariantBuilder * builder = (GVariantBuilder *)user_data; |
367 | + |
368 | + GVariantBuilder tuple; |
369 | + g_variant_builder_init(&tuple, G_VARIANT_TYPE_TUPLE); |
370 | + |
371 | + g_variant_builder_add_value(&tuple, g_variant_new_int32(edata->id)); |
372 | + g_variant_builder_add_value(&tuple, g_variant_new_string(edata->event)); |
373 | + g_variant_builder_add_value(&tuple, g_variant_new_variant(edata->variant)); |
374 | + g_variant_builder_add_value(&tuple, g_variant_new_uint32(edata->timestamp)); |
375 | + |
376 | + GVariant * vtuple = g_variant_builder_end(&tuple); |
377 | + g_variant_builder_add_value(builder, vtuple); |
378 | + return; |
379 | +} |
380 | + |
381 | +/* Group all the events into a single Dbus message and send |
382 | + that out */ |
383 | +static gboolean |
384 | +event_idle_cb (gpointer user_data) |
385 | +{ |
386 | + g_return_val_if_fail(DBUSMENU_IS_CLIENT(user_data), FALSE); |
387 | + DbusmenuClient * client = DBUSMENU_CLIENT(user_data); |
388 | + DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(user_data); |
389 | + |
390 | + /* We use prepend for speed, but now we want to have them |
391 | + in the order they were called incase that matters. */ |
392 | + GQueue * levents = priv->events_to_go; |
393 | + priv->events_to_go = NULL; |
394 | + priv->event_idle = 0; |
395 | + |
396 | + GVariantBuilder array; |
397 | + g_variant_builder_init(&array, G_VARIANT_TYPE("a(isvu)")); |
398 | + g_queue_foreach(levents, events_to_builder, &array); |
399 | + GVariant * vevents = g_variant_builder_end(&array); |
400 | + |
401 | + if (g_signal_has_handler_pending (client, signals[EVENT_RESULT], 0, TRUE)) { |
402 | + g_dbus_proxy_call(priv->menuproxy, |
403 | + "EventGroup", |
404 | + g_variant_new_tuple(&vevents, 1), |
405 | + G_DBUS_CALL_FLAGS_NONE, |
406 | + 1000, /* timeout */ |
407 | + NULL, /* cancellable */ |
408 | + event_group_cb, levents); |
409 | + } else { |
410 | + g_dbus_proxy_call(priv->menuproxy, |
411 | + "EventGroup", |
412 | + g_variant_new_tuple(&vevents, 1), |
413 | + G_DBUS_CALL_FLAGS_NONE, |
414 | + 1000, /* timeout */ |
415 | + NULL, /* cancellable */ |
416 | + NULL, NULL); |
417 | + g_queue_foreach(levents, (GFunc)event_data_end, NULL); |
418 | + g_queue_free(levents); |
419 | + } |
420 | + |
421 | + return FALSE; |
422 | +} |
423 | + |
424 | /* Sends the event over DBus to the server on the other side |
425 | of the bus. */ |
426 | void |
427 | @@ -1541,7 +1775,7 @@ |
428 | } |
429 | |
430 | /* Don't bother with the reply handling if nobody is watching... */ |
431 | - if (!g_signal_has_handler_pending (client, signals[EVENT_RESULT], 0, TRUE)) { |
432 | + if (!priv->group_events && !g_signal_has_handler_pending (client, signals[EVENT_RESULT], 0, TRUE)) { |
433 | g_dbus_proxy_call(priv->menuproxy, |
434 | "Event", |
435 | g_variant_new("(isvu)", id, name, variant, timestamp), |
436 | @@ -1553,6 +1787,7 @@ |
437 | } |
438 | |
439 | event_data_t * edata = g_new0(event_data_t, 1); |
440 | + edata->id = id; |
441 | edata->client = client; |
442 | g_object_ref(client); |
443 | edata->menuitem = mi; |
444 | @@ -1562,25 +1797,181 @@ |
445 | edata->variant = variant; |
446 | g_variant_ref_sink(variant); |
447 | |
448 | - g_dbus_proxy_call(priv->menuproxy, |
449 | - "Event", |
450 | - g_variant_new("(isvu)", id, name, variant, timestamp), |
451 | - G_DBUS_CALL_FLAGS_NONE, |
452 | - 1000, /* timeout */ |
453 | - NULL, /* cancellable */ |
454 | - menuitem_call_cb, |
455 | - edata); |
456 | + if (!priv->group_events) { |
457 | + g_dbus_proxy_call(priv->menuproxy, |
458 | + "Event", |
459 | + g_variant_new("(isvu)", id, name, variant, timestamp), |
460 | + G_DBUS_CALL_FLAGS_NONE, |
461 | + 1000, /* timeout */ |
462 | + NULL, /* cancellable */ |
463 | + menuitem_call_cb, |
464 | + edata); |
465 | + } else { |
466 | + if (priv->events_to_go == NULL) { |
467 | + priv->events_to_go = g_queue_new(); |
468 | + } |
469 | + |
470 | + g_queue_push_tail(priv->events_to_go, edata); |
471 | + |
472 | + if (priv->event_idle == 0) { |
473 | + priv->event_idle = g_idle_add(event_idle_cb, client); |
474 | + } |
475 | + } |
476 | |
477 | return; |
478 | } |
479 | |
480 | typedef struct _about_to_show_t about_to_show_t; |
481 | struct _about_to_show_t { |
482 | + gint id; |
483 | DbusmenuClient * client; |
484 | void (*cb) (gpointer data); |
485 | gpointer cb_data; |
486 | }; |
487 | |
488 | +/* Takes an about_to_show_t structure and calls the callback correctly |
489 | + and updates the layout if needed. */ |
490 | +static void |
491 | +about_to_show_finish (about_to_show_t * data, gboolean need_update) |
492 | +{ |
493 | + /* If we need to update, do that first. */ |
494 | + if (need_update) { |
495 | + update_layout(data->client); |
496 | + } |
497 | + |
498 | + if (data->cb != NULL) { |
499 | + data->cb(data->cb_data); |
500 | + } |
501 | + |
502 | + g_object_unref(data->client); |
503 | + g_free(data); |
504 | + |
505 | + return; |
506 | +} |
507 | + |
508 | +/* A little function to match prototypes and make sure to convert from |
509 | + a pointer to an int correctly */ |
510 | +static void |
511 | +about_to_show_finish_pntr (gpointer data, gpointer user_data) |
512 | +{ |
513 | + return about_to_show_finish((about_to_show_t *)data, GPOINTER_TO_INT(user_data)); |
514 | +} |
515 | + |
516 | +/* Respond to the DBus message from sending a bunch of about-to-show events |
517 | + to the server */ |
518 | +static void |
519 | +about_to_show_group_cb (GObject * proxy, GAsyncResult * res, gpointer userdata) |
520 | +{ |
521 | + GError * error = NULL; |
522 | + GQueue * showers = (GQueue *)userdata; |
523 | + GVariant * params = NULL; |
524 | + |
525 | + params = g_dbus_proxy_call_finish(G_DBUS_PROXY(proxy), res, &error); |
526 | + |
527 | + if (error != NULL) { |
528 | + g_warning("Unable to send about_to_show_group: %s", error->message); |
529 | + /* Note: we're just ensuring only the callback gets called */ |
530 | + g_error_free(error); |
531 | + error = NULL; |
532 | + } else { |
533 | + GVariant * updates = g_variant_get_child_value(params, 0); |
534 | + GVariantIter iter; |
535 | + |
536 | + /* Okay, so this is kinda interesting. We actually don't care which |
537 | + entries asked us to update the structure, as it's quite simply a |
538 | + single structure. So if we have any ask, we get the update once to |
539 | + avoid itterating through all the structures. */ |
540 | + if (g_variant_iter_init(&iter, updates) > 0) { |
541 | + about_to_show_t * first = (about_to_show_t *)g_queue_peek_head(showers); |
542 | + update_layout(first->client); |
543 | + } |
544 | + |
545 | + g_variant_unref(updates); |
546 | + g_variant_unref(params); |
547 | + params = NULL; |
548 | + } |
549 | + |
550 | + g_queue_foreach(showers, about_to_show_finish_pntr, GINT_TO_POINTER(FALSE)); |
551 | + g_queue_free(showers); |
552 | + |
553 | + return; |
554 | +} |
555 | + |
556 | +/* Check to see if this about to show entry has a callback associated |
557 | + with it */ |
558 | +static void |
559 | +about_to_show_idle_callbacks (gpointer data, gpointer user_data) |
560 | +{ |
561 | + about_to_show_t * abts = (about_to_show_t *)data; |
562 | + gboolean * got_callbacks = (gboolean *)user_data; |
563 | + |
564 | + if (abts->cb != NULL) { |
565 | + *got_callbacks = TRUE; |
566 | + } |
567 | + |
568 | + return; |
569 | +} |
570 | + |
571 | +/* Take the ID out of the about to show structure and put it into the |
572 | + variant builder */ |
573 | +static void |
574 | +about_to_show_idle_ids (gpointer data, gpointer user_data) |
575 | +{ |
576 | + about_to_show_t * abts = (about_to_show_t *)data; |
577 | + GVariantBuilder * builder = (GVariantBuilder *)user_data; |
578 | + |
579 | + g_variant_builder_add_value(builder, g_variant_new_int32(abts->id)); |
580 | + |
581 | + return; |
582 | +} |
583 | + |
584 | +/* Function that gets called with all the queued about_to_show messages, let's |
585 | + get these guys on the bus! */ |
586 | +static gboolean |
587 | +about_to_show_idle (gpointer user_data) |
588 | +{ |
589 | + DbusmenuClient * client = DBUSMENU_CLIENT(user_data); |
590 | + DbusmenuClientPrivate * priv = DBUSMENU_CLIENT_GET_PRIVATE(client); |
591 | + |
592 | + /* Reset our object global props and take ownership of these entries */ |
593 | + priv->about_to_show_idle = 0; |
594 | + GQueue * showers = priv->about_to_show_to_go; |
595 | + priv->about_to_show_to_go = NULL; |
596 | + |
597 | + /* Figure out if we've got any callbacks */ |
598 | + gboolean got_callbacks = FALSE; |
599 | + g_queue_foreach(showers, about_to_show_idle_callbacks, &got_callbacks); |
600 | + |
601 | + /* Build a list of the IDs */ |
602 | + GVariantBuilder idarray; |
603 | + g_variant_builder_init(&idarray, G_VARIANT_TYPE("ai")); |
604 | + g_queue_foreach(showers, about_to_show_idle_ids, &idarray); |
605 | + GVariant * ids = g_variant_builder_end(&idarray); |
606 | + |
607 | + /* Setup our callbacks */ |
608 | + GAsyncReadyCallback cb = NULL; |
609 | + gpointer cb_data = NULL; |
610 | + if (got_callbacks) { |
611 | + cb = about_to_show_group_cb; |
612 | + cb_data = showers; |
613 | + } else { |
614 | + g_queue_foreach(showers, about_to_show_finish_pntr, GINT_TO_POINTER(FALSE)); |
615 | + g_queue_free(showers); |
616 | + } |
617 | + |
618 | + /* Let's call it! */ |
619 | + g_dbus_proxy_call(priv->menuproxy, |
620 | + "AboutToShowGroup", |
621 | + g_variant_new_tuple(&ids, 1), |
622 | + G_DBUS_CALL_FLAGS_NONE, |
623 | + -1, /* timeout */ |
624 | + NULL, /* cancellable */ |
625 | + cb, |
626 | + cb_data); |
627 | + |
628 | + return FALSE; |
629 | +} |
630 | + |
631 | /* Reports errors and responds to update request that were a result |
632 | of sending the about to show signal. */ |
633 | static void |
634 | @@ -1604,18 +1995,7 @@ |
635 | g_variant_unref(params); |
636 | } |
637 | |
638 | - /* If we need to update, do that first. */ |
639 | - if (need_update) { |
640 | - update_layout(data->client); |
641 | - } |
642 | - |
643 | - if (data->cb != NULL) { |
644 | - data->cb(data->cb_data); |
645 | - } |
646 | - |
647 | - g_object_unref(data->client); |
648 | - g_free(data); |
649 | - |
650 | + about_to_show_finish(data, need_update); |
651 | return; |
652 | } |
653 | |
654 | @@ -1631,19 +2011,40 @@ |
655 | g_return_if_fail(priv != NULL); |
656 | |
657 | about_to_show_t * data = g_new0(about_to_show_t, 1); |
658 | + data->id = id; |
659 | data->client = client; |
660 | data->cb = cb; |
661 | data->cb_data = cb_data; |
662 | g_object_ref(client); |
663 | |
664 | - g_dbus_proxy_call(priv->menuproxy, |
665 | - "AboutToShow", |
666 | - g_variant_new("(i)", id), |
667 | - G_DBUS_CALL_FLAGS_NONE, |
668 | - -1, /* timeout */ |
669 | - NULL, /* cancellable */ |
670 | - about_to_show_cb, |
671 | - data); |
672 | + if (priv->group_events) { |
673 | + if (priv->about_to_show_to_go == NULL) { |
674 | + priv->about_to_show_to_go = g_queue_new(); |
675 | + } |
676 | + |
677 | + g_queue_push_tail(priv->about_to_show_to_go, data); |
678 | + |
679 | + if (priv->about_to_show_idle == 0) { |
680 | + priv->about_to_show_idle = g_idle_add(about_to_show_idle, client); |
681 | + } |
682 | + } else { |
683 | + /* If there's no callback we don't need this data, let's |
684 | + clean it up in a consistent way */ |
685 | + if (cb == NULL) { |
686 | + about_to_show_finish(data, FALSE); |
687 | + data = NULL; |
688 | + } |
689 | + |
690 | + g_dbus_proxy_call(priv->menuproxy, |
691 | + "AboutToShow", |
692 | + g_variant_new("(i)", id), |
693 | + G_DBUS_CALL_FLAGS_NONE, |
694 | + -1, /* timeout */ |
695 | + NULL, /* cancellable */ |
696 | + about_to_show_cb, |
697 | + data); |
698 | + } |
699 | + |
700 | return; |
701 | } |
702 | |
703 | |
704 | === modified file 'libdbusmenu-glib/client.h' |
705 | --- libdbusmenu-glib/client.h 2012-02-10 14:26:02 +0000 |
706 | +++ libdbusmenu-glib/client.h 2012-04-04 04:28:21 +0000 |
707 | @@ -105,6 +105,12 @@ |
708 | * String to access property #DbusmenuClient:text-direction |
709 | */ |
710 | #define DBUSMENU_CLIENT_PROP_TEXT_DIRECTION "text-direction" |
711 | +/** |
712 | + * DBUSMENU_CLIENT_PROP_GROUP_EVENTS: |
713 | + * |
714 | + * String to access property #DbusmenuClient:group-events |
715 | + */ |
716 | +#define DBUSMENU_CLIENT_PROP_GROUP_EVENTS "group-events" |
717 | |
718 | /** |
719 | * DBUSMENU_CLIENT_TYPES_DEFAULT: |
720 | |
721 | === modified file 'libdbusmenu-glib/dbus-menu.xml' |
722 | --- libdbusmenu-glib/dbus-menu.xml 2011-08-22 16:35:02 +0000 |
723 | +++ libdbusmenu-glib/dbus-menu.xml 2012-04-04 04:28:21 +0000 |
724 | @@ -326,6 +326,26 @@ |
725 | </arg> |
726 | </method> |
727 | |
728 | + <method name="EventGroup"> |
729 | + <dox:d> |
730 | + Used to pass a set of events as a single message for possibily several |
731 | + different menuitems. This is done to optimize DBus traffic. |
732 | + </dox:d> |
733 | + <arg type="a(isvu)" name="events" direction="in"> |
734 | + <dox:d> |
735 | + An array of all the events that should be passed. This tuple should |
736 | + match the parameters of the 'Event' signal. Which is roughly: |
737 | + id, eventID, data and timestamp. |
738 | + </dox:d> |
739 | + </arg> |
740 | + <arg type="ai" name="idErrors" direction="out"> |
741 | + <dox:d> |
742 | + I list of menuitem IDs that couldn't be found. If none of the ones |
743 | + in the list can be found, a DBus error is returned. |
744 | + </dox:d> |
745 | + </arg> |
746 | + </method> |
747 | + |
748 | <method name="AboutToShow"> |
749 | <dox:d> |
750 | This is called by the applet to notify the application that it is about |
751 | @@ -343,6 +363,32 @@ |
752 | </arg> |
753 | </method> |
754 | |
755 | + <method name="AboutToShowGroup"> |
756 | + <dox:d> |
757 | + A function to tell several menus being shown that they are about to |
758 | + be shown to the user. This is likely only useful for programitc purposes |
759 | + so while the return values are returned, in general, the singular function |
760 | + should be used in most user interacation scenarios. |
761 | + </dox:d> |
762 | + <arg type="ai" name="ids" direction="in"> |
763 | + <dox:d> |
764 | + The IDs of the menu items who's submenus are being shown. |
765 | + </dox:d> |
766 | + </arg> |
767 | + <arg type="ai" name="updatesNeeded" direction="out"> |
768 | + <dox:d> |
769 | + The IDs of the menus that need updates. Note: if no update information |
770 | + is needed the DBus message should set the no reply flag. |
771 | + </dox:d> |
772 | + </arg> |
773 | + <arg type="ai" name="idErrors" direction="out"> |
774 | + <dox:d> |
775 | + I list of menuitem IDs that couldn't be found. If none of the ones |
776 | + in the list can be found, a DBus error is returned. |
777 | + </dox:d> |
778 | + </arg> |
779 | + </method> |
780 | + |
781 | <!-- Signals --> |
782 | <signal name="ItemsPropertiesUpdated"> |
783 | <dox:d> |
784 | |
785 | === modified file 'libdbusmenu-glib/server.c' |
786 | --- libdbusmenu-glib/server.c 2012-03-28 14:07:30 +0000 |
787 | +++ libdbusmenu-glib/server.c 2012-04-04 04:28:21 +0000 |
788 | @@ -42,7 +42,7 @@ |
789 | |
790 | static void layout_update_signal (DbusmenuServer * server); |
791 | |
792 | -#define DBUSMENU_VERSION_NUMBER 2 |
793 | +#define DBUSMENU_VERSION_NUMBER 3 |
794 | #define DBUSMENU_INTERFACE "com.canonical.dbusmenu" |
795 | |
796 | /* Privates, I'll show you mine... */ |
797 | @@ -118,7 +118,9 @@ |
798 | METHOD_GET_PROPERTY, |
799 | METHOD_GET_PROPERTIES, |
800 | METHOD_EVENT, |
801 | + METHOD_EVENT_GROUP, |
802 | METHOD_ABOUT_TO_SHOW, |
803 | + METHOD_ABOUT_TO_SHOW_GROUP, |
804 | /* Counter, do not remove! */ |
805 | METHOD_COUNT |
806 | }; |
807 | @@ -191,9 +193,15 @@ |
808 | static void bus_event (DbusmenuServer * server, |
809 | GVariant * params, |
810 | GDBusMethodInvocation * invocation); |
811 | +static void bus_event_group (DbusmenuServer * server, |
812 | + GVariant * params, |
813 | + GDBusMethodInvocation * invocation); |
814 | static void bus_about_to_show (DbusmenuServer * server, |
815 | GVariant * params, |
816 | GDBusMethodInvocation * invocation); |
817 | +static void bus_about_to_show_group (DbusmenuServer * server, |
818 | + GVariant * params, |
819 | + GDBusMethodInvocation * invocation); |
820 | static void find_servers_cb (GDBusConnection * connection, |
821 | const gchar * sender, |
822 | const gchar * path, |
823 | @@ -358,9 +366,15 @@ |
824 | dbusmenu_method_table[METHOD_EVENT].interned_name = g_intern_static_string("Event"); |
825 | dbusmenu_method_table[METHOD_EVENT].func = bus_event; |
826 | |
827 | + dbusmenu_method_table[METHOD_EVENT_GROUP].interned_name = g_intern_static_string("EventGroup"); |
828 | + dbusmenu_method_table[METHOD_EVENT_GROUP].func = bus_event_group; |
829 | + |
830 | dbusmenu_method_table[METHOD_ABOUT_TO_SHOW].interned_name = g_intern_static_string("AboutToShow"); |
831 | dbusmenu_method_table[METHOD_ABOUT_TO_SHOW].func = bus_about_to_show; |
832 | |
833 | + dbusmenu_method_table[METHOD_ABOUT_TO_SHOW_GROUP].interned_name = g_intern_static_string("AboutToShowGroup"); |
834 | + dbusmenu_method_table[METHOD_ABOUT_TO_SHOW_GROUP].func = bus_about_to_show_group; |
835 | + |
836 | return; |
837 | } |
838 | |
839 | @@ -1633,6 +1647,28 @@ |
840 | return FALSE; |
841 | } |
842 | |
843 | +/* The core menu finding and doing the work part of the two |
844 | + event functions */ |
845 | +static gboolean |
846 | +bus_event_core (DbusmenuServer * server, gint32 id, gchar * event_type, GVariant * data, guint32 timestamp) |
847 | +{ |
848 | + DbusmenuMenuitem * mi = lookup_menuitem_by_id(server, id); |
849 | + |
850 | + if (mi == NULL) { |
851 | + return FALSE; |
852 | + } |
853 | + |
854 | + idle_event_t * event_data = g_new0(idle_event_t, 1); |
855 | + event_data->mi = g_object_ref(mi); |
856 | + event_data->eventid = g_strdup(event_type); |
857 | + event_data->timestamp = timestamp; |
858 | + event_data->variant = g_variant_ref(data); |
859 | + |
860 | + g_timeout_add(0, event_local_handler, event_data); |
861 | + |
862 | + return TRUE; |
863 | +} |
864 | + |
865 | /* Handles the events coming off of DBus */ |
866 | static void |
867 | bus_event (DbusmenuServer * server, GVariant * params, GDBusMethodInvocation * invocation) |
868 | @@ -1654,32 +1690,92 @@ |
869 | |
870 | g_variant_get(params, "(isvu)", &id, &etype, &data, &ts); |
871 | |
872 | - DbusmenuMenuitem * mi = lookup_menuitem_by_id(server, id); |
873 | - |
874 | - if (mi == NULL) { |
875 | + if (!bus_event_core(server, id, etype, data, ts)) { |
876 | g_dbus_method_invocation_return_error(invocation, |
877 | error_quark(), |
878 | INVALID_MENUITEM_ID, |
879 | "The ID supplied %d does not refer to a menu item we have", |
880 | id); |
881 | - g_free(etype); |
882 | - g_variant_unref(data); |
883 | - |
884 | } else { |
885 | - idle_event_t * event_data = g_new0(idle_event_t, 1); |
886 | - event_data->mi = g_object_ref(mi); |
887 | - event_data->eventid = etype; /* give away our allocation */ |
888 | - event_data->timestamp = ts; |
889 | - event_data->variant = data; /* give away our reference */ |
890 | - |
891 | - g_timeout_add(0, event_local_handler, event_data); |
892 | - |
893 | if (~g_dbus_message_get_flags (g_dbus_method_invocation_get_message (invocation)) & G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED) { |
894 | g_dbus_method_invocation_return_value(invocation, NULL); |
895 | } |
896 | } |
897 | |
898 | - return; |
899 | + g_free(etype); |
900 | + g_variant_unref(data); |
901 | + |
902 | + return; |
903 | +} |
904 | + |
905 | +/* Respond to the event group method that will send events to a |
906 | + variety of menuitems */ |
907 | +static void |
908 | +bus_event_group (DbusmenuServer * server, GVariant * params, GDBusMethodInvocation * invocation) |
909 | +{ |
910 | + DbusmenuServerPrivate * priv = DBUSMENU_SERVER_GET_PRIVATE(server); |
911 | + |
912 | + if (priv->root == NULL) { |
913 | + g_dbus_method_invocation_return_error(invocation, |
914 | + error_quark(), |
915 | + NO_VALID_LAYOUT, |
916 | + "There currently isn't a layout in this server"); |
917 | + return; |
918 | + } |
919 | + |
920 | + GVariant * events = g_variant_get_child_value(params, 0); |
921 | + gint32 id; |
922 | + gchar *etype; |
923 | + GVariant *data; |
924 | + guint32 ts; |
925 | + GVariantIter iter; |
926 | + GVariantBuilder builder; |
927 | + |
928 | + g_variant_iter_init(&iter, events); |
929 | + g_variant_builder_init(&builder, G_VARIANT_TYPE("ai")); |
930 | + gboolean gotone = FALSE; |
931 | + |
932 | + while (g_variant_iter_loop(&iter, "(isvu)", &id, &etype, &data, &ts)) { |
933 | + if (bus_event_core(server, id, etype, data, ts)) { |
934 | + gotone = TRUE; |
935 | + } else { |
936 | + g_variant_builder_add_value(&builder, g_variant_new_int32(id)); |
937 | + } |
938 | + } |
939 | + |
940 | + GVariant * errors = g_variant_builder_end(&builder); |
941 | + g_variant_ref_sink(errors); |
942 | + |
943 | + if (gotone) { |
944 | + if (~g_dbus_message_get_flags (g_dbus_method_invocation_get_message (invocation)) & G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED) { |
945 | + g_dbus_method_invocation_return_value(invocation, g_variant_new_tuple(&errors, 1)); |
946 | + } |
947 | + } else { |
948 | + gchar * ids = g_variant_print(errors, FALSE); |
949 | + g_dbus_method_invocation_return_error(invocation, |
950 | + error_quark(), |
951 | + INVALID_MENUITEM_ID, |
952 | + "The IDs supplied '%s' do not refer to any menu items we have", |
953 | + ids); |
954 | + g_free(ids); |
955 | + } |
956 | + |
957 | + g_variant_unref(errors); |
958 | + g_variant_unref(events); |
959 | + |
960 | + return; |
961 | +} |
962 | + |
963 | +/* Does the about-to-show in an idle loop so we don't block things */ |
964 | +/* NOTE: this only works so easily as we don't return the value, if we |
965 | + were to do that it would get more complex. */ |
966 | +static gboolean |
967 | +bus_about_to_show_idle (gpointer user_data) |
968 | +{ |
969 | + DbusmenuMenuitem * mi = DBUSMENU_MENUITEM(user_data); |
970 | + dbusmenu_menuitem_send_about_to_show(mi, NULL, NULL); |
971 | + g_object_unref(mi); |
972 | + return FALSE; |
973 | } |
974 | |
975 | /* Recieve the About To Show function. Pass it to our menu item. */ |
976 | @@ -1709,7 +1805,7 @@ |
977 | return; |
978 | } |
979 | |
980 | - dbusmenu_menuitem_send_about_to_show(mi, NULL, NULL); |
981 | + g_timeout_add(0, bus_about_to_show_idle, g_object_ref(mi)); |
982 | |
983 | /* GTK+ does not support about-to-show concept for now */ |
984 | g_dbus_method_invocation_return_value(invocation, |
985 | @@ -1717,6 +1813,69 @@ |
986 | return; |
987 | } |
988 | |
989 | +/* Handle the about to show on a set of menus and tell all of them that |
990 | + we love them */ |
991 | +static void |
992 | +bus_about_to_show_group (DbusmenuServer * server, GVariant * params, GDBusMethodInvocation * invocation) |
993 | +{ |
994 | + DbusmenuServerPrivate * priv = DBUSMENU_SERVER_GET_PRIVATE(server); |
995 | + |
996 | + if (priv->root == NULL) { |
997 | + g_dbus_method_invocation_return_error(invocation, |
998 | + error_quark(), |
999 | + NO_VALID_LAYOUT, |
1000 | + "There currently isn't a layout in this server"); |
1001 | + return; |
1002 | + } |
1003 | + |
1004 | + gint32 id; |
1005 | + GVariantIter iter; |
1006 | + GVariantBuilder builder; |
1007 | + |
1008 | + g_variant_iter_init(&iter, params); |
1009 | + g_variant_builder_init(&builder, G_VARIANT_TYPE("ai")); |
1010 | + gboolean gotone = FALSE; |
1011 | + |
1012 | + while (g_variant_iter_loop(&iter, "(i)", &id)) { |
1013 | + DbusmenuMenuitem * mi = lookup_menuitem_by_id(server, id); |
1014 | + if (mi != NULL) { |
1015 | + g_timeout_add(0, bus_about_to_show_idle, g_object_ref(mi)); |
1016 | + gotone = TRUE; |
1017 | + } else { |
1018 | + g_variant_builder_add_value(&builder, g_variant_new_int32(id)); |
1019 | + } |
1020 | + } |
1021 | + |
1022 | + GVariant * errors = g_variant_builder_end(&builder); |
1023 | + g_variant_ref_sink(errors); |
1024 | + |
1025 | + if (gotone) { |
1026 | + if (~g_dbus_message_get_flags (g_dbus_method_invocation_get_message (invocation)) & G_DBUS_MESSAGE_FLAGS_NO_REPLY_EXPECTED) { |
1027 | + GVariantBuilder tuple; |
1028 | + g_variant_builder_init(&tuple, G_VARIANT_TYPE_TUPLE); |
1029 | + |
1030 | + /* Updates needed */ |
1031 | + g_variant_builder_add_value(&tuple, g_variant_new_array(G_VARIANT_TYPE_INT32, NULL, 0)); |
1032 | + /* Errors */ |
1033 | + g_variant_builder_add_value(&tuple, errors); |
1034 | + |
1035 | + g_dbus_method_invocation_return_value(invocation, g_variant_builder_end(&tuple)); |
1036 | + } |
1037 | + } else { |
1038 | + gchar * ids = g_variant_print(errors, FALSE); |
1039 | + g_dbus_method_invocation_return_error(invocation, |
1040 | + error_quark(), |
1041 | + INVALID_MENUITEM_ID, |
1042 | + "The IDs supplied '%s' do not refer to any menu items we have", |
1043 | + ids); |
1044 | + g_free(ids); |
1045 | + } |
1046 | + |
1047 | + g_variant_unref(errors); |
1048 | + |
1049 | + return; |
1050 | +} |
1051 | + |
1052 | /* Public Interface */ |
1053 | /** |
1054 | dbusmenu_server_new: |
1055 | |
1056 | === modified file 'tests/Makefile.am' |
1057 | --- tests/Makefile.am 2012-04-02 14:25:13 +0000 |
1058 | +++ tests/Makefile.am 2012-04-04 04:28:21 +0000 |
1059 | @@ -8,6 +8,7 @@ |
1060 | TESTS = \ |
1061 | test-glib-objects-test \ |
1062 | test-glib-events \ |
1063 | + test-glib-events-nogroup \ |
1064 | test-glib-layout \ |
1065 | test-glib-properties \ |
1066 | test-glib-proxy \ |
1067 | @@ -45,6 +46,7 @@ |
1068 | test-glib-objects \ |
1069 | test-glib-events-client \ |
1070 | test-glib-events-server \ |
1071 | + test-glib-events-nogroup-client \ |
1072 | test-glib-layout-client \ |
1073 | test-glib-layout-server \ |
1074 | test-glib-properties-client \ |
1075 | @@ -200,6 +202,28 @@ |
1076 | ../libdbusmenu-glib/libdbusmenu-glib.la \ |
1077 | $(DBUSMENUGLIB_LIBS) |
1078 | |
1079 | +################################ |
1080 | +# Test Glib Events No Grouping |
1081 | +################################ |
1082 | + |
1083 | +test-glib-events-nogroup: test-glib-events-nogroup-client test-glib-events-server Makefile.am |
1084 | + @echo "#!/bin/bash" > $@ |
1085 | + @echo export UBUNTU_MENUPROXY="" >> $@ |
1086 | + @echo export G_DEBUG=fatal_criticals >> $@ |
1087 | + @echo $(DBUS_RUNNER) --task ./test-glib-events-nogroup-client --task-name Client --task ./test-glib-events-server --task-name Server --ignore-return >> $@ |
1088 | + @chmod +x $@ |
1089 | + |
1090 | +test_glib_events_nogroup_client_SOURCES = \ |
1091 | + test-glib-events-nogroup-client.c |
1092 | + |
1093 | +test_glib_events_nogroup_client_CFLAGS = \ |
1094 | + -I $(srcdir)/.. \ |
1095 | + $(DBUSMENUGLIB_CFLAGS) -Wall -Werror |
1096 | + |
1097 | +test_glib_events_nogroup_client_LDADD = \ |
1098 | + ../libdbusmenu-glib/libdbusmenu-glib.la \ |
1099 | + $(DBUSMENUGLIB_LIBS) |
1100 | + |
1101 | ###################### |
1102 | # Test JSON |
1103 | ###################### |
1104 | |
1105 | === added file 'tests/test-glib-events-nogroup-client.c' |
1106 | --- tests/test-glib-events-nogroup-client.c 1970-01-01 00:00:00 +0000 |
1107 | +++ tests/test-glib-events-nogroup-client.c 2012-04-04 04:28:21 +0000 |
1108 | @@ -0,0 +1,142 @@ |
1109 | +/* |
1110 | +A test for libdbusmenu to ensure its quality. |
1111 | + |
1112 | +Copyright 2009 Canonical Ltd. |
1113 | + |
1114 | +Authors: |
1115 | + Ted Gould <ted@canonical.com> |
1116 | + |
1117 | +This program is free software: you can redistribute it and/or modify it |
1118 | +under the terms of the GNU General Public License version 3, as published |
1119 | +by the Free Software Foundation. |
1120 | + |
1121 | +This program is distributed in the hope that it will be useful, but |
1122 | +WITHOUT ANY WARRANTY; without even the implied warranties of |
1123 | +MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR |
1124 | +PURPOSE. See the GNU General Public License for more details. |
1125 | + |
1126 | +You should have received a copy of the GNU General Public License along |
1127 | +with this program. If not, see <http://www.gnu.org/licenses/>. |
1128 | +*/ |
1129 | + |
1130 | +#include <glib.h> |
1131 | + |
1132 | +#include <libdbusmenu-glib/client.h> |
1133 | +#include <libdbusmenu-glib/menuitem.h> |
1134 | + |
1135 | +#include "test-glib-submenu.h" |
1136 | + |
1137 | +#define TIMESTAMP_VALUE 54 |
1138 | +#define DATA_VALUE 32 |
1139 | +#define USER_VALUE 76 |
1140 | + |
1141 | +static GMainLoop * mainloop = NULL; |
1142 | +static gboolean passed = TRUE; |
1143 | +static gboolean first = TRUE; |
1144 | + |
1145 | +static void |
1146 | +event_status (DbusmenuClient * client, DbusmenuMenuitem * item, gchar * name, GVariant * data, guint timestamp, GError * error, gpointer user_data) |
1147 | +{ |
1148 | + g_debug("Event status: %s", error == NULL ? "Sent" : "Error"); |
1149 | + |
1150 | + if (timestamp != TIMESTAMP_VALUE) { |
1151 | + g_debug("Timestamp value pass fail got: %d", timestamp); |
1152 | + passed = FALSE; |
1153 | + g_main_loop_quit(mainloop); |
1154 | + return; |
1155 | + } |
1156 | + |
1157 | + if (g_variant_get_int32(data) != DATA_VALUE) { |
1158 | + g_debug("Data value pass fail got: %d", g_variant_get_int32(g_variant_get_child_value(data, 0))); |
1159 | + passed = FALSE; |
1160 | + g_main_loop_quit(mainloop); |
1161 | + return; |
1162 | + } |
1163 | + |
1164 | + if (GPOINTER_TO_INT(user_data) != USER_VALUE) { |
1165 | + g_debug("User value pass fail got: %d", GPOINTER_TO_INT(user_data)); |
1166 | + passed = FALSE; |
1167 | + g_main_loop_quit(mainloop); |
1168 | + return; |
1169 | + } |
1170 | + |
1171 | + if (first && error != NULL) { |
1172 | + passed = FALSE; |
1173 | + g_debug("First signal back failed."); |
1174 | + g_main_loop_quit(mainloop); |
1175 | + return; |
1176 | + } |
1177 | + |
1178 | + if (!first && error == NULL) { |
1179 | + passed = FALSE; |
1180 | + g_debug("Second signal didn't fail."); |
1181 | + g_main_loop_quit(mainloop); |
1182 | + return; |
1183 | + } |
1184 | + |
1185 | + if (!first && error != NULL) { |
1186 | + g_debug("Second signal failed: pass."); |
1187 | + g_main_loop_quit(mainloop); |
1188 | + return; |
1189 | + } |
1190 | + |
1191 | + first = FALSE; |
1192 | + dbusmenu_menuitem_handle_event(item, "clicked", data, timestamp); |
1193 | + return; |
1194 | +} |
1195 | + |
1196 | +static void |
1197 | +layout_updated (DbusmenuClient * client, gpointer user_data) |
1198 | +{ |
1199 | + g_debug("Layout Updated"); |
1200 | + |
1201 | + DbusmenuMenuitem * menuroot = dbusmenu_client_get_root(client); |
1202 | + if (menuroot == NULL) { |
1203 | + g_debug("Root is NULL?"); |
1204 | + return; |
1205 | + } |
1206 | + |
1207 | + g_object_set(G_OBJECT(client), |
1208 | + DBUSMENU_CLIENT_PROP_GROUP_EVENTS, FALSE, |
1209 | + NULL); |
1210 | + |
1211 | + GVariant * data = g_variant_new_int32(DATA_VALUE); |
1212 | + dbusmenu_menuitem_handle_event(menuroot, "clicked", data, TIMESTAMP_VALUE); |
1213 | + |
1214 | + return; |
1215 | +} |
1216 | + |
1217 | +static gboolean |
1218 | +timer_func (gpointer data) |
1219 | +{ |
1220 | + g_debug("Death timer. Oops."); |
1221 | + passed = FALSE; |
1222 | + g_main_loop_quit(mainloop); |
1223 | + return FALSE; |
1224 | +} |
1225 | + |
1226 | +int |
1227 | +main (int argc, char ** argv) |
1228 | +{ |
1229 | + g_type_init(); |
1230 | + |
1231 | + DbusmenuClient * client = dbusmenu_client_new("org.dbusmenu.test", "/org/test"); |
1232 | + g_signal_connect(G_OBJECT(client), DBUSMENU_CLIENT_SIGNAL_LAYOUT_UPDATED, G_CALLBACK(layout_updated), NULL); |
1233 | + g_signal_connect(G_OBJECT(client), DBUSMENU_CLIENT_SIGNAL_EVENT_RESULT, G_CALLBACK(event_status), GINT_TO_POINTER(USER_VALUE)); |
1234 | + |
1235 | + g_timeout_add_seconds(5, timer_func, client); |
1236 | + |
1237 | + mainloop = g_main_loop_new(NULL, FALSE); |
1238 | + g_main_loop_run(mainloop); |
1239 | + |
1240 | + g_debug("Main loop complete"); |
1241 | + g_object_unref(G_OBJECT(client)); |
1242 | + |
1243 | + if (passed) { |
1244 | + g_debug("Quiting"); |
1245 | + return 0; |
1246 | + } else { |
1247 | + g_debug("Quiting as we're a failure"); |
1248 | + return 1; |
1249 | + } |
1250 | +} |
In broad strokes, this is pretty much what I had in mind, but the devil is in the details.
A few suggestions:
- use a GQueue instead of prepend- and-reversing a list
- the flag to enable/disable event-grouping is awkward. By my read, it gets set according to the remote version but can be manually changed by the user but may change again depending on when we get the version number from the remote? Also: I may want to group open/close but not activations. It might be better (not to mention vastly reducing the lines of code in this patch) to have an explicit API to send a message to a group as specified by the user? I guess it could take a GList of DbusmenuMenuItem instances? In the case that it's not supported, the flattening would be done on the client side. It's not a totally awesome API but it would be a lot less complicated and I'd be willing to do the collecting of like-queries in the HUD.
- the above API would also allow simplifying the dbus method signature since there would only be the possibility of a single event-type and a single timestamp (and only multiple IDs)
- when doing mass-dispatching of events I'm unlikely to care about errors. I wouldn't bother reporting them. In fact, the only user of this API (the HUD) will have set the "don't send reply" flag.
- it's very possible that I might send some events and then unref the client before returning to the mainloop (or by returning to the mainloop to find an event source with a higher priority than your idle that causes the unref).
- the /* Get the icon theme directories if available */ comment is intensely confusing (copy/paste error?)
- some of your use of GVariant is a bit odd (although you could argue a style of prefering to avoid varargs use):
- typically you use g_variant_new("()") for GDBus argument lists -- it helps to document the signature of the method call at the site of use
- g_variant_ builder_ add_value( x, g_variant_new_*()) is odd -- this is what g_variant_ builder_ add() is for
- creating a tuple builder, loading it up with 4 values, ending it and then packing it inside of another builder can be replaced with a single g_variant_ builder_ add (x, "(....)", ...) line
- glad to see you took care to preserve calling gdbus with NULL async callbacks in the don't-care case :)
- the receiving side looks good except for the questionable use of an idle/timeout(0). Why did you do that? You're already in a rather direct dispatch from the mainloop -- punting to another idle here gains you nothing except for additional complexity and may also introduce ordering issues (ie: the next dbus message to arrive may end up being processed before the idle for your group-of-events that arrived first).