Merge lp:~3v1n0/unity/quick-app-reopen-icon-fix into lp:unity

[Marco Trevisan (Treviño)]

BamfLauncherIcon: add a timeout on application close before removing the icon

This makes the application to correctly hide when closed, but avoids
that the launcher icon is removed when an application is quickly opened/closed

Also we override the Remove() method to make sure that once it's called
we remove the "unity-seen" flag from the BamfApplication object.
So an application icon could be re-added in the period between the Remove()
call and the ~BamfLauncherIcon call (that could take some time).

If an application (i.e. GIMP, Hugin ...) quickly opens and closes a window so that the application is considered closed for few milliseconds, unity doesn't handle correctly the application re-open.

This is caused by the fact that when SimpleLauncherIcon::Remove() is called, then unity takes few (milli)seconds to actually remove the icon from the launcher, and then to call ~BamfLauncherIcon that unsets the "unity-seen" flag.
This makes the checks in LauncherController not to re-add an icon for the just re-opened application (as the "unity-seen" guard check fails).

So, to fix this I've overridden the Remove() method to force it to unset the "unity-seen" flag, and added a timeout before calling Remove() on application's close signal, so that if it's quickly re-opened the remove call is cancelled.
This won't cause any delay on actually hiding the icon from the launcher, since that is done when the application is not anymore on running state.

Added autopilot test to check this.

A crash can happen in libbamf with this fix, lp:~3v1n0/bamf/fix-window-removed-crash fixes it.

[Thomi Richards (thomir-deactivatedaccount)]

+1 from me. Probably needs someone to look at the C++ stuff as well.

review: Approve

[Tim Penhey (thumper)]

C++ stuff looks fine

review: Approve

=== modified file 'launcher/BamfLauncherIcon.cpp'
--- launcher/BamfLauncherIcon.cpp	2012-05-07 19:52:54 +0000
+++ launcher/BamfLauncherIcon.cpp	2012-05-15 23:08:19 +0000
@@ -55,7 +55,7 @@
   , _window_moved_id(0)
 {
   g_object_set_qdata(G_OBJECT(app), g_quark_from_static_string("unity-seen"),
-                     GINT_TO_POINTER(1));
+                     GUINT_TO_POINTER(1));
   auto bamf_view = glib::object_cast<BamfView>(_bamf_app);
 
   glib::String icon(bamf_view_get_icon(bamf_view));
@@ -101,10 +101,17 @@
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "running-changed",
                           [&] (BamfView*, gboolean running) {
                             SetQuirk(QUIRK_RUNNING, running);
+
                             if (running)
                             {
                               EnsureWindowState();
                               UpdateIconGeometries(GetCenters());
+
+                              if (_remove_timeout_id)
+                              {
+                                g_source_remove(_remove_timeout_id);
+                                _remove_timeout_id = 0;
+                              }
                             }
                           });
   _gsignals.Add(sig);
@@ -119,7 +126,23 @@
   sig = new glib::Signal<void, BamfView*>(bamf_view, "closed",
                           [&] (BamfView*) {
                             if (!IsSticky())
-                              Remove();
+                            {
+                              /* Use a timeout to remove the icon, this avoids
+                               * that we remove an application that is going
+                               * to be reopened soon. So applications that
+                               * have a splash screen won't be removed from
+                               * the launcher while the splash is closed and
+                               * a new window is opened. */
+                              if (_remove_timeout_id)
+                                g_source_remove(_remove_timeout_id);
+
+                              _remove_timeout_id = g_timeout_add_seconds(1, [] (gpointer data) -> gboolean {
+                                auto self = static_cast<BamfLauncherIcon*>(data);
+                                self->Remove();
+                                self->_remove_timeout_id = 0;
+                                return false;
+                              }, this);
+                            }
                           });
   _gsignals.Add(sig);
 
@@ -147,6 +170,9 @@
   g_object_set_qdata(G_OBJECT(_bamf_app.RawPtr()),
                      g_quark_from_static_string("unity-seen"), nullptr);
 
