Merge lp:~bilalakhtar/unity/software-center-integration-for-o into lp:unity

Proposed by Bilal Akhtar on 2012-01-20
Status: Merged
Approved by: Mikkel Kamstrup Erlandsen on 2012-01-23
Approved revision: 1371
Merged at revision: 1858
Proposed branch: lp:~bilalakhtar/unity/software-center-integration-for-o
Merge into: lp:unity
Diff against target: 330 lines (+223/-4)
6 files modified
plugins/unityshell/src/Launcher.cpp (+3/-3)
plugins/unityshell/src/Launcher.h (+1/-0)
plugins/unityshell/src/LauncherController.cpp (+55/-1)
plugins/unityshell/src/LauncherController.h (+2/-0)
plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp (+102/-0)
plugins/unityshell/src/SoftwareCenterLauncherIcon.h (+60/-0)
To merge this branch: bzr merge lp:~bilalakhtar/unity/software-center-integration-for-o
Reviewer Review Type Date Requested Status
Mikkel Kamstrup Erlandsen (community) 2012-01-20 Approve on 2012-01-23
Unity Team 2012-01-20 Pending
Neil J. Patel 2012-01-20 Pending
Review via email:

This proposal supersedes a proposal from 2011-08-17.

Description of the change

This branch contains the first phase of the implementation of software-center integration with Unity. The complete spec on how it should be implemented, is located at:

This branch brings the following changes:
1) An app being installed is shown in the launcher with a tooltip "Waiting to install"
2) A progress bar on the launcher item displays the download/install progress.
3) When the app gets installed, the launcher items becomes usable, and the tooltip gets changed to the app name.

Things which are still to be implemented, in the next phases:
1) Animate the movement of the icon from the USC window to the Unity launcher.
2) Make the launcher icon wiggle when installation is complete
3) Stop the launcher icon from blinking when clicked in "waiting to install" state.

In the meantime, you can merge this branch into Unity while I implement the remaining aspects

Branch Testing instructions:
1) Make sure Unity built from my branch is running.
2) Get software-center from bzr branch lp:~gary-lasker/software-center/launcher-integration-lp761851
3) In the software center branch dir, run PYTHONPATH=. python ./software-center
4) Make sure that in the software center View menu, "New Applications In Launcher" is CHECKED.
5) Install a package using software-center that contains a desktop file (like gnome-color-chooser, gnome-mplayer or could be any package with a .desktop file)
6) Watch the Unity launcher :-)

To post a comment you must log in.
Neil J. Patel (njpatel) wrote : Posted in a previous version of this proposal

Hey Bilal, great work so far! I have some points which will hopefully reduce the code and make it a bit nicer to merge:

+ We try and not do anything _sync() in Unity, as it effects startup time and, if done after startup, painting time of Compiz. I see that you are connecting to the apt daemon via D-Bus, to make the code cleaner and automatically async, I suggest you use unity::glib::DBusProxy in <UnityCore/GLibDBusProxy.h> I added, which wraps the GIO DBusProxy bits in a C++ wrapper that:

  - Connects to your chosen bus and object asynchronously
  - Let's you use the Connect() method that allows you to connect to signals from the proxy individually [instead of
    having a if (signal_name == foo) else if () etc etc], and also allows you to use a sigc::mem_fun instead of a
    static function, which should make the code nicer.

You also forgot to store the proxy pointer and unref it on destruction (as well as disconnecting the signal). Using the glib::DBusProxy will handle all this for you internally, so you'll only need to delete the proxy instance once your done with it.

+ You shouldn't be g_variant_unref'ing the "params" value, as it is passed as a function argument and not expected to be unref'd. I think you wanted to unref "property_value".

+ When you start using glib::DBusProxy, you should be able to incorporate initialize_tooltip_text(); into your constructor as the constructor won't be as large anymore.

+ Instead of if (!app), do if (!BAMF_IS_APPLICATION(app))

review: Needs Fixing
Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

Hi Neil,

I managed to get my file to use GLibDBusProxy. Remember the memory issue when adding the tooltip line suddenly made all the dbus calls fail mysteriously ? Moving to GLibDBusProxy has resurrected that weird issue, and now, I'm not able to find out the line which is causing it :(

Can you help me at it? I'll describe exactly what the issue is and where its happening:

1) The SCLauncherIcon class constructor is called, and both _aptdaemon_trans and _aptdaemon_trans_prop are initialized.

