Merge lp:~azzar1/unity/lp-1165097 into lp:unity

Proposed by Andrea Azzarone on 2013-04-05
Status: Merged
Approved by: Brandon Schaefer on 2013-04-08
Approved revision: 3269
Merged at revision: 3286
Proposed branch: lp:~azzar1/unity/lp-1165097
Merge into: lp:unity
Diff against target: 336 lines (+99/-51)
6 files modified
launcher/ApplicationLauncherIcon.cpp (+62/-30)
launcher/ApplicationLauncherIcon.h (+13/-1)
launcher/Launcher.cpp (+1/-7)
launcher/SoftwareCenterLauncherIcon.cpp (+1/-1)
tests/test_application_launcher_icon.cpp (+12/-1)
tests/test_software_center_launcher_icon.cpp (+10/-11)
To merge this branch: bzr merge lp:~azzar1/unity/lp-1165097
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2013-04-08
Brandon Schaefer (community) 2013-04-05 Approve on 2013-04-08
Review via email: mp+157462@code.launchpad.net

Commit message

Connect/Disconnect unity::Application signals when replacing app_ in SCLauncherIcon.

Description of the change

== Problem ==
Related bugs and blueprints:
Bug #1165097: Launcher icons automatically placed in the launcher by USC (or apps lens) fail to have running 'pips'.

== Fix ==
Disconnect/Connect signals.

== Tests ==
Unit tests updated/added.

To post a comment you must log in.
Brandon Schaefer (brandontschaefer) wrote :

Confirmed fixed, and tests look good as well.

