Merge lp:~3v1n0/unity/app-icon-ensure-on-running into lp:unity

[Marco Trevisan (Treviño)]

ApplicationLauncherIcon: ensure the icon and name values when the running state changes

This avoids to get "?" icons

It can happen that a bamf view is opened for very few ms after being closed and again reopened, in that case unity is sometimes not enough fast to cache the view's name and icon.
So as soon as this gets running again we should update these values.

Added a new mock bamf application to allow testing of this, plus new unit tests.

[Brandon Schaefer (brandontschaefer)]

The code looks good, the fix is in the first 80 line of the diff. Most of it is testing, which is good :)

review: Approve

[jenkins (martin-mrazik+qa)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/29/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/29/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/29/console

review: Needs Fixing (continuous-integration)

=== modified file 'launcher/ApplicationLauncherIcon.cpp'
--- launcher/ApplicationLauncherIcon.cpp	2012-10-04 08:00:29 +0000
+++ launcher/ApplicationLauncherIcon.cpp	2012-10-12 16:30:44 +0000
@@ -79,7 +79,7 @@
   glib::SignalBase* sig;
 
   sig = new glib::Signal<void, BamfView*, BamfView*>(bamf_view, "child-added",
-                          [&] (BamfView*, BamfView*) {
+                          [this] (BamfView*, BamfView*) {
                             EnsureWindowState();
                             UpdateMenus();
                             UpdateIconGeometries(GetCenters());
@@ -87,7 +87,7 @@
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, BamfView*>(bamf_view, "child-removed",
-                          [&] (BamfView*, BamfView*) { EnsureWindowState(); });
+                          [this] (BamfView*, BamfView*) { EnsureWindowState(); });
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, BamfView*>(bamf_view, "child-moved",
@@ -97,39 +97,53 @@
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "urgent-changed",
-                          [&] (BamfView*, gboolean urgent) {
+                          [this] (BamfView*, gboolean urgent) {
                             SetQuirk(Quirk::URGENT, urgent);
                           });
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "active-changed",
-                          [&] (BamfView*, gboolean active) {
+                          [this] (BamfView*, gboolean active) {
                             SetQuirk(Quirk::ACTIVE, active);
                           });
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "running-changed",
-                          [&] (BamfView*, gboolean running) {
+                          [this] (BamfView* view, gboolean running) {
                             SetQuirk(Quirk::RUNNING, running);
 
                             if (running)
                             {
+                              _source_manager.Remove(ICON_REMOVE_TIMEOUT);
+
+                              /* It can happen that these values are not set
+                               * during initialization if the view is closed
+                               * very early, so we need to make sure that they
+                               * are updated as soon as the view is re-opened. */
+                              if (tooltip_text().empty())
+                                tooltip_text = BamfName();
+
+                              if (icon_name == DEFAULT_ICON)
+                              {
+                                glib::String icon(bamf_view_get_icon(view));
+                                icon_name = (icon ? icon.Str() : DEFAULT_ICON);
+                              }
+
                               EnsureWindowState();
                               UpdateIconGeometries(GetCenters());
-                              _source_manager.Remove(ICON_REMOVE_TIMEOUT);
                             }
                           });
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view, "user-visible-changed",
-                          [&] (BamfView*, gboolean visible) {
+                          [this] (BamfView*, gboolean visible) {
                             if (!IsSticky())
                               SetQuirk(Quirk::VISIBLE, visible);
                           });
   _gsignals.Add(sig);
 
   sig = new glib::Signal<void, BamfView*>(bamf_view, "closed",
-                          [&] (BamfView*) {
+                          [this] (BamfView*) {
                             if (!IsSticky())
                             {
                               SetQuirk(Quirk::VISIBLE, false);

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2012-10-09 09:48:07 +0000
+++ tests/CMakeLists.txt	2012-10-12 16:30:44 +0000
@@ -244,6 +244,7 @@
                  test_unity_settings.cpp
                  test_volume_imp.cpp
                  test_volume_launcher_icon.cpp
+                 bamf-mock-application.c
                  gmockmount.c
                  gmockvolume.c
                  ${CMAKE_SOURCE_DIR}/dash/DashViewPrivate.cpp

=== added file 'tests/bamf-mock-application.c'
--- tests/bamf-mock-application.c	1970-01-01 00:00:00 +0000
+++ tests/bamf-mock-application.c	2012-10-12 16:30:44 +0000
@@ -0,0 +1,201 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3, as
+ * published by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the applicable version of the GNU Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of both the GNU Lesser General Public
+ * License version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Marco Trevisan <[e-mail]>
+ *
+ */
+
+#include "bamf-mock-application.h"
+
+G_DEFINE_TYPE (BamfMockApplication, bamf_mock_application, BAMF_TYPE_APPLICATION);
+
+#define BAMF_MOCK_APPLICATION_GET_PRIVATE(o) \
+  (G_TYPE_INSTANCE_GET_PRIVATE ((o), BAMF_TYPE_MOCK_APPLICATION, BamfMockApplicationPrivate))
+
+struct _BamfMockApplicationPrivate
+{
+  gboolean active;
+  gboolean running;
+  gboolean urgent;
+  gchar * name;
+  gchar * icon;
+  GList * children;
+};
+
+void
+bamf_mock_application_set_active (BamfMockApplication * self, gboolean active)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  if (self->priv->active != active)
+  {
+    self->priv->active = active;
+    g_signal_emit_by_name (G_OBJECT (self), "active-changed", active, NULL);
+  }
+}
+
+void
+bamf_mock_application_set_running (BamfMockApplication * self, gboolean running)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  if (self->priv->running != running)
+  {
+    self->priv->running = running;
+    g_signal_emit_by_name (G_OBJECT (self), "running-changed", running, NULL);
+  }
+}
+
+void
+bamf_mock_application_set_urgent (BamfMockApplication * self, gboolean urgent)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  if (self->priv->urgent != urgent)
+  {
+    self->priv->urgent = urgent;
+    g_signal_emit_by_name (G_OBJECT (self), "urgent-changed", urgent, NULL);
+  }
+}
+
+void
+bamf_mock_application_set_name (BamfMockApplication * self, const gchar * name)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  if (g_strcmp0 (self->priv->name, name) != 0)
+  {
+    char *old = self->priv->name;
+    self->priv->name = g_strdup (name);
+    g_signal_emit_by_name (G_OBJECT (self), "name-changed", old, self->priv->name, NULL);
+    g_free (old);
+  }
+}
+
+void
+bamf_mock_application_set_icon (BamfMockApplication * self, const gchar * icon)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  g_free (self->priv->icon);
+  self->priv->icon = g_strdup (icon);
+}
+
+void
+bamf_mock_application_set_children (BamfMockApplication * self, GList * children)
+{
+  g_return_if_fail (BAMF_IS_MOCK_APPLICATION (self));
+
+  g_list_free (self->priv->children);
+  self->priv->children = g_list_copy (children);
+}
+
+static void
+bamf_mock_application_finalize (GObject *object)
+{
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (object);
+
+  g_free (self->priv->name);
+  g_free (self->priv->icon);
+  g_list_free (self->priv->children);
+}
+
+static GList *
+bamf_mock_application_get_children (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), NULL);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return g_list_copy (self->priv->children);
+}
+
+static gboolean
+bamf_mock_application_is_active (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), FALSE);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return self->priv->active;
+}
+
+static gboolean
+bamf_mock_application_is_running (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), FALSE);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return self->priv->running;
+}
+
+static gboolean
+bamf_mock_application_is_urgent (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), FALSE);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return self->priv->urgent;
+}
+
+static char *
+bamf_mock_application_get_name (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), NULL);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return g_strdup (self->priv->name);
+}
+
+static char *
+bamf_mock_application_get_icon (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), NULL);
+  BamfMockApplication *self = BAMF_MOCK_APPLICATION (view);
+  return g_strdup (self->priv->icon);
+}
+
+static const char *
+bamf_mock_application_view_type (BamfView *view)
+{
+  g_return_val_if_fail (BAMF_IS_MOCK_APPLICATION (view), NULL);
+  return "mock-application";
+}
+
+static void
+bamf_mock_application_class_init (BamfMockApplicationClass *klass)
+{
+  GObjectClass *obj_class = G_OBJECT_CLASS (klass);
+  BamfViewClass *view_class = BAMF_VIEW_CLASS (klass);
+
+  obj_class->finalize = bamf_mock_application_finalize;
+  view_class->get_children = bamf_mock_application_get_children;
+  view_class->is_active = bamf_mock_application_is_active;
+  view_class->is_running = bamf_mock_application_is_running;
+  view_class->is_urgent = bamf_mock_application_is_urgent;
+  view_class->get_name = bamf_mock_application_get_name;
+  view_class->get_icon = bamf_mock_application_get_icon;
+  view_class->view_type = bamf_mock_application_view_type;
+
+  g_type_class_add_private (obj_class, sizeof (BamfMockApplicationPrivate));
+}
+
+static void
+bamf_mock_application_init (BamfMockApplication *self)
+{
+  self->priv = BAMF_MOCK_APPLICATION_GET_PRIVATE (self);
+}
+
+BamfMockApplication *
+bamf_mock_application_new ()
+{
+  return g_object_new (BAMF_TYPE_MOCK_APPLICATION, NULL);
+}
\ No newline at end of file

