Merge lp:~3v1n0/unity/missing-launcher-icon-fix-5.0 into lp:unity/5.0

Proposed by Marco Trevisan (Treviño) on 2012-05-21
Status: Merged
Approved by: Didier Roche on 2012-05-21
Approved revision: 2350
Merged at revision: 2349
Proposed branch: lp:~3v1n0/unity/missing-launcher-icon-fix-5.0
Merge into: lp:unity/5.0
Diff against target: 394 lines (+155/-35)
8 files modified
plugins/unityshell/src/BamfLauncherIcon.cpp (+42/-3)
plugins/unityshell/src/BamfLauncherIcon.h (+2/-0)
plugins/unityshell/src/LauncherController.cpp (+0/-4)
tests/autopilot/autopilot/emulators/bamf.py (+15/-8)
tests/autopilot/autopilot/emulators/unity/icons.py (+7/-2)
tests/autopilot/autopilot/emulators/unity/launcher.py (+8/-0)
tests/autopilot/autopilot/tests/__init__.py (+3/-2)
tests/autopilot/autopilot/tests/test_launcher.py (+78/-16)
To merge this branch: bzr merge lp:~3v1n0/unity/missing-launcher-icon-fix-5.0
Reviewer Review Type Date Requested Status
Gord Allott (community) 2012-05-21 Needs Information on 2012-05-22
Review via email: mp+106644@code.launchpad.net
To post a comment you must log in.
Unity Merger (unity-merger) wrote :

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/737/console reported an error when processing this lp:~3v1n0/unity/missing-launcher-icon-fix-5.0 branch.
Not merging it.

Gord Allott (gordallott) wrote :

Surely this means that it now takes every application a second before its removed from the launcher? i don't think that is something we want

review: Needs Information
Marco Trevisan (Treviño) (3v1n0) wrote :

