Merge lp://qastaging/~bratsche/xsplash/configurable-signals into lp://qastaging/xsplash

Proposed by Cody Russell
Status: Merged
Merged at revision: not available
Proposed branch: lp://qastaging/~bratsche/xsplash/configurable-signals
Merge into: lp://qastaging/xsplash
Diff against target: None lines
To merge this branch: bzr merge lp://qastaging/~bratsche/xsplash/configurable-signals
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Review via email: mp+11368@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2009-09-02.

To post a comment you must log in.
Revision history for this message
Cody Russell (bratsche) wrote : Posted in a previous version of this proposal

Since alpha5 is done, I'm going to go ahead and propose merging this branch.

This branch removes hard-coded signal names. Applications (e.g., nautilus, gnome-panel, netbook-launcher, gnome-shell, xfce) would instead install a file to /etc/xsplash. The filename would be the name of the signal they emit to xsplash. The file won't be read by xsplash, it will only look for the filename.

So if gnome-panel sends a signal to xsplash that identifies itself as "gnome-panel", it should touch /etc/xsplash/gnome-panel when it is being installed.

Revision history for this message
Neil J. Patel (njpatel) wrote : Posted in a previous version of this proposal

Looks good, had a couple of points:

configure_signals:
 - I'd normally use slightly more higher-level API such as g_dir_open, g_dir_read_names and g_dir_close, as I think they protect you from some errors (as well as giving you some useful GError's if somethings gone wrong).
 - Speaking of g_dir_close, I don't see *d being closed :)

add_signal:
 - I'd use g_strcmp0 instead of strcmp as it'll protect you if name == NULL
 - g_slist_next just expands to signal->next, so maybe that looks cleaner (this is just style stuff, so ignore me if you want :)?

last block of code:
 - Generally, I'd s/strcmp/g_strcmp0 as mentioned before
 - If you find the matching string and you delete the link, you should also break out of the forloop as deleting a link half-way through an iteration could invalidate the list pointers (afaik)?.

review: Needs Fixing
66. By Cody Russell

Remove some old info from help output

