Merge lp:~mvo/unity/usc-launcher-fix-982921 into lp:unity

[Michael Vogt]

Check for "exit_state" on the install-finished signal from aptdaemon (LP: #982921) and remove launcher icon again on failure.

This branch adds a check for the "exit_state" of the install-finished signal from aptdaemon for the
software-center launcher integration and removes the launcher again if we get something other than
"exit-success". This fixes #982921.

[Tim Penhey (thumper)]

review: Approve

[Didier Roche-Tolomelli (didrocks)]

@Tim, @Michael: I would like to see tests on that one before approving :)

[Michael Vogt (mvo)]

Hey Didier,

I'm not familiar with untiy much, so pardon my ignorance. But I could not find a existing test for the SoftwareCenterLauncherIcon or the dbus signal(s) that are used for it. If you could point me towards that
I can see how to add a test.

[Didier Roche-Tolomelli (didrocks)]

Hey Michael.

Rev 2183 is definitively what you want to look for.
The autopilot test (autopilot is controling the session graphically) is in test/autopilot/autopilot/tests/test_launcher.py: test_software_center_add_icon()

Maybe Thomi who is the most familiar for autopilot can cook something really quick and easily though?

[Thomi Richards (thomir-deactivatedaccount)]

Hi everyone,

Short answer: Testing this is tricky.

Long answer: Testing this is tricky. The software center launcher icon is tightly coupled to the apt daemon dbus protocol, which makes it hard to test from autopilot. In fact, even writing unit tests for this is tricky. What we need to do is to decouple the code that handles the launcher icon from the notion of a data source for the icon. This will allow us to create a fake data source in a unit test and feed it whatever input we like. However, looking at the code, I suspect that's not going to be the end of the issues - the SCLauncherIcon also depends on the Launcher for several things, and a couple of other classes as well. With some refactoring we can fix all this, but it's a reasonable amount of work.

If this is a really high priority I can have a look at it (dropping the tasks I'm currently on), but it's not likely to be done before tomorrow lunchtime.

What's the deadline on this?

[Didier Roche-Tolomelli (didrocks)]

@Thomi: I'm happy to merge it if Michael adds a manual test to ensure that we don't loose the idea of getting it automatically tested later on.

[Michael Vogt (mvo)]

Thanks, I added a manual test now. Sorry about the needed refactor for a proper fix but this was just a drive-by-fix by me.

review: Needs Resubmitting

[Unity Merger (unity-merger)]

No commit message specified.

=== added file 'manual-tests/SoftwareCenter.txt'
--- manual-tests/SoftwareCenter.txt	1970-01-01 00:00:00 +0000
+++ manual-tests/SoftwareCenter.txt	2012-04-26 08:10:23 +0000
@@ -0,0 +1,16 @@
+The SoftwareCenter Launcher Integration cancel
+----------------------------------------------
+
+Setup: 
+#. Open software-center
+
+Actions:  
+#. Click on install on a big application like wesnoth
+#. Verify that the icon "flies" into the launcher
+#. Wait until the "In Progress" appears in the toplevel toolbar of s-c
+#. Click on the "In Progress" button in the toolbar
+#. Click on the "cancel" button of the install of the application
+
+Expected Result:
+  The icon is removed from the launcher again after the download was
+  canceled.

=== modified file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp'
--- plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp	2012-03-28 21:03:59 +0000
+++ plugins/unityshell/src/SoftwareCenterLauncherIcon.cpp	2012-04-26 08:10:23 +0000
@@ -46,15 +46,7 @@
 {
 
   aptdaemon_trans_.Connect("PropertyChanged", sigc::mem_fun(this, &SoftwareCenterLauncherIcon::OnPropertyChanged));
-  aptdaemon_trans_.Connect("Finished", [&] (GVariant *)
-  {
-    tooltip_text = BamfName();
-    SetQuirk(QUIRK_PROGRESS, false);
-    SetQuirk(QUIRK_URGENT, true);
-    SetProgress(0.0f);
-    finished_ = true;
-    needs_urgent_ = true;
-  });
+  aptdaemon_trans_.Connect("Finished", sigc::mem_fun(this, &SoftwareCenterLauncherIcon::OnFinished));
 
   SetIconType(TYPE_APPLICATION);
   icon_name = icon_path;
@@ -130,6 +122,25 @@
       SetQuirk(QUIRK_STARTING, false);
 }
 
+void SoftwareCenterLauncherIcon::OnFinished(GVariant *params)
+{
+   glib::String exit_state;
+   g_variant_get_child(params, 0, "s", &exit_state);
+
+   if (exit_state.Str() == "exit-success")
+   {
+      tooltip_text = BamfName();
+      SetQuirk(QUIRK_PROGRESS, false);
+      SetQuirk(QUIRK_URGENT, true);
+      SetProgress(0.0f);
+      finished_ = true;
+      needs_urgent_ = true;
+   } else {
+      // failure condition, remove icon again
+      UnStick();
+   }
+};
+
 void SoftwareCenterLauncherIcon::OnPropertyChanged(GVariant* params)
 {
   gint32 progress;

=== modified file 'plugins/unityshell/src/SoftwareCenterLauncherIcon.h'
--- plugins/unityshell/src/SoftwareCenterLauncherIcon.h	2012-03-28 02:42:13 +0000
+++ plugins/unityshell/src/SoftwareCenterLauncherIcon.h	2012-04-26 08:10:23 +0000
@@ -51,6 +51,7 @@
 
 private:
   void OnPropertyChanged(GVariant* params);
+  void OnFinished(GVariant *params);
   void OnDragAnimationFinished();
 
   glib::DBusProxy aptdaemon_trans_;