2) The Progress Get call is made. It fails, GLibDBusProxy returns an error that the proxy doesn't exist (while in fact it does, I've double-checked it with the aptdaemon log). Nothing comes in the log after this, except for point 3 below:

3) Mysteriously, the OnFinished callback gets called correctly when the transaction finishes (as expected), and I get the "Transaction Finished" message in the log at the right time.

Sure, C/C++ suck :(

Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

Have you tried running a 'dbus-monitor > dbus-dump.txt' before doing a test run (and ctrl-c the command after). And then inspect the DBus traffic in that file carefully to make sure it looks like you expect?

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

Yes, its exactly how its expected to be. Aptdaemon is creating the exactly same object which is being said by Unity to "not exist".

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

Okay, so this merge request is okay for a review again. Everything which I've mentioned in the description works, I've tested it with Gary's branch and all.

Do note that the animation hasn't been implemented yet, I'll do that later on. Most of the groundwork is complete, only the animation and the wiggle etc has to land, which I'll do next weekend.

This was originally supposed to land in Oneiric but due to my faults it was postponed to Precise., but I've had good progress so far in the Precise cycle and as per current state, expect all changes to have landed by Alpha 2.

Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

Just some drive by comments - I am not sure I am the right guy to review this...

166 + * Copyright (C) 2010 Canonical Ltd

I believe we write 2012 these days ;-)

223 + _aptdaemon_trans->Connect("Finished", sigc::mem_fun(*this, &SoftwareCenterLauncherIcon::OnFinished));
224 + _aptdaemon_trans->Connect("PropertyChanged", sigc::mem_fun(*this, &SoftwareCenterLauncherIcon::OnPropertyChanged));

Are you sure you mean '*this' and not 'this'?

Generally; I don't like the 'if (!g_strcmp0 (variable, "XYZ"))' construction. Do it like how it's done elsewhere too, with == 0. It improves legibility. Humans doesn't not suck at negations ;-)

review: Needs Fixing
Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

Okay, fixed, ready for a review again!

Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

Sorry :-)

249 + if (!g_strcmp0 (property_name, "Progress")) {

Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

I am taking the branch for a test spin - and one problem I've discovered so far is that if S-C hangs (because apt is broken typically) then you'll be perpetually stuck with a launcher icon with an empty progress bar on it.

I am not sure what to do in these situations. I guess the right thing is to simply have some sort of timeout. If there haven't been progress in 20s then silently remove the launcher icon. There are probably better ways though..?

review: Needs Fixing
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

More comments - mostly also noted in your merge request:

 - Clicking the icon when the root password dialog from USC is up, starts pulsing the icon as if launched. From the spec I gather the icon should be grayed out and unresponive.

 - Same for while the app is installing

 - With lots of apps in the launcher the new app is "folded away". Meaning that I can't see the progress bar or tell when the app is done installing. I'd suggest that we wiggle it/call for attention/set urgency when done installing.

I guess these are points 2) and 3) on your todo list - except that you should "graying out while installing" as well?

But with the g_strcmp0() fix I noted, I guess this branch is good enough to merge. It is at leats ways better than not having it.

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

As for the grey out part, I chatted with MPT at UDS and he was ready to compromise on that part and instead get a progress bar implemented.So, we'll implement a progress bar for now and we'll see later what we can do about the graying part.

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

I'll make the g_strcmp0 fix this evening and then you can go ahead with it for now.

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

There, you can review it again :)

Bilal Akhtar (bilalakhtar) wrote : Posted in a previous version of this proposal

As for your question on SC hanging, in theory a crashed SC shouldn't screw anything up, the progress bar will still be updated regularly as it queries progress status from aptdaemon and not software-center. And if aptdaemon crashes, the apt process itself would crash I guess. If Apt crashes and not aptdaemon, aptdaemon would send a "finished" signal which would clear up the progress bar and remove it from the launcher.

So I guess the current implementation is okay for now, we'll investigate later on if issues arise.

Looks good now. Thanks Bilal!

review: Approve
Gary Lasker (gary-lasker) wrote :

Bilal, this is awesome!! I'm so excited that this is landed. Thank you for your great work, and thank you Mikkel for reviewing!

I'm am wondering if we need to coordinate releasing the USC side of this (currently being held in the branch lp:~gary-lasker/software-center/launcher-integration-lp761851, as you noted above) with the Unity release that contains this change. It definitely seems like we should release the USC side as soon as possible after the Unity update goes out. Please let me know and we can make sure that happens.

