Merge lp:~jamesh/whoopsie-preferences/bug-1720331 into lp:whoopsie-preferences

Proposed by James Henstridge
Status: Merged
Merged at revision: 72
Proposed branch: lp:~jamesh/whoopsie-preferences/bug-1720331
Merge into: lp:whoopsie-preferences
Diff against target: 81 lines (+27/-13)
3 files modified
configure.ac (+1/-1)
debian/changelog (+14/-0)
src/whoopsie-preferences.c (+12/-12)
To merge this branch: bzr merge lp:~jamesh/whoopsie-preferences/bug-1720331
Reviewer Review Type Date Requested Status
Daisy Pluckers Pending
Review via email: mp+332105@code.launchpad.net

Commit message

Don't export object on the bus until it has been fully constructed.

Description of the change

Don't export object on the bus until it has been fully constructed.

As well as ensuring that the WhoopsiePreferences object is usable as soon as it attaches to the bus, it also silences a PropertiesChanged signal broadcast during startup as whoopsie-preferences reads the current system configuration.

This helps avoid confusion when whoopsie-preferences is activated by one of the Set method calls. For example, the following chain of events:

 1. C->S SetReportCrashes(true) method call
 2. dbus-daemon activates whoopsie-preferences
 3. S->C PropertiesChanged(ReportCrashes=true) signal with current state
 4. S->C SetReportCrashes method return message
 5. S->C PropertiesChanged(ReportCrashes=false) signal with new state

Here, the first signal broadcast at (3) is unwanted, and can confuse the client into thinking that its change has been ignored or some other client has changed the state.

By setting the initial values for the properties before exporting on the bus, the initial signal broadcast is avoided.

To post a comment you must log in.
Revision history for this message
James Henstridge (jamesh) wrote :

The difference can be demonstrated by running the following command in one terminal:

    sudo dbus-monitor --system 'interface=com.ubuntu.WhoopsiePreferences' 'interface=org.freedesktop.DBus.Properties,arg0=com.ubuntu.WhoopsiePreferences'

And then in another terminal compare starting the old version of whoopsie-preferences vs. the version from this branch. The spurious signal is gone.

Using d-feet to call SetReportCrashes shows that PropertiesChanged is still sent when a change actually happens though.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configure.ac'
2--- configure.ac 2014-04-01 11:31:55 +0000
3+++ configure.ac 2017-10-11 10:57:52 +0000
4@@ -1,5 +1,5 @@
5 AC_INIT([whoopsie-preferences],
6- [0.11],
7+ [0.19],
8 [http://launchpad.net/whoopsie-preferences])
9
10 m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
11
12=== modified file 'debian/changelog'
13--- debian/changelog 2015-09-10 14:00:51 +0000
14+++ debian/changelog 2017-10-11 10:57:52 +0000
15@@ -1,3 +1,17 @@
16+whoopsie-preferences (0.19) artful; urgency=medium
17+
18+ * Don't export object on the bus until it has been fully constructed.
19+ (LP: #1720331)
20+
21+ -- James Henstridge <james.henstridge@canonical.com> Wed, 11 Oct 2017 18:20:35 +0800
22+
23+whoopsie-preferences (0.18build1) artful; urgency=medium
24+
25+ * No-change rebuild to pick up -fPIE compiler default in static
26+ libraries
27+
28+ -- Steve Langasek <steve.langasek@ubuntu.com> Fri, 21 Apr 2017 20:59:50 +0000
29+
30 whoopsie-preferences (0.18) wily; urgency=medium
31
32 * Truncate the file manually if update-rc.d fails (e.g. on an r/o phone).
33
34=== modified file 'src/whoopsie-preferences.c'
35--- src/whoopsie-preferences.c 2015-09-10 13:19:51 +0000
36+++ src/whoopsie-preferences.c 2017-10-11 10:57:52 +0000
37@@ -487,18 +487,6 @@
38 WhoopsiePreferences* interface;
39 GError* error = NULL;
40
41- interface = whoopsie_preferences_skeleton_new ();
42- if (!g_dbus_interface_skeleton_export (
43- G_DBUS_INTERFACE_SKELETON (interface),
44- connection,
45- "/com/ubuntu/WhoopsiePreferences", &error)) {
46-
47- g_print ("Could not export path: %s\n", error->message);
48- g_error_free (error);
49- g_main_loop_quit (loop);
50- return;
51- }
52-
53 authority = polkit_authority_get_sync (NULL, &error);
54 if (authority == NULL) {
55 g_print ("Could not get authority: %s\n", error->message);
56@@ -509,6 +497,7 @@
57 loop_shutdown = g_timeout_add_seconds (60, (GSourceFunc) g_main_loop_quit,
58 loop);
59
60+ interface = whoopsie_preferences_skeleton_new ();
61 g_signal_connect (interface, "notify::report-crashes",
62 G_CALLBACK (whoopsie_preferences_changed),
63 "report_crashes");
64@@ -535,6 +524,17 @@
65 monitor_autoreport_file (interface);
66 whoopsie_preferences_load (interface);
67 whoopsie_preferences_dbus_notify (interface);
68+
69+ if (!g_dbus_interface_skeleton_export (
70+ G_DBUS_INTERFACE_SKELETON (interface),
71+ connection,
72+ "/com/ubuntu/WhoopsiePreferences", &error)) {
73+
74+ g_print ("Could not export path: %s\n", error->message);
75+ g_error_free (error);
76+ g_main_loop_quit (loop);
77+ return;
78+ }
79 }
80
81 static void

Subscribers

People subscribed via source and target branches