=== added file 'tests/bamf-mock-application.h'
--- tests/bamf-mock-application.h	1970-01-01 00:00:00 +0000
+++ tests/bamf-mock-application.h	2012-10-12 16:30:44 +0000
@@ -0,0 +1,77 @@
+// -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
+/*
+ * Copyright 2012 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3, as
+ * published by the  Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the applicable version of the GNU Lesser General Public
+ * License for more details.
+ *
+ * You should have received a copy of both the GNU Lesser General Public
+ * License version 3 along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>
+ *
+ * Authored by: Marco Trevisan <[e-mail]>
+ *
+ */
+
+#ifndef MOCK_BAMF_MOCK_APPLICATION
+#define MOCK_BAMF_MOCK_APPLICATION
+
+#include <libbamf/libbamf.h>
+
+G_BEGIN_DECLS
+
+#define BAMF_TYPE_MOCK_APPLICATION (bamf_mock_application_get_type ())
+
+#define BAMF_MOCK_APPLICATION(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj),\
+        BAMF_TYPE_MOCK_APPLICATION, BamfMockApplication))
+
+#define BAMF_MOCK_APPLICATION_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass),\
+        BAMF_TYPE_MOCK_APPLICATION, BamfMockApplicationClass))
+
+#define BAMF_IS_MOCK_APPLICATION(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj),\
+        BAMF_TYPE_MOCK_APPLICATION))
+
+#define BAMF_IS_MOCK_APPLICATION_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass),\
+        BAMF_TYPE_MOCK_APPLICATION))
+
+#define BAMF_MOCK_APPLICATION_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj),\
+        BAMF_TYPE_MOCK_APPLICATION, BamfMockApplicationClass))
+
+typedef struct _BamfMockApplication        BamfMockApplication;
+typedef struct _BamfMockApplicationClass   BamfMockApplicationClass;
+typedef struct _BamfMockApplicationPrivate BamfMockApplicationPrivate;
+
+struct _BamfMockApplication
+{
+  BamfApplication parent;
+
+  BamfMockApplicationPrivate *priv;
+};
+
+struct _BamfMockApplicationClass
+{
+  BamfApplicationClass parent_class;
+};
+
+GType bamf_mock_application_get_type (void) G_GNUC_CONST;
+
+BamfMockApplication * bamf_mock_application_new ();
+
+void bamf_mock_application_set_active (BamfMockApplication * self, gboolean active);
+void bamf_mock_application_set_running (BamfMockApplication * self, gboolean running);
+void bamf_mock_application_set_urgent (BamfMockApplication * self, gboolean urgent);
+void bamf_mock_application_set_name (BamfMockApplication * self, const gchar * name);
+void bamf_mock_application_set_icon (BamfMockApplication * self, const gchar * icon);
+void bamf_mock_application_set_children (BamfMockApplication * self, GList * children);
+
+G_END_DECLS
+
+#endif
+

