Merge lp:~unity-team/unity/unity.fix-757588 into lp:unity

Proposed by Mirco Müller on 2011-04-12
Status: Merged
Approved by: Mirco Müller on 2011-04-13
Approved revision: 1117
Merged at revision: 1121
Proposed branch: lp:~unity-team/unity/unity.fix-757588
Merge into: lp:unity
Diff against target: 461 lines (+148/-70)
14 files modified
src/Launcher.cpp (+35/-20)
src/Launcher.h (+2/-0)
src/PanelHomeButton.cpp (+9/-6)
src/PanelMenuView.cpp (+9/-3)
src/PlacesController.cpp (+19/-7)
src/PlacesController.h (+2/-0)
src/PlacesHomeView.cpp (+9/-3)
src/PlacesHomeView.h (+2/-0)
src/PlacesSearchBar.cpp (+9/-4)
src/PlacesSearchBar.h (+2/-0)
src/PlacesView.cpp (+22/-12)
src/PlacesView.h (+2/-0)
src/unityshell.cpp (+24/-14)
src/unityshell.h (+2/-1)
To merge this branch: bzr merge lp:~unity-team/unity/unity.fix-757588
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) 2011-04-12 Approve on 2011-04-13
Review via email: mp+57332@code.launchpad.net

Description of the change

Correctly unregister all registered interests with the ubus-server in a class' destructor.

To post a comment you must log in.

Great work. Can you maybe use the G_N_ELEMENTS(array) macro instead of the *_MAX_UBUS_HANDLES constants? That would make the code slightly more robust.

review: Needs Information
Mirco Müller (macslow) wrote :

Done.

1117. By Mirco Müller on 2011-04-13

Make use of G_N_ELEMENTS-macro to harden ubus-handles juggling