Revision history for this message
David Barth (dbarth) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'build/autotools/libtool.m4'
2--- build/autotools/libtool.m4 2009-08-26 15:11:53 +0000
3+++ build/autotools/libtool.m4 2009-09-08 15:05:37 +0000
4@@ -4261,9 +4261,6 @@
5 openbsd*)
6 with_gnu_ld=no
7 ;;
8- linux* | k*bsd*-gnu)
9- _LT_TAGVAR(link_all_deplibs, $1)=no
10- ;;
11 esac
12
13 _LT_TAGVAR(ld_shlibs, $1)=yes
14
15=== modified file 'src/xsplash-dbus.xml'
16--- src/xsplash-dbus.xml 2009-08-06 11:24:01 +0000
17+++ src/xsplash-dbus.xml 2009-09-08 15:05:37 +0000
18@@ -2,6 +2,9 @@
19
20 <node name="/">
21 <interface name="com.ubuntu.BootCurtain">
22+ <method name="AddWaitSignal">
23+ <arg type="s" name="wait" direction="in" />
24+ </method>
25 <method name="SignalLoaded">
26 <arg type="s" name="app" direction="in" />
27 </method>
28
29=== modified file 'src/xsplash.c'
30--- src/xsplash.c 2009-09-02 14:04:26 +0000
31+++ src/xsplash.c 2009-09-08 15:05:37 +0000
32@@ -22,6 +22,7 @@
33 #include <string.h>
34 #include <unistd.h>
35 #include <pwd.h>
36+#include <dirent.h>
37
38 #include <gtk/gtk.h>
39 #include <gdk/gdkkeysyms.h>
40@@ -61,9 +62,6 @@
41
42 GdkWindow *cow;
43 GdkScreen *screen;
44-
45- gboolean nautilus_done;
46- gboolean panel_done;
47 };
48
49 enum {
50@@ -73,29 +71,6 @@
51 PROP_DBUS_PROXY
52 };
53
54-#define XSPLASH_DBUS_NAME "com.ubuntu.BootCurtain"
55-#define XSPLASH_DBUS_OBJECT "/com/ubuntu/BootCurtain"
56-
57-static gboolean gdm_session = FALSE;
58-static gchar *background_image = NULL;
59-static gchar *logo_image = NULL;
60-static gchar *throbber_image = NULL;
61-static guint throbber_frames = 50;
62-static gboolean ping_pong = FALSE;
63-static gboolean have_xcomposite = FALSE;
64-static gboolean is_composited = FALSE;
65-static gboolean redirected = FALSE;
66-
67-static GOptionEntry entries[] = {
68- { "gdm-session", 'g', 0, G_OPTION_ARG_NONE, &gdm_session, "Run in gdm session", NULL },
69- { "background", 'b', 0, G_OPTION_ARG_FILENAME, &background_image, "Filename for background image", NULL },
70- { "logo", 'l', 0, G_OPTION_ARG_FILENAME, &logo_image, "Filename for logo image", NULL },
71- { "throbber", 't', 0, G_OPTION_ARG_FILENAME, &throbber_image, "Filename for throbber image", NULL },
72- { "frames", 'f', 0, G_OPTION_ARG_INT, &throbber_frames, "Number of frames for the throbber (default 50)", NULL },
73- { "pingpong", 'p', 0, G_OPTION_ARG_NONE, &ping_pong, "Whether to reverse throbber directions", NULL },
74- { NULL }
75-};
76-
77 static AnimContext * anim_context_new (XsplashServer *server,
78 GtyTimeline *timeline,
79 gpointer id);
80@@ -123,6 +98,9 @@
81
82 static void xsplash_server_dispose (GObject *object);
83
84+gboolean xsplash_server_add_wait_signal (XsplashServer *server,
85+ gchar *waitfor,
86+ GError **error);
87 gboolean xsplash_server_signal_loaded (XsplashServer *server,
88 gchar *app,
89 GError **error);
90@@ -131,6 +109,65 @@
91 gdouble progress,
92 gpointer user_data);
93
94+static void add_signal (gchar *name);
95+static void xsplash_add_signal (const gchar *option_name,
96+ const gchar *value,
97+ gpointer data,
98+ GError **error);
99+
100+#define XSPLASH_DBUS_NAME "com.ubuntu.BootCurtain"
101+#define XSPLASH_DBUS_OBJECT "/com/ubuntu/BootCurtain"
102+
103+static gboolean gdm_session = FALSE;
104+static gchar *background_image = NULL;
105+static gchar *logo_image = NULL;
106+static gchar *throbber_image = NULL;
107+static guint throbber_frames = 50;
108+static gboolean ping_pong = FALSE;
109+static gboolean have_xcomposite = FALSE;
110+static gboolean is_composited = FALSE;
111+static gboolean redirected = FALSE;
112+static GSList *signal_list = NULL;
113+
114+static GOptionEntry entries[] = {
115+ {
116+ "gdm-session", 'g', 0,
117+ G_OPTION_ARG_NONE, &gdm_session,
118+ "Run in gdm session", NULL
119+ },
120+ {
121+ "background", 'b', 0,
122+ G_OPTION_ARG_FILENAME, &background_image,
123+ "Filename for background image", NULL
124+ },
125+ {
126+ "logo", 'l', 0,
127+ G_OPTION_ARG_FILENAME, &logo_image,
128+ "Filename for logo image", NULL
129+ },
130+ {
131+ "throbber", 't', 0,
132+ G_OPTION_ARG_FILENAME, &throbber_image,
133+ "Filename for throbber image", NULL
134+ },
135+ {
136+ "frames", 'f', 0,
137+ G_OPTION_ARG_INT, &throbber_frames,
138+ "Number of frames for the throbber (default 50)", NULL
139+ },
140+ {
141+ "pingpong", 'p', 0,
142+ G_OPTION_ARG_NONE, &ping_pong,
143+ "Whether to reverse throbber directions", NULL
144+ },
145+ {
146+ "add-signal", 's', 0,
147+ G_OPTION_ARG_CALLBACK, (GOptionArgFunc) xsplash_add_signal,
148+ "Add a signal to listen for. Alternately signals can be added to the XSPLASH_WAITFOR environment.", NULL
149+ },
150+ { NULL }
151+};
152+
153 #include "dbus-xsplash-server.h"
154
155 G_DEFINE_TYPE (XsplashServer, xsplash_server, G_TYPE_OBJECT);
156@@ -145,7 +182,7 @@
157
158 g_type_class_add_private (class, sizeof (XsplashServerPrivate));
159
160- object_class->dispose = xsplash_server_dispose;
161+ object_class->dispose = xsplash_server_dispose;
162 object_class->set_property = set_property;
163 object_class->get_property = get_property;
164
165@@ -219,8 +256,6 @@
166 g_return_if_reached ();
167 break;
168 }
169-
170- return;
171 }
172
173 static void
174@@ -245,8 +280,6 @@
175 g_return_if_reached ();
176 break;
177 }
178-
179- return;
180 }
181
182 static GdkPixbuf *
183@@ -292,7 +325,7 @@
184 }
185 else
186 {
187- g_debug (" ** %dx%d is too small, using last good size.\n", widths[i], heights[i]);
188+ g_debug (" ** %dx%d is too small, using last good size.", widths[i], heights[i]);
189 break;
190 }
191 }
192@@ -301,7 +334,7 @@
193
194 ret = g_strdup_printf (DATADIR "/images/xsplash/bg_%dx%d.jpg", widths[last_good], heights[last_good]);
195
196- g_debug (" ** filename: %s\n", ret);
197+ g_debug (" ** filename: %s", ret);
198
199 return ret;
200 }
201@@ -331,7 +364,7 @@
202
203 if (throbber_image != NULL)
204 {
205- g_debug ("get_throbber_filename(): user provided a throbber on the command line; using that\n");
206+ g_debug ("get_throbber_filename(): user provided a throbber on the command line; using that");
207
208 return g_strdup (throbber_image);
209 }
210@@ -342,7 +375,7 @@
211 get_filename_size_modifier (width));
212
213 g_debug (" ** Chose `%s'", get_filename_size_modifier (width));
214- g_debug (" ** throbber filename is: %s\n", ret);
215+ g_debug (" ** throbber filename is: %s", ret);
216
217 return ret;
218 }
219@@ -359,7 +392,7 @@
220
221 if (logo_image != NULL)
222 {
223- g_debug ("get_logo_filename(): user provided a logo on the command line; using that\n");
224+ g_debug ("get_logo_filename(): user provided a logo on the command line; using that");
225
226 return g_strdup (logo_image);
227 }
228@@ -370,7 +403,7 @@
229 get_filename_size_modifier (width));
230
231 g_debug (" ** Chose `%s'", get_filename_size_modifier (width));
232- g_debug (" ** logo filename is: %s\n", ret);
233+ g_debug (" ** logo filename is: %s", ret);
234
235 return ret;
236 }
237@@ -743,6 +776,37 @@
238 return FALSE;
239 }
240
241+static void
242+add_signal (gchar *name)
243+{
244+ GSList *tmp = NULL;
245+
246+ if ((strcmp (name, ".") == 0) || strcmp (name, "..") == 0)
247+ return;
248+
249+ g_debug ("adding signal `%s'", name);
250+
251+ for (tmp = signal_list; tmp != NULL; tmp = g_slist_next (signal_list))
252+ {
253+ if (strcmp (tmp->data, name) == 0)
254+ {
255+ g_debug (" ** that signal is already being listened for; skipping");
256+ return;
257+ }
258+ }
259+
260+ signal_list = g_slist_prepend (signal_list, g_strdup (name));
261+}
262+
263+static void
264+xsplash_add_signal (const gchar *option_name,
265+ const gchar *value,
266+ gpointer data,
267+ GError **error)
268+{
269+ add_signal (g_strdup (value));
270+}
271+
272 void
273 sig_handler (int signum)
274 {
275@@ -808,6 +872,7 @@
276 g_debug ("throbber_image = %s", throbber_image);
277
278 system_bus = dbus_g_bus_get (DBUS_BUS_SYSTEM, NULL);
279+
280 bus_proxy = dbus_g_proxy_new_for_name (system_bus,
281 DBUS_SERVICE_DBUS,
282 DBUS_PATH_DBUS,
283@@ -850,16 +915,36 @@
284 if (throbber_image != NULL)
285 g_free (throbber_image);
286
287+ if (signal_list != NULL)
288+ {
289+ g_slist_foreach (signal_list, (GFunc)g_free, NULL);
290+ g_slist_free (signal_list);
291+ signal_list = NULL;
292+ }
293+
294 return 0;
295 }
296
297 gboolean
298+xsplash_server_add_wait_signal (XsplashServer *server,
299+ gchar *waitfor,
300+ GError **error)
301+{
302+ g_debug ("received a new signal to wait for: %s", waitfor);
303+
304+ if (waitfor)
305+ {
306+ add_signal (waitfor);
307+ }
308+
309+ return TRUE;
310+}
311+
312+gboolean
313 xsplash_server_signal_loaded (XsplashServer *server,
314 gchar *app,
315 GError **error)
316 {
317- XsplashServerPrivate *priv = XSPLASH_SERVER_GET_PRIVATE (server);
318-
319 g_debug ("received signal: %s", app);
320
321 if (gdm_session)
322@@ -869,19 +954,24 @@
323 }
324 else
325 {
326- if (strcmp (app, "nautilus") == 0)
327- {
328- priv->nautilus_done = TRUE;
329- }
330- else if (strcmp (app, "gnome-panel") == 0)
331- {
332- priv->panel_done = TRUE;
333- }
334-
335- if (priv->nautilus_done && priv->panel_done)
336- {
337- g_debug ("received both \"nautilus\" and \"panel\" signals: fading out");
338- begin_fade (server, TRUE);
339+ GSList *l = NULL;
340+
341+ /* Remove this signal if it's in our list; if the list is empty, begin fading */
342+ for (l = signal_list; l != NULL; l = l->next)
343+ {
344+ if (strcmp (app, (gchar *)l->data) == 0)
345+ {
346+ g_debug ("received signal %s", (gchar*)l->data);
347+
348+ g_free (l->data);
349+ signal_list = g_slist_delete_link (signal_list, l);
350+
351+ /* There are no more signals we are listening for */
352+ if (signal_list == NULL)
353+ {
354+ begin_fade (server, TRUE);
355+ }
356+ }
357 }
358 }
359

Subscribers

People subscribed via source and target branches