=== modified file 'tests/test_application_launcher_icon.cpp'
--- tests/test_application_launcher_icon.cpp	2012-10-04 08:00:29 +0000
+++ tests/test_application_launcher_icon.cpp	2012-10-12 16:30:44 +0000
@@ -27,6 +27,7 @@
 
 #include "ApplicationLauncherIcon.h"
 #include "FavoriteStore.h"
+#include "bamf-mock-application.h"
 
 using namespace unity;
 using namespace unity::launcher;
@@ -34,6 +35,7 @@
 namespace
 {
 
+const std::string DEFAULT_EMPTY_ICON = "application-default-icon";
 const std::string USC_DESKTOP = BUILDDIR"/tests/data/applications/ubuntu-software-center.desktop";
 const std::string NO_ICON_DESKTOP = BUILDDIR"/tests/data/applications/no-icon.desktop";
 
@@ -53,15 +55,16 @@
     empty_icon = new launcher::ApplicationLauncherIcon(bamf_app);
     ASSERT_EQ(empty_icon->DesktopFile(), NO_ICON_DESKTOP);
 
-    bamf_app = static_cast<BamfApplication*>(g_object_new(BAMF_TYPE_APPLICATION, nullptr));
-    empty_app = new launcher::ApplicationLauncherIcon(bamf_app);
-    ASSERT_TRUE(empty_app->DesktopFile().empty());
+    mock_app = bamf_mock_application_new();
+    mock_icon = new launcher::ApplicationLauncherIcon(glib::object_cast<BamfApplication>(mock_app));
+    ASSERT_TRUE(mock_icon->DesktopFile().empty());
   }
 
   glib::Object<BamfMatcher> bamf_matcher;