Love it!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Launcher.cpp'
2--- src/Launcher.cpp 2011-04-12 05:20:12 +0000
3+++ src/Launcher.cpp 2011-04-13 07:50:54 +0000
4@@ -419,26 +419,35 @@
5 _drag_window = NULL;
6 _offscreen_drag_texture = nux::GetThreadGLDeviceFactory()->CreateSystemCapableDeviceTexture (2, 2, 1, nux::BITFMT_R8G8B8A8);
7 _offscreen_progress_texture = nux::GetThreadGLDeviceFactory()->CreateSystemCapableDeviceTexture (2, 2, 1, nux::BITFMT_R8G8B8A8);
8-
9+
10+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
11+ _ubus_handles[i] = 0;
12+
13 UBusServer *ubus = ubus_server_get_default ();
14- ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_SHOWN,
15- (UBusCallback)&Launcher::OnPlaceViewShown,
16- this);
17- ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_HIDDEN,
18- (UBusCallback)&Launcher::OnPlaceViewHidden,
19- this);
20-
21- ubus_server_register_interest (ubus, UBUS_HOME_BUTTON_BFB_UPDATE,
22- (UBusCallback)&Launcher::OnBFBUpdate,
23- this);
24-
25- ubus_server_register_interest (ubus, UBUS_LAUNCHER_ACTION_DONE,
26- (UBusCallback)&Launcher::OnActionDone,
27- this);
28-
29- ubus_server_register_interest (ubus, UBUS_HOME_BUTTON_BFB_DND_ENTER,
30- (UBusCallback)&Launcher::OnBFBDndEnter,
31- this);
32+ _ubus_handles[0] = ubus_server_register_interest (ubus,
33+ UBUS_PLACE_VIEW_SHOWN,
34+ (UBusCallback) &Launcher::OnPlaceViewShown,
35+ this);
36+
37+ _ubus_handles[1] = ubus_server_register_interest (ubus,
38+ UBUS_PLACE_VIEW_HIDDEN,
39+ (UBusCallback)&Launcher::OnPlaceViewHidden,
40+ this);
41+
42+ _ubus_handles[2] = ubus_server_register_interest (ubus,
43+ UBUS_HOME_BUTTON_BFB_UPDATE,
44+ (UBusCallback) &Launcher::OnBFBUpdate,
45+ this);
46+
47+ _ubus_handles[3] = ubus_server_register_interest (ubus,
48+ UBUS_LAUNCHER_ACTION_DONE,
49+ (UBusCallback) &Launcher::OnActionDone,
50+ this);
51+
52+ _ubus_handles[4] = ubus_server_register_interest (ubus,
53+ UBUS_HOME_BUTTON_BFB_DND_ENTER,
54+ (UBusCallback) &Launcher::OnBFBDndEnter,
55+ this);
56
57 _dbus_owner = g_bus_own_name (G_BUS_TYPE_SESSION,
58 S_DBUS_NAME,
59@@ -554,7 +563,13 @@
60
61 if (_launcher_animation_timeout > 0)
62 g_source_remove (_launcher_animation_timeout);
63-
64+
65+ UBusServer* ubus = ubus_server_get_default ();
66+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
67+ {
68+ if (_ubus_handles[i] != 0)
69+ ubus_server_unregister_interest (ubus, _ubus_handles[i]);
70+ }
71 }
72
73 /* Introspection */
74
75=== modified file 'src/Launcher.h'
76--- src/Launcher.h 2011-04-11 09:23:51 +0000
77+++ src/Launcher.h 2011-04-13 07:50:54 +0000
78@@ -556,6 +556,8 @@
79 sigc::connection _on_drag_finish_connection;
80
81 GSettings *_settings;
82+
83+ guint _ubus_handles[5];
84 };
85
86 #endif // LAUNCHER_H
87
88=== modified file 'src/PanelHomeButton.cpp'
89--- src/PanelHomeButton.cpp 2011-04-09 06:17:28 +0000
90+++ src/PanelHomeButton.cpp 2011-04-13 07:50:54 +0000
91@@ -39,7 +39,8 @@
92 NUX_IMPLEMENT_OBJECT_TYPE (PanelHomeButton);
93
94 PanelHomeButton::PanelHomeButton ()
95-: TextureArea (NUX_TRACKER_LOCATION)
96+: TextureArea (NUX_TRACKER_LOCATION),
97+ _urgent_interest (0)
98 {
99 _urgent_count = 0;
100 _button_width = 66;
101@@ -55,10 +56,10 @@
102 _theme_changed_id = g_signal_connect (gtk_icon_theme_get_default (), "changed",
103 G_CALLBACK (PanelHomeButton::OnIconThemeChanged), this);
104
105- UBusServer *ubus = ubus_server_get_default ();
106- _urgent_interest = ubus_server_register_interest (ubus, UBUS_LAUNCHER_ICON_URGENT_CHANGED,
107- (UBusCallback)&PanelHomeButton::OnLauncherIconUrgentChanged,
108- this);
109+ _urgent_interest = ubus_server_register_interest (ubus_server_get_default (),
110+ UBUS_LAUNCHER_ICON_URGENT_CHANGED,
111+ (UBusCallback) &PanelHomeButton::OnLauncherIconUrgentChanged,
112+ this);
113
114 Refresh ();
115
116@@ -70,7 +71,9 @@
117 if (_theme_changed_id)
118 g_signal_handler_disconnect (gtk_icon_theme_get_default (), _theme_changed_id);
119
120- ubus_server_unregister_interest (ubus_server_get_default (), _urgent_interest);
121+ if (_urgent_interest != 0)
122+ ubus_server_unregister_interest (ubus_server_get_default (),
123+ _urgent_interest);
124 }
125
126 void
127
128=== modified file 'src/PanelMenuView.cpp'
129--- src/PanelMenuView.cpp 2011-04-10 22:54:12 +0000
130+++ src/PanelMenuView.cpp 2011-04-13 07:50:54 +0000
131@@ -72,7 +72,9 @@
132 _we_control_active (false),
133 _monitor (0),
134 _active_xid (0),
135- _active_moved_id (0)
136+ _active_moved_id (0),
137+ _place_shown_interest (0),
138+ _place_hidden_interest (0)
139 {
140 WindowManager *win_manager;
141
142@@ -169,8 +171,12 @@
143 _window_buttons->UnReference ();
144 _panel_titlebar_grab_area->UnReference ();
145
146- ubus_server_unregister_interest (ubus_server_get_default (), _place_shown_interest);
147- ubus_server_unregister_interest (ubus_server_get_default (), _place_hidden_interest);
148+ UBusServer* ubus = ubus_server_get_default ();
149+ if (_place_shown_interest != 0)
150+ ubus_server_unregister_interest (ubus, _place_shown_interest);
151+
152+ if (_place_hidden_interest != 0)
153+ ubus_server_unregister_interest (ubus, _place_hidden_interest);
154 }
155
156 void
157
158=== modified file 'src/PlacesController.cpp'
159--- src/PlacesController.cpp 2011-04-09 05:53:28 +0000
160+++ src/PlacesController.cpp 2011-04-13 07:50:54 +0000
161@@ -43,15 +43,20 @@
162 _timeline_id (0)
163 {
164 _need_show = false;
165-
166+
167+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
168+ _ubus_handles[i] = 0;
169+
170 // register interest with ubus so that we get activation messages
171 UBusServer *ubus = ubus_server_get_default ();
172- ubus_server_register_interest (ubus, UBUS_DASH_EXTERNAL_ACTIVATION,
173- (UBusCallback)&PlacesController::ExternalActivation,
174- this);
175- ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_CLOSE_REQUEST,
176- (UBusCallback)&PlacesController::CloseRequest,
177- this);
178+ _ubus_handles[0] = ubus_server_register_interest (ubus,
179+ UBUS_DASH_EXTERNAL_ACTIVATION,
180+ (UBusCallback) &PlacesController::ExternalActivation,
181+ this);
182+ _ubus_handles[1] = ubus_server_register_interest (ubus,
183+ UBUS_PLACE_VIEW_CLOSE_REQUEST,
184+ (UBusCallback) &PlacesController::CloseRequest,
185+ this);
186
187 _factory = PlaceFactory::GetDefault ();
188
189@@ -95,6 +100,13 @@
190 PlacesController::~PlacesController ()
191 {
192 _window->UnReference ();
193+
194+ UBusServer* ubus = ubus_server_get_default ();
195+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
196+ {
197+ if (_ubus_handles[i] != 0)
198+ ubus_server_unregister_interest (ubus, _ubus_handles[i]);
199+ }
200 }
201
202 void
203
204=== modified file 'src/PlacesController.h'
205--- src/PlacesController.h 2011-03-24 00:35:26 +0000
206+++ src/PlacesController.h 2011-04-13 07:50:54 +0000
207@@ -82,6 +82,8 @@
208 GdkRectangle _monitor_rect;
209
210 bool _need_show;
211+
212+ guint _ubus_handles[2];
213 };
214
215 #endif // PLACES_CONTROLLER_H
216
217=== modified file 'src/PlacesHomeView.cpp'
218--- src/PlacesHomeView.cpp 2011-03-31 16:15:21 +0000
219+++ src/PlacesHomeView.cpp 2011-04-13 07:50:54 +0000
220@@ -83,6 +83,7 @@
221 };
222
223 PlacesHomeView::PlacesHomeView ()
224+: _ubus_handle (0)
225 {
226 PlacesStyle *style = PlacesStyle::GetDefault ();
227
228@@ -131,9 +132,11 @@
229 (GConfClientNotifyFunc)OnKeyChanged,
230 this,
231 NULL, NULL);
232-
233- UBusServer *ubus = ubus_server_get_default ();
234- ubus_server_register_interest (ubus, UBUS_DASH_EXTERNAL_ACTIVATION, (UBusCallback)&PlacesHomeView::DashVisible, this);
235+
236+ _ubus_handle = ubus_server_register_interest (ubus_server_get_default (),
237+ UBUS_DASH_EXTERNAL_ACTIVATION,
238+ (UBusCallback) &PlacesHomeView::DashVisible,
239+ this);
240
241 //In case the GConf key is invalid (e.g. when an app was uninstalled), we
242 //rely on a fallback "whitelist" mechanism instead of showing nothing at all
243@@ -168,6 +171,9 @@
244 PlacesHomeView::~PlacesHomeView ()
245 {
246 g_object_unref (_client);
247+
248+ if (_ubus_handle != 0)
249+ ubus_server_unregister_interest (ubus_server_get_default (), _ubus_handle);
250 }
251
252 void
253
254=== modified file 'src/PlacesHomeView.h'
255--- src/PlacesHomeView.h 2011-03-14 01:42:22 +0000
256+++ src/PlacesHomeView.h 2011-04-13 07:50:54 +0000
257@@ -66,6 +66,8 @@
258 std::vector<std::string> _photo_alternatives;
259 std::vector<std::string> _email_alternatives;
260 std::vector<std::string> _music_alternatives;
261+
262+ guint _ubus_handle;
263 };
264
265
266
267=== modified file 'src/PlacesSearchBar.cpp'
268--- src/PlacesSearchBar.cpp 2011-04-10 22:12:46 +0000
269+++ src/PlacesSearchBar.cpp 2011-04-13 07:50:54 +0000
270@@ -50,7 +50,8 @@
271 PlacesSearchBar::PlacesSearchBar (NUX_FILE_LINE_DECL)
272 : View (NUX_FILE_LINE_PARAM),
273 _entry (NULL),
274- _live_search_timeout (0)
275+ _live_search_timeout (0),
276+ _ubus_handle (0)
277 {
278 PlacesStyle *style = PlacesStyle::GetDefault ();
279 nux::BaseTexture *icon = style->GetSearchMagnifyIcon ();
280@@ -104,9 +105,10 @@
281
282 _cursor_moved_conn = _pango_entry->cursor_moved.connect (sigc::mem_fun (this, &PlacesSearchBar::OnLayeredLayoutQueueDraw));
283
284- ubus_server_register_interest (ubus_server_get_default (), UBUS_PLACE_VIEW_HIDDEN,
285- (UBusCallback)PlacesSearchBar::OnPlacesClosed, this);
286-
287+ _ubus_handle = ubus_server_register_interest (ubus_server_get_default (),
288+ UBUS_PLACE_VIEW_HIDDEN,
289+ (UBusCallback) PlacesSearchBar::OnPlacesClosed,
290+ this);
291 }
292
293 PlacesSearchBar::~PlacesSearchBar ()
294@@ -126,6 +128,9 @@
295 _combo_changed_conn.disconnect ();
296 _menu_conn.disconnect ();
297 _cursor_moved_conn.disconnect ();
298+
299+ if (_ubus_handle != 0)
300+ ubus_server_unregister_interest (ubus_server_get_default (), _ubus_handle);
301 }
302
303 const gchar* PlacesSearchBar::GetName ()
304
305=== modified file 'src/PlacesSearchBar.h'
306--- src/PlacesSearchBar.h 2011-03-31 20:38:49 +0000
307+++ src/PlacesSearchBar.h 2011-04-13 07:50:54 +0000
308@@ -107,6 +107,8 @@
309 friend class PlacesView;
310 PlacesSearchBarSpinner *_spinner;
311 nux::ComboBoxSimple *_combo;
312+
313+ guint _ubus_handle;
314 };
315
316 #endif
317
318=== modified file 'src/PlacesView.cpp'
319--- src/PlacesView.cpp 2011-04-10 19:19:50 +0000
320+++ src/PlacesView.cpp 2011-04-13 07:50:54 +0000
321@@ -114,20 +114,23 @@
322 _bg_layer = new nux::ColorLayer (nux::Color (0.0f, 0.0f, 0.0f, 0.9f), true, rop);
323 }
324
325+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
326+ _ubus_handles[i] = 0;
327+
328 // Register for all the events
329 UBusServer *ubus = ubus_server_get_default ();
330- ubus_server_register_interest (ubus, UBUS_PLACE_ENTRY_ACTIVATE_REQUEST,
331- (UBusCallback)place_entry_activate_request,
332- this);
333- ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_CLOSE_REQUEST,
334- (UBusCallback)&PlacesView::CloseRequest,
335- this);
336- ubus_server_register_interest (ubus, UBUS_PLACE_TILE_ACTIVATE_REQUEST,
337- (UBusCallback)&PlacesView::OnResultActivated,
338- this);
339- ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_QUEUE_DRAW,
340- (UBusCallback)&PlacesView::OnPlaceViewQueueDrawNeeded,
341- this);
342+ _ubus_handles[0] = ubus_server_register_interest (ubus, UBUS_PLACE_ENTRY_ACTIVATE_REQUEST,
343+ (UBusCallback)place_entry_activate_request,
344+ this);
345+ _ubus_handles[1] = ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_CLOSE_REQUEST,
346+ (UBusCallback)&PlacesView::CloseRequest,
347+ this);
348+ _ubus_handles[2] = ubus_server_register_interest (ubus, UBUS_PLACE_TILE_ACTIVATE_REQUEST,
349+ (UBusCallback)&PlacesView::OnResultActivated,
350+ this);
351+ _ubus_handles[3] = ubus_server_register_interest (ubus, UBUS_PLACE_VIEW_QUEUE_DRAW,
352+ (UBusCallback)&PlacesView::OnPlaceViewQueueDrawNeeded,
353+ this);
354
355 _icon_loader = IconLoader::GetDefault ();
356
357@@ -136,6 +139,13 @@
358
359 PlacesView::~PlacesView ()
360 {
361+ UBusServer* ubus = ubus_server_get_default ();
362+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
363+ {
364+ if (_ubus_handles[i] != 0)
365+ ubus_server_unregister_interest (ubus, _ubus_handles[i]);
366+ }
367+
368 if (_close_idle != 0)
369 {
370 g_source_remove (_close_idle);
371
372=== modified file 'src/PlacesView.h'
373--- src/PlacesView.h 2011-04-10 19:19:50 +0000
374+++ src/PlacesView.h 2011-04-13 07:50:54 +0000
375@@ -172,6 +172,8 @@
376 bool _pending_activation;
377
378 bool _search_empty;
379+
380+ guint _ubus_handles[4];
381 };
382
383 #endif // PANEL_HOME_BUTTON_H
384
385=== modified file 'src/unityshell.cpp'
386--- src/unityshell.cpp 2011-04-11 09:23:51 +0000
387+++ src/unityshell.cpp 2011-04-13 07:50:54 +0000
388@@ -942,21 +942,24 @@
389 optionSetPanelFirstMenuTerminate(boost::bind (&UnityScreen::showPanelFirstMenuKeyTerminate, this, _1, _2, _3));
390 optionSetLauncherRevealEdgeInitiate (boost::bind (&UnityScreen::launcherRevealEdgeInitiate, this, _1, _2, _3));
391
392+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
393+ _ubus_handles[i] = 0;
394+
395 UBusServer* ubus = ubus_server_get_default ();
396- ubus_server_register_interest (ubus,
397- UBUS_LAUNCHER_START_KEY_NAV,
398- (UBusCallback)&UnityScreen::OnLauncherStartKeyNav,
399- this);
400-
401- ubus_server_register_interest (ubus,
402- UBUS_LAUNCHER_END_KEY_NAV,
403- (UBusCallback)&UnityScreen::OnLauncherEndKeyNav,
404- this);
405-
406- ubus_server_register_interest (ubus,
407- UBUS_QUICKLIST_END_KEY_NAV,
408- (UBusCallback)&UnityScreen::OnQuicklistEndKeyNav,
409- this);
410+ _ubus_handles[0] = ubus_server_register_interest (ubus,
411+ UBUS_LAUNCHER_START_KEY_NAV,
412+ (UBusCallback)&UnityScreen::OnLauncherStartKeyNav,
413+ this);
414+
415+ _ubus_handles[1] = ubus_server_register_interest (ubus,
416+ UBUS_LAUNCHER_END_KEY_NAV,
417+ (UBusCallback)&UnityScreen::OnLauncherEndKeyNav,
418+ this);
419+
420+ _ubus_handles[2] = ubus_server_register_interest (ubus,
421+ UBUS_QUICKLIST_END_KEY_NAV,
422+ (UBusCallback)&UnityScreen::OnQuicklistEndKeyNav,
423+ this);
424
425 g_timeout_add (0, &UnityScreen::initPluginActions, this);
426 g_timeout_add (5000, (GSourceFunc) write_logger_data_to_disk, NULL);
427@@ -987,6 +990,13 @@
428 panelController->UnReference ();
429 unity_a11y_finalize ();
430
431+ UBusServer* ubus = ubus_server_get_default ();
432+ for (int i = 0; i < G_N_ELEMENTS (_ubus_handles); i++)
433+ {
434+ if (_ubus_handles[i] != 0)
435+ ubus_server_unregister_interest (ubus, _ubus_handles[i]);
436+ }
437+
438 if (relayoutSourceId != 0)
439 g_source_remove (relayoutSourceId);
440 }
441
442=== modified file 'src/unityshell.h'
443--- src/unityshell.h 2011-04-10 16:59:02 +0000
444+++ src/unityshell.h 2011-04-13 07:50:54 +0000
445@@ -63,7 +63,7 @@
446 /* We store these to avoid unecessary calls to ::get */
447 CompScreen *screen;
448 CompositeScreen *cScreen;
449- GLScreen *gScreen;
450+ GLScreen *gScreen;
451
452 /* prepares nux for drawing */
453 void
454@@ -222,6 +222,7 @@
455 bool needsRelayout;
456 guint32 relayoutSourceId;
457 guint _edge_trigger_handle;
458+ guint _ubus_handles[3];
459
460 /* keyboard-nav mode */
461 CompWindow* newFocusedWindow;