No, the icon is immediately removed (because this is controlled by the "running-changed" signal), but the icon itself is not removed by the model too early. We wait some time.
See the the description of the MR on lp:~3v1n0/unity/quick-app-reopen-icon-fix

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/BamfLauncherIcon.cpp'
2--- plugins/unityshell/src/BamfLauncherIcon.cpp 2012-04-26 09:19:21 +0000
3+++ plugins/unityshell/src/BamfLauncherIcon.cpp 2012-05-21 15:34:22 +0000
4@@ -54,6 +54,8 @@
5 , _fill_supported_types_id(0)
6 , _window_moved_id(0)
7 {
8+ g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"),
9+ GUINT_TO_POINTER(1));
10 auto bamf_view = glib::object_cast<BamfView>(_bamf_app);
11
12 glib::String icon(bamf_view_get_icon(bamf_view));
13@@ -99,10 +101,17 @@
14 sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "running-changed",
15 [&] (BamfView*, gboolean running) {
16 SetQuirk(QUIRK_RUNNING, running);
17+
18 if (running)
19 {
20 EnsureWindowState();
21 UpdateIconGeometries(GetCenters());
22+
23+ if (_remove_timeout_id)
24+ {
25+ g_source_remove(_remove_timeout_id);
26+ _remove_timeout_id = 0;
27+ }
28 }
29 });
30 _gsignals.Add(sig);
31@@ -117,7 +126,23 @@
32 sig = new glib::Signal<void, BamfView*>(bamf_view, "closed",
33 [&] (BamfView*) {
34 if (!IsSticky())
35- Remove();
36+ {
37+ /* Use a timeout to remove the icon, this avoids
38+ * that we remove an application that is going
39+ * to be reopened soon. So applications that
40+ * have a splash screen won't be removed from
41+ * the launcher while the splash is closed and
42+ * a new window is opened. */
43+ if (_remove_timeout_id)
44+ g_source_remove(_remove_timeout_id);
45+
46+ _remove_timeout_id = g_timeout_add_seconds(1, [] (gpointer data) -> gboolean {
47+ auto self = static_cast<BamfLauncherIcon*>(data);
48+ self->Remove();
49+ self->_remove_timeout_id = 0;
50+ return false;
51+ }, this);
52+ }
53 });
54 _gsignals.Add(sig);
55
56@@ -143,8 +168,10 @@
57 BamfLauncherIcon::~BamfLauncherIcon()
58 {
59 g_object_set_qdata(G_OBJECT(_bamf_app.RawPtr()),
60- g_quark_from_static_string("unity-seen"),
61- GUINT_TO_POINTER(0));
62+ g_quark_from_static_string("unity-seen"), nullptr);
63+
64+ if (_remove_timeout_id != 0)
65+ g_source_remove(_remove_timeout_id);
66
67 if (_fill_supported_types_id != 0)
68 g_source_remove(_fill_supported_types_id);
69@@ -159,6 +186,18 @@
70 g_source_remove(_dnd_hover_timer);
71 }
72
73+void BamfLauncherIcon::Remove()
74+{
75+ /* Removing the unity-seen flag to the wrapped bamf application, on remove
76+ * request we make sure that if the bamf application is re-opened while
77+ * the removal process is still ongoing, the application will be shown
78+ * on the launcher. */
79+ g_object_set_qdata(G_OBJECT(_bamf_app.RawPtr()),
80+ g_quark_from_static_string("unity-seen"), nullptr);
81+
82+ SimpleLauncherIcon::Remove();
83+}
84+
85 bool BamfLauncherIcon::IsSticky() const
86 {
87 return bamf_view_is_sticky(BAMF_VIEW(_bamf_app.RawPtr()));
88
89=== modified file 'plugins/unityshell/src/BamfLauncherIcon.h'
90--- plugins/unityshell/src/BamfLauncherIcon.h 2012-04-26 00:35:37 +0000
91+++ plugins/unityshell/src/BamfLauncherIcon.h 2012-05-21 15:34:22 +0000
92@@ -66,6 +66,7 @@
93 std::string NameForWindow(Window window);
94
95 protected:
96+ void Remove();
97 void UpdateIconGeometries(std::vector<nux::Point3> center);
98 void OnCenterStabilized(std::vector<nux::Point3> center);
99 void AddProperties(GVariantBuilder* builder);
100@@ -124,6 +125,7 @@
101 guint _dnd_hover_timer;
102
103 bool _supported_types_filled;
104+ guint _remove_timeout_id;
105 guint _fill_supported_types_id;
106 guint _window_moved_id;
107 guint _quicklist_activated_id;
108
109=== modified file 'plugins/unityshell/src/LauncherController.cpp'
110--- plugins/unityshell/src/LauncherController.cpp 2012-04-26 00:53:36 +0000
111+++ plugins/unityshell/src/LauncherController.cpp 2012-05-21 15:34:22 +0000
112@@ -740,7 +740,6 @@
113 return;
114 }
115
116- g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GUINT_TO_POINTER(1));
117 AbstractLauncherIcon::Ptr icon(new BamfLauncherIcon(app));
118 icon->visibility_changed.connect(sigc::mem_fun(self, &Impl::SortAndUpdate));
119 icon->SetSortPriority(self->sort_priority_++);
120@@ -764,7 +763,6 @@
121 }
122
123 bamf_view_set_sticky(BAMF_VIEW(app), true);
124- g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GUINT_TO_POINTER(1));
125 AbstractLauncherIcon::Ptr icon (new BamfLauncherIcon(app));
126 icon->SetSortPriority(sort_priority_++);
127 result = icon;
128@@ -790,7 +788,6 @@
129 }
130
131 bamf_view_set_sticky(BAMF_VIEW(app), true);
132- g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GUINT_TO_POINTER(1));
133 result = new SoftwareCenterLauncherIcon(app, aptdaemon_trans_id, icon_path);
134 result->SetSortPriority(sort_priority_++);
135
136@@ -833,7 +830,6 @@
137 if (g_object_get_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen")))
138 continue;
139
140- g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GUINT_TO_POINTER(1));
141 AbstractLauncherIcon::Ptr icon(new BamfLauncherIcon(app));
142 icon->SetSortPriority(sort_priority_++);
143 RegisterIcon(icon);
144
145=== modified file 'tests/autopilot/autopilot/emulators/bamf.py'
146--- tests/autopilot/autopilot/emulators/bamf.py 2012-04-26 02:53:18 +0000
147+++ tests/autopilot/autopilot/emulators/bamf.py 2012-05-21 15:34:22 +0000
148@@ -75,6 +75,14 @@
149 """
150 return [a for a in self.get_running_applications() if a.desktop_file == desktop_file]
151
152+ def get_application_by_xid(self, xid):
153+ """Return the application that has a child with the requested xid or None."""
154+
155+ app_path = self.matcher_interface.ApplicationForXid(xid)
156+ if len(app_path):
157+ return BamfApplication(app_path)
158+ return None
159+
160 def get_open_windows(self, user_visible_only=True):
161 """Get a list of currently open windows.
162
163@@ -85,17 +93,16 @@
164
165 """
166
167- # Get the stacking order from the root window.
168- root_win = _X_DISPLAY.screen().root
169- prop = root_win.get_full_property(
170- _X_DISPLAY.get_atom('_NET_CLIENT_LIST_STACKING'), X.AnyPropertyType)
171- stack = prop.value.tolist()
172-
173- windows = [BamfWindow(w) for w in self.matcher_interface.WindowPaths()]
174+ windows = [BamfWindow(w) for w in self.matcher_interface.WindowStackForMonitor(-1)]
175 if user_visible_only:
176 windows = filter(_filter_user_visible, windows)
177 # Now sort on stacking order.
178- return sorted(windows, key=lambda w: stack.index(w.x_id), reverse=True)
179+ return reversed(windows)
180+
181+ def get_window_by_xid(self, xid):
182+ """Get the BamfWindow that matches the provided 'xid'."""
183+ windows = [BamfWindow(w) for w in self.matcher_interface.WindowPaths() if BamfWindow(w).x_id == xid]
184+ return windows[0] if windows else None
185
186 def wait_until_application_is_running(self, desktop_file, timeout):
187 """Wait until a given application is running.
188
189=== modified file 'tests/autopilot/autopilot/emulators/unity/icons.py'
190--- tests/autopilot/autopilot/emulators/unity/icons.py 2012-04-24 13:14:52 +0000
191+++ tests/autopilot/autopilot/emulators/unity/icons.py 2012-05-21 15:34:22 +0000
192@@ -20,7 +20,7 @@
193
194 @property
195 def center_position(self):
196- """Get the center point of an icon, returns a tuple with (x, y, z)"""
197+ """Get the center point of an icon, returns a tuple with (x, y, z)."""
198 return (self.center_x, self.center_y, self.center_z)
199
200 def get_quicklist(self):
201@@ -34,12 +34,17 @@
202 return matches[0] if matches else None
203
204 def is_on_monitor(self, monitor):
205- """Returns True if the icon is available in the defined monitor"""
206+ """Returns True if the icon is available in the defined monitor."""
207 if monitor >= 0 and monitor < len(self.monitors_visibility):
208 return self.monitors_visibility[monitor]
209
210 return False
211
212+ def controls_window(self, xid):
213+ """Returns true if the icon controls the specified xid."""
214+
215+ return self.xids.contains(xid)
216+
217
218 class BFBLauncherIcon(SimpleLauncherIcon):
219 """Represents the BFB button in the launcher."""
220
221=== modified file 'tests/autopilot/autopilot/emulators/unity/launcher.py'
222--- tests/autopilot/autopilot/emulators/unity/launcher.py 2012-04-26 05:11:45 +0000
223+++ tests/autopilot/autopilot/emulators/unity/launcher.py 2012-05-21 15:34:22 +0000
224@@ -338,6 +338,14 @@
225
226 return None
227
228+ def get_icon_by_window_xid(self, xid):
229+ """Gets a launcher icon that controls the specified window xid."""
230+ icons = [i for i in self.get_children_by_type(SimpleLauncherIcon) if i.xids.contains(xid)]
231+ if (len(icons)):
232+ return icons[0]
233+
234+ return None
235+
236 def get_icons_by_filter(self, **kwargs):
237 """Get a list of icons that satisfy the given filters.
238
239
240=== modified file 'tests/autopilot/autopilot/tests/__init__.py'
241--- tests/autopilot/autopilot/tests/__init__.py 2012-04-26 04:02:39 +0000
242+++ tests/autopilot/autopilot/tests/__init__.py 2012-05-21 15:34:22 +0000
243@@ -277,8 +277,9 @@
244 def close_all_app(self, app_name):
245 """Close all instances of the app_name."""
246 app = self.KNOWN_APPS[app_name]
247- self.addCleanup(call, "kill `pidof %s`" % (app['process-name']), shell=True)
248- super(LoggedTestCase, self).tearDown()
249+ pids = check_output(["pidof", app['process-name']]).split()
250+ if len(pids):
251+ call(["kill"] + pids)
252
253 def get_app_instances(self, app_name):
254 """Get BamfApplication instances for app_name."""
255
256=== modified file 'tests/autopilot/autopilot/tests/test_launcher.py'
257--- tests/autopilot/autopilot/tests/test_launcher.py 2012-04-26 06:44:00 +0000
258+++ tests/autopilot/autopilot/tests/test_launcher.py 2012-05-21 15:34:22 +0000
259@@ -417,7 +417,6 @@
260 sleep(1)
261 [mah_win2] = [w for w in mahj.get_windows() if w.x_id != mah_win1.x_id]
262 self.assertTrue(mah_win2.is_focused)
263-
264 self.assertVisibleWindowStack([mah_win2, calc_win, mah_win1])
265
266 mahj_icon = self.launcher.model.get_icon_by_desktop_id(mahj.desktop_file)
267@@ -446,6 +445,20 @@
268 self.assertTrue(mah_win2.is_hidden)
269 self.assertVisibleWindowStack([mah_win1, calc_win])
270
271+ def test_icon_shows_on_quick_application_reopen(self):
272+ """Icons should stay on launcher when an application is quickly closed/reopened."""
273+ calc = self.start_app("Calculator")
274+ desktop_file = calc.desktop_file
275+ calc_icon = self.launcher.model.get_icon_by_desktop_id(desktop_file)
276+ self.assertThat(calc_icon.visible, Eventually(Equals(True)))
277+
278+ self.close_all_app("Calculator")
279+ calc = self.start_app("Calculator")
280+ sleep(2)
281+
282+ calc_icon = self.launcher.model.get_icon_by_desktop_id(desktop_file)
283+ self.assertThat(calc_icon, NotEquals(None))
284+ self.assertThat(calc_icon.visible, Eventually(Equals(True)))
285
286 class LauncherRevealTests(LauncherTestCase):
287 """Test the launcher reveal behavior when in autohide mode."""
288@@ -579,6 +592,15 @@
289 self.start_app("Calculator")
290 self.start_app("System Settings")
291
292+ def start_desktopless_test_apps(self):
293+ """Start test applications with no .desktop file associated."""
294+ test_apps = ["xclock"]
295+
296+ for app in test_apps:
297+ os.spawnlp(os.P_NOWAIT, app, app)
298+ self.addCleanup(call, ["killall", app])
299+ self.wait_for_process_started(app)
300+
301 def get_test_apps(self):
302 """Return a tuple of test application instances.
303
304@@ -588,35 +610,75 @@
305 """
306 [calc] = self.get_app_instances("Calculator")
307 [sys_settings] = self.get_app_instances("System Settings")
308- return (calc, sys_settings)
309+ return [calc, sys_settings]
310+
311+ def get_desktopless_test_apps(self):
312+ """Return a tuple of test application with no .desktop files instances."""
313+ [xclock_win] = [w for w in self.bamf.get_open_windows() if w.name == "xclock"]
314+ return [xclock_win.application]
315
316 def assertOnlyOneLauncherIcon(self, **kwargs):
317 """Asserts that there is only one launcher icon with the given filter."""
318 icons = self.launcher.model.get_icons_by_filter(**kwargs)
319 self.assertThat(len(icons), Equals(1))
320
321- def wait_for_bamf_daemon(self):
322- """Wait until the bamf daemon has been started."""
323- for i in range(10):
324- #pgrep returns 0 if it matched something:
325- if call(["pgrep", "bamfdaemon"]) == 0:
326- return
327- sleep(1)
328-
329- def test_killing_bamfdaemon_does_not_duplicate_desktop_ids(self):
330- """Killing bamfdaemon should not duplicate any desktop ids in the model."""
331- self.start_test_apps()
332-
333+ def wait_for_process_started(self, app):
334+ """Wait until the application app has been started."""
335+ for i in range(10):
336+ sleep(1)
337+ #pgrep returns 0 if it matched something:
338+ if call(["pgrep", app]) == 0:
339+ return
340+
341+ def wait_for_process_killed(self, app):
342+ """Wait until the application app has been killed."""
343+ for i in range(10):
344+ #pgrep returns 0 if it matched something:
345+ if call(["pgrep", app]) != 0:
346+ return
347+ sleep(1)
348+
349+ def kill_and_restart_bamfdaemon(self):
350+ """Kills and resumes bamfdaemon."""
351 call(["pkill", "bamfdaemon"])
352- sleep(1)
353+ self.wait_for_process_killed("bamfdaemon")
354
355 # trigger the bamfdaemon to be reloaded again, and wait for it to appear:
356 self.bamf = Bamf()
357- self.wait_for_bamf_daemon()
358+ self.wait_for_process_started("bamfdaemon")
359+
360+ def test_killing_bamfdaemon_does_not_duplicate_desktop_ids(self):
361+ """Killing bamfdaemon should not duplicate any desktop ids in the model."""
362+ self.start_test_apps()
363+ self.kill_and_restart_bamfdaemon()
364
365 for test_app in self.get_test_apps():
366+ logger.info("Checking for duplicated launcher icon for application %s", test_app.name)
367 self.assertOnlyOneLauncherIcon(desktop_id=test_app.desktop_file)
368
369+ def test_killing_bamfdaemon_does_not_duplicate_application_xids(self):
370+ """Killing bamfdaemon should not duplicate any xid in the model."""
371+ self.start_test_apps()
372+ self.start_desktopless_test_apps()
373+ self.kill_and_restart_bamfdaemon()
374+
375+ test_apps = self.get_test_apps() + self.get_desktopless_test_apps()
376+
377+ for test_app in test_apps:
378+ logger.info("Checking for duplicated launcher icon for application %s", test_app.name)
379+ test_windows = [w.x_id for w in test_app.get_windows()]
380+ self.assertOnlyOneLauncherIcon(xids=test_windows)
381+
382+ def test_killing_bamfdaemon_does_not_duplicate_any_icon_application_id(self):
383+ """Killing bamfdaemon should not duplicate any application ids in the model."""
384+ self.start_test_apps()
385+ self.start_desktopless_test_apps()
386+ self.kill_and_restart_bamfdaemon()
387+
388+ for icon in self.launcher.model.get_bamf_launcher_icons():
389+ logger.info("Checking for duplicated launcher icon %s", icon.tooltip_text)
390+ self.assertOnlyOneLauncherIcon(application_id=icon.application_id)
391+
392
393 class LauncherCaptureTests(AutopilotTestCase):
394 """Test the launchers ability to capture/not capture the mouse."""

Subscribers

People subscribed via source and target branches