review: Approve
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'launcher/ApplicationLauncherIcon.cpp'
2--- launcher/ApplicationLauncherIcon.cpp 2013-04-05 17:04:59 +0000
3+++ launcher/ApplicationLauncherIcon.cpp 2013-04-05 19:17:20 +0000
4@@ -59,7 +59,6 @@
5
6 ApplicationLauncherIcon::ApplicationLauncherIcon(ApplicationPtr const& app)
7 : SimpleLauncherIcon(IconType::APPLICATION)
8- , app_(app)
9 , _startup_notification_timestamp(0)
10 , _last_scroll_timestamp(0)
11 , _last_scroll_direction(ScrollDirection::DOWN)
12@@ -88,25 +87,68 @@
13 << ", active: " << (app->active() ? "yes" : "no")
14 << ", running: " << (app->running() ? "yes" : "no");
15
16+ SetApplication(app);
17+
18+ WindowManager& wm = WindowManager::Default();
19+ wm.window_minimized.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::OnWindowMinimized));
20+ wm.window_moved.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::OnWindowMoved));
21+ wm.screen_viewport_switch_ended.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
22+ wm.terminate_expo.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
23+
24+ EnsureWindowState();
25+ UpdateMenus();
26+ UpdateDesktopFile();
27+ UpdateBackgroundColor();
28+
29+ // hack
30+ SetProgress(0.0f);
31+}
32+
33+ApplicationLauncherIcon::~ApplicationLauncherIcon()
34+{
35+ if (app_)
36+ {
37+ app_->sticky = false;
38+ app_->seen = false;
39+ }
40+
41+ DisconnectApplicationSignalsConnections();
42+}
43+
44+void ApplicationLauncherIcon::SetApplication(ApplicationPtr const& app)
45+{
46+ if (app_ == app)
47+ return;
48+
49+ app_ = app;
50+ DisconnectApplicationSignalsConnections();
51+ SetupApplicationSignalsConnections();
52+}
53+
54+void ApplicationLauncherIcon::SetupApplicationSignalsConnections()
55+{
56 // Lambda functions should be fine here because when the application the icon
57 // is only ever removed when the application is closed.
58- app->window_opened.connect([this](ApplicationWindow const&) {
59+ window_opened_connection_ = app_->window_opened.connect([this](ApplicationWindow const&) {
60 EnsureWindowState();
61 UpdateMenus();
62 UpdateIconGeometries(GetCenters());
63 });
64- app->window_closed.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
65- app->window_moved.connect(sigc::hide(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState)));
66-
67- app->urgent.changed.connect([this](bool const& urgent) {
68+
69+ window_closed_connection_ = app_->window_closed.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
70+ window_moved_connection_ = app_->window_moved.connect(sigc::hide(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState)));
71+
72+ urgent_changed_connection_ = app_->urgent.changed.connect([this](bool const& urgent) {
73 LOG_DEBUG(logger) << tooltip_text() << " urgent now " << (urgent ? "true" : "false");
74 SetQuirk(Quirk::URGENT, urgent);
75 });
76- app->active.changed.connect([this](bool const& active) {
77+
78+ active_changed_connection_ = app_->active.changed.connect([this](bool const& active) {
79 LOG_DEBUG(logger) << tooltip_text() << " active now " << (active ? "true" : "false");
80 SetQuirk(Quirk::ACTIVE, active);
81 });
82- app->running.changed.connect([this](bool const& running) {
83+
84+ running_changed_connection_ = app_->running.changed.connect([this](bool const& running) {
85 LOG_DEBUG(logger) << tooltip_text() << " running now " << (running ? "true" : "false");
86 SetQuirk(Quirk::RUNNING, running);
87
88@@ -131,12 +173,13 @@
89 UpdateIconGeometries(GetCenters());
90 }
91 });
92- app->visible.changed.connect([this](bool const& visible) {
93+
94+ visible_changed_connection_ = app_->visible.changed.connect([this](bool const& visible) {
95 if (!IsSticky())
96 SetQuirk(Quirk::VISIBLE, visible);
97 });
98
99- app->closed.connect([this]() {
100+ closed_changed_connection_ = app_->closed.connect([this]() {
101 if (!IsSticky())
102 {
103 SetQuirk(Quirk::VISIBLE, false);
104@@ -153,29 +196,18 @@
105 }, ICON_REMOVE_TIMEOUT);
106 }
107 });
108-
109- WindowManager& wm = WindowManager::Default();
110- wm.window_minimized.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::OnWindowMinimized));
111- wm.window_moved.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::OnWindowMoved));
112- wm.screen_viewport_switch_ended.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
113- wm.terminate_expo.connect(sigc::mem_fun(this, &ApplicationLauncherIcon::EnsureWindowState));
114-
115- EnsureWindowState();
116- UpdateMenus();
117- UpdateDesktopFile();
118- UpdateBackgroundColor();
119-
120- // hack
121- SetProgress(0.0f);
122 }
123
124-ApplicationLauncherIcon::~ApplicationLauncherIcon()
125+void ApplicationLauncherIcon::DisconnectApplicationSignalsConnections()
126 {
127- if (app_)
128- {
129- app_->sticky = false;
130- app_->seen = false;
131- }
132+ window_opened_connection_.disconnect();
133+ window_closed_connection_.disconnect();
134+ window_moved_connection_.disconnect();
135+ urgent_changed_connection_.disconnect();
136+ active_changed_connection_.disconnect();
137+ running_changed_connection_.disconnect();
138+ visible_changed_connection_.disconnect();
139+ closed_changed_connection_.disconnect();
140 }
141
142 bool ApplicationLauncherIcon::GetQuirk(AbstractLauncherIcon::Quirk quirk) const
143
144=== modified file 'launcher/ApplicationLauncherIcon.h'
145--- launcher/ApplicationLauncherIcon.h 2013-03-29 15:57:40 +0000
146+++ launcher/ApplicationLauncherIcon.h 2013-04-05 19:17:20 +0000
147@@ -71,6 +71,7 @@
148 void PerformScroll(ScrollDirection direction, Time timestamp) override;
149
150 protected:
151+ void SetApplication(ApplicationPtr const& app);
152 void Remove();
153 void UpdateIconGeometries(std::vector<nux::Point3> center);
154 void OnCenterStabilized(std::vector<nux::Point3> center);
155@@ -97,7 +98,6 @@
156 void UpdateDesktopFile();
157 void UpdateRemoteUri();
158 std::string _desktop_file;
159- ApplicationPtr app_;
160
161 private:
162 typedef unsigned long int WindowFilterMask;
163@@ -109,6 +109,8 @@
164 ON_ALL_MONITORS = (1 << 3),
165 };
166
167+ void SetupApplicationSignalsConnections();
168+ void DisconnectApplicationSignalsConnections();
169 void EnsureWindowState();
170 void EnsureMenuItemsWindowsReady();
171 void EnsureMenuItemsReady();
172@@ -128,6 +130,7 @@
173 std::string GetDesktopID();
174 WindowList GetWindowsOnCurrentDesktopInStackingOrder();
175
176+ ApplicationPtr app_;
177 std::string _remote_uri;
178 Time _startup_notification_timestamp;
179 Time _last_scroll_timestamp;
180@@ -145,6 +148,15 @@
181
182 bool use_custom_bg_color_;
183 nux::Color bg_color_;
184+
185+ sigc::connection window_opened_connection_;
186+ sigc::connection window_closed_connection_;
187+ sigc::connection window_moved_connection_;
188+ sigc::connection urgent_changed_connection_;
189+ sigc::connection active_changed_connection_;
190+ sigc::connection running_changed_connection_;
191+ sigc::connection visible_changed_connection_;
192+ sigc::connection closed_changed_connection_;
193 };
194
195 }
196
197=== modified file 'launcher/Launcher.cpp'
198--- launcher/Launcher.cpp 2013-04-02 18:47:58 +0000
199+++ launcher/Launcher.cpp 2013-04-05 19:17:20 +0000
200@@ -837,13 +837,7 @@
201 // we dont need to show strays
202 if (!icon->GetQuirk(AbstractLauncherIcon::Quirk::RUNNING))
203 {
204- if (icon->GetQuirk(AbstractLauncherIcon::Quirk::URGENT))
205- {
206- arg.running_arrow = true;
207- arg.window_indicators = 1;
208- }
209- else
210- arg.window_indicators = 0;
211+ arg.window_indicators = 0;
212 }
213 else
214 {
215
216=== modified file 'launcher/SoftwareCenterLauncherIcon.cpp'
217--- launcher/SoftwareCenterLauncherIcon.cpp 2013-01-15 00:19:43 +0000
218+++ launcher/SoftwareCenterLauncherIcon.cpp 2013-04-05 19:17:20 +0000
219@@ -205,7 +205,7 @@
220 std::string new_desktop_path = GetActualDesktopFileAfterInstall();
221
222 // exchange the temp Application with the real one
223- app_ = ApplicationManager::Default().GetApplicationForDesktopFile(new_desktop_path);
224+ SetApplication(ApplicationManager::Default().GetApplicationForDesktopFile(new_desktop_path));
225
226 UpdateDesktopFile();
227 UpdateRemoteUri();
228
229=== modified file 'tests/test_application_launcher_icon.cpp'
230--- tests/test_application_launcher_icon.cpp 2013-04-05 17:04:59 +0000
231+++ tests/test_application_launcher_icon.cpp 2013-04-05 19:17:20 +0000
232@@ -14,7 +14,7 @@
233 * version 3 along with this program. If not, see
234 * <http://www.gnu.org/licenses/>
235 *
236- * Authored by: Andrea Azzarone <azzarone@gmail.com>
237+ * Authored by: Andrea Azzarone <andrea.azzarone@canonical.com>
238 * Brandon Schaefer <brandon.schaefer@canonical.com>
239 * Marco Trevisan <marco.trevisan@canonical.com>
240 */
241@@ -102,6 +102,17 @@
242 nux::ObjectPtr<MockApplicationLauncherIcon> mock_icon;
243 };
244
245+TEST_F(TestApplicationLauncherIcon, ApplicationSignalDisconnection)
246+{
247+ std::shared_ptr<MockApplication> app = std::make_shared<MockApplication>(USC_DESKTOP);
248+ {
249+ nux::ObjectPtr<MockApplicationLauncherIcon> icon(new NiceMock<MockApplicationLauncherIcon>(app));
250+ EXPECT_FALSE(app->closed.empty());
251+ }
252+
253+ EXPECT_TRUE(app->closed.empty());
254+}
255+
256 TEST_F(TestApplicationLauncherIcon, Position)
257 {
258 EXPECT_EQ(usc_icon->position(), AbstractLauncherIcon::Position::FLOATING);
259
260=== modified file 'tests/test_software_center_launcher_icon.cpp'
261--- tests/test_software_center_launcher_icon.cpp 2013-03-07 04:36:33 +0000
262+++ tests/test_software_center_launcher_icon.cpp 2013-04-05 19:17:20 +0000
263@@ -24,13 +24,14 @@
264 #include <config.h>
265 #include <gmock/gmock.h>
266
267-#include "ApplicationManager.h"
268+#include "mock-application.h"
269 #include "SoftwareCenterLauncherIcon.h"
270 #include "Launcher.h"
271 #include "PanelStyle.h"
272 #include "UnitySettings.h"
273 #include "test_utils.h"
274
275+using namespace testmocks;
276 using namespace unity;
277 using namespace unity::launcher;
278
279@@ -40,6 +41,7 @@
280 {
281 const std::string LOCAL_DATA_DIR = BUILDDIR"/tests/data";
282 const std::string USC_DESKTOP = LOCAL_DATA_DIR+"/applications/ubuntu-software-center.desktop";
283+const std::string USC_APP_INSTALL_DESKTOP = "/usr/share/app-install/desktop/software-center:ubuntu-software-center.desktop";
284
285 class MockSoftwareCenterLauncherIcon : public SoftwareCenterLauncherIcon
286 {
287@@ -54,14 +56,13 @@
288 using SoftwareCenterLauncherIcon::_desktop_file;
289 using SoftwareCenterLauncherIcon::GetRemoteUri;
290 using SoftwareCenterLauncherIcon::OnFinished;
291-
292 };
293
294 struct TestSoftwareCenterLauncherIcon : testing::Test
295 {
296 public:
297 TestSoftwareCenterLauncherIcon()
298- : usc(ApplicationManager::Default().GetApplicationForDesktopFile(USC_DESKTOP))
299+ : usc(std::make_shared<MockApplication>(USC_APP_INSTALL_DESKTOP, "softwarecenter"))
300 , icon(usc, "/com/canonical/unity/test/object/path", "")
301 {}
302
303@@ -79,7 +80,8 @@
304 TEST_F(TestSoftwareCenterLauncherIcon, DesktopFileTransformTrivial)
305 {
306 // no transformation needed
307- EXPECT_EQ(icon.GetActualDesktopFileAfterInstall(), USC_DESKTOP);
308+ icon._desktop_file = USC_DESKTOP;
309+ EXPECT_EQ(icon.GetActualDesktopFileAfterInstall(), USC_DESKTOP);
310 }
311
312 TEST_F(TestSoftwareCenterLauncherIcon, DesktopFileTransformAppInstall)
313@@ -101,19 +103,16 @@
314 // and ensure that the remote uri is updated from temp location to
315 // the real location
316 TEST_F(TestSoftwareCenterLauncherIcon, OnFinished)
317-{
318-
319- // simulate desktop file from app-install-data
320- icon._desktop_file = "/usr/share/app-install/desktop/software-center:ubuntu-software-center.desktop";
321-
322+{
323 // now simulate that the install was successful
324 GVariant *params = g_variant_new("(s)", "exit-success");
325 icon.OnFinished(params);
326
327 // and verify that both the desktop file and the remote uri gets updated
328+
329 EXPECT_EQ(icon._desktop_file, USC_DESKTOP);
330- EXPECT_EQ(icon.GetRemoteUri(),
331- "application://ubuntu-software-center.desktop");
332+ EXPECT_EQ(icon.GetRemoteUri(), "application://ubuntu-software-center.desktop");
333+ EXPECT_TRUE(usc->closed.empty());
334
335 g_variant_unref(params);
336 }