+  glib::Object<BamfMockApplication> mock_app;
   nux::ObjectPtr<launcher::ApplicationLauncherIcon> usc_icon;
   nux::ObjectPtr<launcher::ApplicationLauncherIcon> empty_icon;
-  nux::ObjectPtr<launcher::ApplicationLauncherIcon> empty_app;
+  nux::ObjectPtr<launcher::ApplicationLauncherIcon> mock_icon;
 };
 
 TEST_F(TestApplicationLauncherIcon, Position)
@@ -81,9 +84,9 @@
 
 TEST_F(TestApplicationLauncherIcon, TestDefaultIcon)
 {
-  EXPECT_EQ(usc_icon->icon_name.Get(), "softwarecenter");
-  EXPECT_EQ(empty_icon->icon_name.Get(), "application-default-icon");
-  EXPECT_EQ(empty_app->icon_name.Get(), "application-default-icon");
+  EXPECT_EQ(usc_icon->icon_name(), "softwarecenter");
+  EXPECT_EQ(empty_icon->icon_name(), DEFAULT_EMPTY_ICON);
+  EXPECT_EQ(mock_icon->icon_name(), DEFAULT_EMPTY_ICON);
 }
 
 TEST_F(TestApplicationLauncherIcon, Stick)
@@ -143,7 +146,39 @@
 TEST_F(TestApplicationLauncherIcon, RemoteUri)
 {
   EXPECT_EQ(usc_icon->RemoteUri(), FavoriteStore::URI_PREFIX_APP + DesktopUtilities::GetDesktopID(USC_DESKTOP));
-  EXPECT_TRUE(empty_app->RemoteUri().empty());
+  EXPECT_TRUE(mock_icon->RemoteUri().empty());
+}
+
+TEST_F(TestApplicationLauncherIcon, EmptyTooltipUpdatesOnRunning)
+{
+  ASSERT_TRUE(mock_icon->tooltip_text().empty());
+  bamf_mock_application_set_name (mock_app, "Got Name");
+
+  ASSERT_TRUE(mock_icon->tooltip_text().empty());
+
+  bamf_mock_application_set_running(mock_app, TRUE);
+  EXPECT_EQ(mock_icon->tooltip_text(), "Got Name");
+
+  bamf_mock_application_set_running(mock_app, FALSE);
+  bamf_mock_application_set_name (mock_app, "New Name");
+  bamf_mock_application_set_running(mock_app, TRUE);
+  EXPECT_EQ(mock_icon->tooltip_text(), "Got Name");
+}
+
+TEST_F(TestApplicationLauncherIcon, InvalidIconUpdatesOnRunning)
+{
+  ASSERT_EQ(mock_icon->icon_name(), DEFAULT_EMPTY_ICON);
+  bamf_mock_application_set_icon (mock_app, "icon-name");
+
+  ASSERT_EQ(mock_icon->icon_name(), DEFAULT_EMPTY_ICON);
+
+  bamf_mock_application_set_running(mock_app, TRUE);
+  EXPECT_EQ(mock_icon->icon_name(), "icon-name");
+
+  bamf_mock_application_set_running(mock_app, FALSE);
+  bamf_mock_application_set_icon (mock_app, "new-icon-name");
+  bamf_mock_application_set_running(mock_app, TRUE);
+  EXPECT_EQ(mock_icon->icon_name(), "icon-name");
 }
 
 }

=== modified file 'tests/test_launcher_controller.cpp'
--- tests/test_launcher_controller.cpp	2012-10-04 08:00:29 +0000
+++ tests/test_launcher_controller.cpp	2012-10-12 16:30:44 +0000
@@ -18,7 +18,6 @@
  */
 
 #include <gmock/gmock.h>
-#include "test_uscreen_mock.h"
 
 #include "FavoriteStore.h"
 #include "LauncherController.h"
@@ -34,7 +33,9 @@
 #include "PanelStyle.h"
 #include "UnitySettings.h"
 #include "test_utils.h"
+#include "test_uscreen_mock.h"
 #include "test_mock_devices.h"
+#include "bamf-mock-application.h"
 
 using namespace unity::launcher;
 using namespace testing;
@@ -131,7 +132,7 @@
   typedef bool Fake;
 
   MockApplicationLauncherIcon(Fake = true, std::string const& remote_uri = "")
-    : ApplicationLauncherIcon(static_cast<BamfApplication*>(g_object_new(BAMF_TYPE_APPLICATION, nullptr)))
+    : ApplicationLauncherIcon(BAMF_APPLICATION(bamf_mock_application_new()))
     , remote_uri_(remote_uri)
   {
     InitMock();