Thanks again!!! (/me ^5s Bilal)

Bilal Akhtar (bilalakhtar) wrote :

Yes, Gary's right, we'd surely need a co-ordinated upload.

I'm not sure about when the next Unity precise upload will be, otherwise I'd have acted as a mediator. Could someone from the Unity team point me/Gary at a roadmap OR poke me or Gary when Unity packages with this change get uploaded to the repos?


Michal Hruby (mhr3) wrote :

Hey Bilal, next unity release (5.2) should be next week (most likely Thursday).

Michael Vogt (mvo) wrote :

Once the new unity is uploaded we will upload a matching s-c as well.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/unityshell/src/Launcher.cpp'
2--- plugins/unityshell/src/Launcher.cpp 2012-01-18 11:16:38 +0000
3+++ plugins/unityshell/src/Launcher.cpp 2012-01-20 00:43:27 +0000
4@@ -112,8 +112,8 @@
5 " <interface name='com.canonical.Unity.Launcher'>"
6 ""
7 " <method name='AddLauncherItemFromPosition'>"
8+ " <arg type='s' name='title' direction='in'/>"
9 " <arg type='s' name='icon' direction='in'/>"
10- " <arg type='s' name='title' direction='in'/>"
11 " <arg type='i' name='icon_x' direction='in'/>"
12 " <arg type='i' name='icon_y' direction='in'/>"
13 " <arg type='i' name='icon_size' direction='in'/>"
14@@ -3300,10 +3300,10 @@
15 gchar* desktop_file;
16 gchar* aptdaemon_task;
18- g_variant_get(parameters, "(ssiiiss)", &icon, &title, &icon_x, &icon_y, &icon_size, &desktop_file, &aptdaemon_task, NULL);
19+ g_variant_get(parameters, "(ssiiiss)", &title, &icon, &icon_x, &icon_y, &icon_size, &desktop_file, &aptdaemon_task, NULL);
21 Launcher* self = (Launcher*)user_data;
22- self->launcher_addrequest.emit(desktop_file, NULL);
23+ self->launcher_addrequest_special.emit(desktop_file, NULL, aptdaemon_task, icon);
25 g_dbus_method_invocation_return_value(invocation, NULL);
26 g_free(icon);
28=== modified file 'plugins/unityshell/src/Launcher.h'
29--- plugins/unityshell/src/Launcher.h 2012-01-17 15:41:30 +0000
30+++ plugins/unityshell/src/Launcher.h 2012-01-20 00:43:27 +0000
31@@ -196,6 +196,7 @@
32 void EnableCheckWindowOverLauncher(gboolean enabled);
34 sigc::signal<void, char*, LauncherIcon*> launcher_addrequest;
35+ sigc::signal<void, char*, LauncherIcon*, char*, char*> launcher_addrequest_special;
36 sigc::signal<void, LauncherIcon*> launcher_removerequest;
37 sigc::signal<void> selection_change;
38 sigc::signal<void> hidden_changed;
40=== modified file 'plugins/unityshell/src/LauncherController.cpp'
41--- plugins/unityshell/src/LauncherController.cpp 2012-01-10 15:26:51 +0000
42+++ plugins/unityshell/src/LauncherController.cpp 2012-01-20 00:43:27 +0000
43@@ -38,12 +38,12 @@
44 #include "LauncherEntryRemote.h"
45 #include "LauncherEntryRemoteModel.h"
46 #include "LauncherIcon.h"
47+#include "SoftwareCenterLauncherIcon.h"
48 #include "LauncherModel.h"
49 #include "WindowManager.h"
50 #include "TrashLauncherIcon.h"
51 #include "BFBLauncherIcon.h"
54 namespace unity
55 {
56 namespace launcher
57@@ -68,6 +68,7 @@
58 void OnIconRemoved(LauncherIcon* icon);
60 void OnLauncherAddRequest(char* path, LauncherIcon* before);
61+ void OnLauncherAddRequestSpecial(char* path, LauncherIcon* before, char* aptdaemon_trans_id, char* icon_path);
62 void OnLauncherRemoveRequest(LauncherIcon* icon);
64 void OnLauncherEntryRemoteAdded(LauncherEntryRemote* entry);
65@@ -85,6 +86,8 @@
67 LauncherIcon* CreateFavorite(const char* file_path);
69+ SoftwareCenterLauncherIcon* CreateSCLauncherIcon(const char* file_path, const char* aptdaemon_trans_id, char* icon_path);
71 void SetupBamf();
73 void OnExpoActivated();
74@@ -148,6 +151,7 @@
76 launcher_->SetModel(model_.get());
77 launcher_->launcher_addrequest.connect(sigc::mem_fun(this, &Impl::OnLauncherAddRequest));
78+ launcher_->launcher_addrequest_special.connect(sigc::mem_fun(this, &Impl::OnLauncherAddRequestSpecial));
79 launcher_->launcher_removerequest.connect(sigc::mem_fun(this, &Impl::OnLauncherRemoveRequest));
81 device_section_ = new DeviceLauncherSection(raw_launcher);
82@@ -236,6 +240,30 @@
83 unity::FavoriteStore::GetDefault().SetFavorites(desktop_paths);
84 }
87+Controller::Impl::OnLauncherAddRequestSpecial(char* path, LauncherIcon* before, char* aptdaemon_trans_id, char* icon_path)
89+ std::list<BamfLauncherIcon*> launchers;
90+ std::list<BamfLauncherIcon*>::iterator it;
92+ launchers = model_->GetSublist<BamfLauncherIcon> ();
93+ for (it = launchers.begin(); it != launchers.end(); it++)
94+ {
95+ if (g_strcmp0(path, (*it)->DesktopFile()) == 0)
96+ return;
97+ }
99+ SoftwareCenterLauncherIcon* result = CreateSCLauncherIcon(path, aptdaemon_trans_id, icon_path);
100+ if (result)
101+ {
102+ RegisterIcon(result);
104+ if (before)
105+ model_->ReorderBefore(result, before, false);
106+ }
107+ Save();
110 void Controller::Impl::SortAndUpdate()
111 {
112 gint shortcut = 1;
113@@ -450,6 +478,32 @@
114 return icon;
115 }
118+Controller::Impl::CreateSCLauncherIcon(const char* file_path, const char* aptdaemon_trans_id, char* icon_path)
120+ BamfApplication* app;
121+ SoftwareCenterLauncherIcon* icon;
123+ app = bamf_matcher_get_application_for_desktop_file(matcher_, file_path, true);
124+ if (!BAMF_IS_APPLICATION(app))
125+ return NULL;
127+ if (g_object_get_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen")))
128+ {
129+ bamf_view_set_sticky(BAMF_VIEW(app), true);
130+ return 0;
131+ }
133+ g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"), GINT_TO_POINTER(1));
135+ bamf_view_set_sticky(BAMF_VIEW(app), true);
136+ icon = new SoftwareCenterLauncherIcon(launcher_.GetPointer(), app, (char*)aptdaemon_trans_id, icon_path);
137+ icon->SetIconType(LauncherIcon::TYPE_APPLICATION);
138+ icon->SetSortPriority(sort_priority_++);
140+ return icon;
143 void Controller::Impl::SetupBamf()
144 {
145 GList* apps, *l;
147=== modified file 'plugins/unityshell/src/LauncherController.h'
148--- plugins/unityshell/src/LauncherController.h 2011-11-30 03:06:44 +0000
149+++ plugins/unityshell/src/LauncherController.h 2012-01-20 00:43:27 +0000
150@@ -26,6 +26,8 @@
151 #include <sigc++/sigc++.h>
152 #include <core/core.h>
154+#include "SoftwareCenterLauncherIcon.h"
156 namespace unity
157 {
158 namespace launcher
160=== added file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp'
161--- plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp 1970-01-01 00:00:00 +0000
162+++ plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp 2012-01-20 00:43:27 +0000
163@@ -0,0 +1,102 @@
164+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
166+ * Copyright (C) 2012 Canonical Ltd
167+ *
168+ * This program is free software: you can redistribute it and/or modify
169+ * it under the terms of the GNU General Public License version 3 as
170+ * published by the Free Software Foundation.
171+ *
172+ * This program is distributed in the hope that it will be useful,
173+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
175+ * GNU General Public License for more details.
176+ *
177+ * You should have received a copy of the GNU General Public License
178+ * along with this program. If not, see <>.
179+ *
180+ * Authored by: Bilal Akhtar <>
181+ */
183+#include "Nux/Nux.h"
184+#include "Nux/BaseWindow.h"
186+#include "BamfLauncherIcon.h"
187+#include "Launcher.h"
188+#include "LauncherController.h"
189+#include "PluginAdapter.h"
190+#include "FavoriteStore.h"
192+#include "ubus-server.h"
193+#include "UBusMessages.h"
195+#include <glib.h>
196+#include <glib/gvariant.h>
197+#include <glib/gi18n-lib.h>
198+#include <gio/gio.h>
199+#include <libindicator/indicator-desktop-shortcuts.h>
200+#include <core/core.h>
201+#include <core/atoms.h>
203+#include "SoftwareCenterLauncherIcon.h"
205+namespace unity
207+namespace launcher
210+SoftwareCenterLauncherIcon::SoftwareCenterLauncherIcon(Launcher* IconManager, BamfApplication* app, char* aptdaemon_trans_id, char* icon_path)
211+: BamfLauncherIcon(IconManager, app)
213+ _aptdaemon_trans_id = aptdaemon_trans_id;
215+ g_debug("Aptdaemon transaction ID: %s", _aptdaemon_trans_id);
217+ _aptdaemon_trans = new unity::glib::DBusProxy("org.debian.apt",
218+ _aptdaemon_trans_id,
219+ "org.debian.apt.transaction",
223+ _aptdaemon_trans->Connect("Finished", sigc::mem_fun(this, &SoftwareCenterLauncherIcon::OnFinished));
224+ _aptdaemon_trans->Connect("PropertyChanged", sigc::mem_fun(this, &SoftwareCenterLauncherIcon::OnPropertyChanged));
226+ icon_name = icon_path;
229+SoftwareCenterLauncherIcon::~SoftwareCenterLauncherIcon() {
234+SoftwareCenterLauncherIcon::OnFinished(GVariant* params) {
236+ tooltip_text = BamfName();
238+ SetQuirk(LauncherIcon::QUIRK_PROGRESS, FALSE);
242+SoftwareCenterLauncherIcon::OnPropertyChanged(GVariant* params) {
244+ gint32 progress;
245+ gchar* property_name;
246+ GVariant* property_value;
248+ g_variant_get_child (params, 0, "s", &property_name);
249+ if (g_strcmp0 (property_name, "Progress") == 0) {
250+ g_variant_get_child (params,1,"v",&property_value);
251+ g_variant_get (property_value, "i", &progress);
253+ if (progress < 100) {
254+ SetQuirk(LauncherIcon::QUIRK_PROGRESS, TRUE);
255+ tooltip_text = _("Waiting to install");
256+ }
257+ SetProgress(((float)progress) / ((float)100));
258+ }
259+ g_variant_unref(property_value);
260+ g_free(property_name);
267=== added file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.h'
268--- plugins/unityshell/src/SoftwareCenterLauncherIcon.h 1970-01-01 00:00:00 +0000
269+++ plugins/unityshell/src/SoftwareCenterLauncherIcon.h 2012-01-20 00:43:27 +0000
270@@ -0,0 +1,60 @@
271+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
273+ * Copyright (C) 2010 Canonical Ltd
274+ *
275+ * This program is free software: you can redistribute it and/or modify
276+ * it under the terms of the GNU General Public License version 3 as
277+ * published by the Free Software Foundation.
278+ *
279+ * This program is distributed in the hope that it will be useful,
280+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
282+ * GNU General Public License for more details.
283+ *
284+ * You should have received a copy of the GNU General Public License
285+ * along with this program. If not, see <>.
286+ *
287+ * Authored by: Bilal Akhtar <>
288+ */
293+#include "BamfLauncherIcon.h"
294+#include <Nux/BaseWindow.h>
295+#include <NuxCore/Math/MathInc.h>
296+#include <core/core.h>
297+#include <gio/gio.h>
298+#include <glib.h>
299+#include <glib/gvariant.h>
300+#include <UnityCore/GLibDBusProxy.h>
303+namespace unity
305+namespace launcher
309+class SoftwareCenterLauncherIcon : public BamfLauncherIcon
313+ SoftwareCenterLauncherIcon(Launcher* IconManager, BamfApplication* app, char* aptdaemon_trans_id, char* icon_path);
314+ virtual ~SoftwareCenterLauncherIcon();
316+ gchar* original_tooltip_text;
319+ char* _aptdaemon_trans_id;
320+ unity::glib::DBusProxy* _aptdaemon_trans;
322+ void OnFinished(GVariant* params);
324+ void OnPropertyChanged(GVariant* params);