+  if (_remove_timeout_id != 0)
+    g_source_remove(_remove_timeout_id);
+
   if (_fill_supported_types_id != 0)
     g_source_remove(_fill_supported_types_id);
 
@@ -160,6 +186,18 @@
     g_source_remove(_dnd_hover_timer);
 }
 
+void BamfLauncherIcon::Remove()
+{
+  /* Removing the unity-seen flag to the wrapped bamf application, on remove
+   * request we make sure that if the bamf application is re-opened while
+   * the removal process is still ongoing, the application will be shown
+   * on the launcher. */
+  g_object_set_qdata(G_OBJECT(_bamf_app.RawPtr()),
+                     g_quark_from_static_string("unity-seen"), nullptr);
+
+  SimpleLauncherIcon::Remove();
+}
+
 bool BamfLauncherIcon::IsSticky() const
 {
   return bamf_view_is_sticky(BAMF_VIEW(_bamf_app.RawPtr()));

=== modified file 'launcher/BamfLauncherIcon.h'
--- launcher/BamfLauncherIcon.h	2012-05-07 19:52:54 +0000
+++ launcher/BamfLauncherIcon.h	2012-05-15 23:08:19 +0000
@@ -66,6 +66,7 @@
   std::string NameForWindow(Window window);
 
 protected:
+  void Remove();
   void UpdateIconGeometries(std::vector<nux::Point3> center);
   void OnCenterStabilized(std::vector<nux::Point3> center);
   void AddProperties(GVariantBuilder* builder);
@@ -124,6 +125,7 @@
   guint _dnd_hover_timer;
 
   bool _supported_types_filled;
+  guint _remove_timeout_id;
   guint _fill_supported_types_id;
   guint _window_moved_id;
   guint _quicklist_activated_id;

=== modified file 'tests/autopilot/autopilot/tests/__init__.py'
--- tests/autopilot/autopilot/tests/__init__.py	2012-05-04 17:33:59 +0000
+++ tests/autopilot/autopilot/tests/__init__.py	2012-05-15 23:08:19 +0000
@@ -274,8 +274,9 @@
     def close_all_app(self, app_name):
         """Close all instances of the app_name."""
         app = self.KNOWN_APPS[app_name]
-        self.addCleanup(call, "kill `pidof %s`" % (app['process-name']), shell=True)
-        super(LoggedTestCase, self).tearDown()
+        pids = check_output(["pidof", app['process-name']]).split()
+        if len(pids):
+            call(["kill"] + pids)
 
     def get_app_instances(self, app_name):
         """Get BamfApplication instances for app_name."""

=== modified file 'tests/autopilot/autopilot/tests/test_launcher.py'
--- tests/autopilot/autopilot/tests/test_launcher.py	2012-05-15 19:33:20 +0000
+++ tests/autopilot/autopilot/tests/test_launcher.py	2012-05-15 23:08:19 +0000
@@ -445,6 +445,20 @@
         self.assertTrue(mah_win2.is_hidden)
         self.assertVisibleWindowStack([mah_win1, calc_win])
 
+    def test_icon_shows_on_quick_application_reopen(self):
+        """Icons should stay on launcher when an application is quickly closed/reopened."""
+        calc = self.start_app("Calculator")
+        desktop_file = calc.desktop_file
+        calc_icon = self.launcher.model.get_icon_by_desktop_id(desktop_file)
+        self.assertThat(calc_icon.visible, Eventually(Equals(True)))
+
+        self.close_all_app("Calculator")
+        calc = self.start_app("Calculator")
+        sleep(2)
+
+        calc_icon = self.launcher.model.get_icon_by_desktop_id(desktop_file)
+        self.assertThat(calc_icon, NotEquals(None))
+        self.assertThat(calc_icon.visible, Eventually(Equals(True)))
 
 class LauncherRevealTests(LauncherTestCase):
     """Test the launcher reveal behavior when in autohide mode."""