Merge lp:~3v1n0/unity/app-title-icon-properties into lp:unity

[Marco Trevisan (Treviño)]

ApplicationManager: Use nux::ROProperty for Window and Application title and icon

So BamfApplicationManager connect to {name,icon}-changed signals, set getters for properties
and ApplicationLauncherIcon update icon tooltip_text and icon when App title/icon changes

Use nux::ROProperty's to handle an application title and icon, update the code to work with it and connect to related BamfApplication signals so that we can update these values if they change (unlikely, but it can happen on some desktop-less applications, especially after that lp:~3v1n0/bamf/app-main-view will land).

Removed one workaround that is not needed anymore since we moved to libgdbus properties on libbamf.

Tests updated.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3395
http://jenkins.qa.ubuntu.com/job/unity-ci/156/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-amd64-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-armhf-ci/44
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-saucy-i386-ci/44

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-ci/156/rebuild

review: Approve (continuous-integration)

[Andrea Azzarone (azzar1)]

LGTM.

review: Approve

=== modified file 'debian/control'
--- debian/control	2013-06-26 14:29:55 +0000
+++ debian/control	2013-06-27 10:54:34 +0000
@@ -15,7 +15,7 @@
                libsigc++-2.0-dev,
                libgee-dev (>= 0.5.0),
                libunique-dev,
-               libbamf3-dev (>= 0.4),
+               libbamf3-dev (>= 0.5.0+13.10.20130626),
                gsettings-desktop-schemas-dev,
                libglib2.0-dev (>= 2.30),
                libdbusmenu-glib-dev (>= 0.3.91),
 
=== modified file 'launcher/ApplicationLauncherIcon.cpp'
--- launcher/ApplicationLauncherIcon.cpp	2013-05-17 22:53:57 +0000
+++ launcher/ApplicationLauncherIcon.cpp	2013-06-27 10:54:34 +0000
@@ -69,7 +69,7 @@
   app->seen = true;
 
   tooltip_text = app->title();
-  std::string icon = app->icon();
+  std::string const& icon = app->icon();
   icon_name = (icon.empty() ? DEFAULT_ICON : icon);
 
   SetQuirk(Quirk::VISIBLE, app->visible());
@@ -148,6 +148,16 @@
     SetQuirk(Quirk::ACTIVE, active);
   });
 
+  title_changed_connection_ = app_->title.changed.connect([this](std::string const& name) {
+    LOG_DEBUG(logger) << tooltip_text() << " name now " << name;
+    tooltip_text = name;
+  });
+
+  icon_changed_connection_ = app_->icon.changed.connect([this](std::string const& icon) {
+    LOG_DEBUG(logger) << tooltip_text() << " icon now " << icon;
+    icon_name = (icon.empty() ? DEFAULT_ICON : icon);
+  });
+
   running_changed_connection_ = app_->running.changed.connect([this](bool const& running) {
     LOG_DEBUG(logger) << tooltip_text() << " running now " << (running ? "true" : "false");
     SetQuirk(Quirk::RUNNING, running);
@@ -156,19 +166,6 @@
     {
       _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 = app_->title();
-
-      if (icon_name == DEFAULT_ICON)
-      {
-        std::string icon = app_->icon();
-        icon_name = (icon.empty() ? DEFAULT_ICON : icon);
-      }
-
       EnsureWindowState();
       UpdateIconGeometries(GetCenters());
     }
@@ -207,6 +204,8 @@
   active_changed_connection_.disconnect();
   running_changed_connection_.disconnect();
   visible_changed_connection_.disconnect();
+  title_changed_connection_.disconnect();
+  icon_changed_connection_.disconnect();
   closed_changed_connection_.disconnect();
 }
 
 
=== modified file 'launcher/ApplicationLauncherIcon.h'
--- launcher/ApplicationLauncherIcon.h	2013-05-17 22:53:57 +0000
+++ launcher/ApplicationLauncherIcon.h	2013-06-27 10:54:34 +0000
@@ -153,6 +153,8 @@
   sigc::connection window_opened_connection_;
   sigc::connection window_closed_connection_;
   sigc::connection window_moved_connection_;
+  sigc::connection icon_changed_connection_;
+  sigc::connection title_changed_connection_;
   sigc::connection urgent_changed_connection_;
   sigc::connection active_changed_connection_;
   sigc::connection running_changed_connection_;
 
=== modified file 'launcher/MockLauncherIcon.h'
--- launcher/MockLauncherIcon.h	2013-05-17 22:53:57 +0000
+++ launcher/MockLauncherIcon.h	2013-06-27 10:54:34 +0000
@@ -43,10 +43,12 @@
 class MockApplicationWindow : public ApplicationWindow
 {
 public:
-  MockApplicationWindow(Window xid) : xid_(xid) {}
+  MockApplicationWindow(Window xid) : xid_(xid)
+  {
+    title.SetGetterFunction([this] { return "MockApplicationWindow"; });
+    icon.SetGetterFunction([this] { return ""; });
+  }
 
-  std::string title() const { return "MockApplicationWindow"; }
-  virtual std::string icon() const { return ""; }
   virtual std::string type() const { return "mock"; }
 
   virtual Window window_id() const { return xid_; }
 
=== modified file 'launcher/StandaloneLauncher.cpp'
--- launcher/StandaloneLauncher.cpp	2013-03-19 12:41:22 +0000
+++ launcher/StandaloneLauncher.cpp	2013-06-27 10:54:34 +0000
@@ -21,6 +21,7 @@
 #include <Nux/Nux.h>
 #include <Nux/NuxTimerTickSource.h>
 #include <NuxCore/AnimationController.h>
+#include <NuxCore/Logger.h>
 #include <gtk/gtk.h>
 
 #include "unity-shared/BackgroundEffectHelper.h"
@@ -96,6 +97,7 @@
 int main(int argc, char** argv)
 {
   gtk_init(&argc, &argv);
+  nux::logging::configure_logging(::getenv("UNITY_LOG_SEVERITY"));
   nux::NuxInitialize(0);
 
   LauncherWindow lc;
 
=== modified file 'panel/PanelMenuView.cpp'
--- panel/PanelMenuView.cpp	2013-02-21 13:43:48 +0000
+++ panel/PanelMenuView.cpp	2013-06-27 10:54:34 +0000
@@ -1019,6 +1019,10 @@
 {
   if (BAMF_IS_APPLICATION(new_app))
   {
+    app_name_changed_signal_.Disconnect();
+    app_name_changed_signal_.Connect(BAMF_VIEW(new_app), "name-changed",
+                                     sigc::mem_fun(this, &PanelMenuView::OnNameChanged));
+
     if (std::find(new_apps_.begin(), new_apps_.end(), new_app) != new_apps_.end())
     {
       if (new_application_ != new_app)
@@ -1087,7 +1091,6 @@
 
     // first see if we need to remove and old callback
     view_name_changed_signal_.Disconnect();
-
     // register callback for new view
     view_name_changed_signal_.Connect(new_view, "name-changed",
                                       sigc::mem_fun(this, &PanelMenuView::OnNameChanged));
 
=== modified file 'panel/PanelMenuView.h'
--- panel/PanelMenuView.h	2013-02-01 18:13:40 +0000
+++ panel/PanelMenuView.h	2013-06-27 10:54:34 +0000
@@ -194,6 +194,7 @@
   glib::Signal<void, BamfMatcher*, BamfView*, BamfView*> active_win_changed_signal_;
   glib::Signal<void, BamfMatcher*, BamfApplication*, BamfApplication*> active_app_changed_signal_;
   glib::Signal<void, BamfView*, gchar*, gchar*> view_name_changed_signal_;
+  glib::Signal<void, BamfView*, gchar*, gchar*> app_name_changed_signal_;
   sigc::connection style_changed_connection_;
 
   UBusManager ubus_manager_;
 
=== modified file 'tests/mock-application.h'
--- tests/mock-application.h	2013-04-17 19:16:54 +0000
+++ tests/mock-application.h	2013-06-27 10:54:34 +0000
@@ -40,6 +40,8 @@
     visible.SetGetterFunction([this] { return visible_; });
     active.SetGetterFunction([this] { return active_; });
     urgent.SetGetterFunction([this] { return urgent_; });
+    title.SetGetterFunction([this] { return title_; });
+    icon.SetGetterFunction([this] { return icon_; });
   }
 
   Window xid_;
@@ -52,8 +54,6 @@
   bool active_;
   bool urgent_;
 
-  virtual std::string title() const { return title_; }
-  virtual std::string icon() const { return icon_; }
   virtual std::string type() const { return type_; }
 
   virtual Window window_id() const { return xid_; }
@@ -73,11 +73,11 @@
 struct MockApplication : unity::Application
 {
   MockApplication(std::string const& desktop_file,
-                  std::string const& icon = "",
-                  std::string const& title = "")
+                  std::string const& icon_name = "",
+                  std::string const& title_str = "")
     : desktop_file_(desktop_file)
-    , icon_(icon)
-    , title_(title)
+    , icon_(icon_name)
+    , title_(title_str)
     , seen_(false)
     , sticky_(false)
     , visible_(false)
@@ -86,14 +86,17 @@
     , urgent_(false)
     , type_("mock")
     {
-      seen.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetSeen));
       seen.SetSetterFunction(sigc::mem_fun(this, &MockApplication::SetSeen));
-      sticky.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetSticky));
       sticky.SetSetterFunction(sigc::mem_fun(this, &MockApplication::SetSticky));
-      visible.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetVisible));
-      active.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetActive));
-      running.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetRunning));
-      urgent.SetGetterFunction(sigc::mem_fun(this, &MockApplication::GetUrgent));
+
+      seen.SetGetterFunction([this] { return seen_; });
+      sticky.SetGetterFunction([this] { return sticky_; });
+      visible.SetGetterFunction([this] { return visible_; });
+      active.SetGetterFunction([this] { return active_; });
+      running.SetGetterFunction([this] { return running_; });
+      urgent.SetGetterFunction([this] { return urgent_; });
+      title.SetGetterFunction([this] { return title_; });
+      icon.SetGetterFunction([this] { return icon_; });
     }
 
   std::string desktop_file_;
@@ -108,9 +111,6 @@
   unity::WindowList windows_;
   std::string type_;
 
-
-  virtual std::string icon() const { return icon_; }
-  virtual std::string title() const { return title_; }
   virtual std::string desktop_file() const { return desktop_file_; }
   virtual std::string type() const { return type_; }
   virtual std::string repr() const { return "MockApplication"; }
@@ -133,7 +133,6 @@
     running.changed.emit(state);
     }
 
-  bool GetSeen() const { return seen_; }
   bool SetSeen(bool const& param) {
     if (param != seen_) {
       seen_ = param;
@@ -142,7 +141,6 @@
     return false;
   }
 
-  bool GetSticky() const { return sticky_; }
   bool SetSticky(bool const& param) {
     if (param != sticky_) {
       sticky_ = param;
@@ -158,11 +156,6 @@
     active_ = state;
     active.changed.emit(state);
   }
-
-  bool GetVisible() const { return visible_; }
-  bool GetActive() const { return active_; }
-  bool GetRunning() const { return running_; }
-  bool GetUrgent() const { return urgent_; }
 };
 
 class MockApplicationManager : public unity::ApplicationManager
 
=== modified file 'tests/test_application_launcher_icon.cpp'
--- tests/test_application_launcher_icon.cpp	2013-05-17 22:02:43 +0000
+++ tests/test_application_launcher_icon.cpp	2013-06-27 10:54:34 +0000
@@ -195,35 +195,33 @@
   EXPECT_TRUE(mock_icon->RemoteUri().empty());
 }
 
-TEST_F(TestApplicationLauncherIcon, EmptyTooltipUpdatesOnRunning)
+TEST_F(TestApplicationLauncherIcon, TooltipUpdates)
 {
   ASSERT_TRUE(mock_icon->tooltip_text().empty());
   mock_app->title_ = "Got Name";
   ASSERT_TRUE(mock_icon->tooltip_text().empty());
 
-  mock_app->SetRunState(true);
+  mock_app->title.changed.emit(mock_app->title_);
   EXPECT_EQ(mock_icon->tooltip_text(), "Got Name");
 
-  mock_app->SetRunState(false);
   mock_app->title_ = "New Name";
-  mock_app->SetRunState(true);
-  EXPECT_EQ(mock_icon->tooltip_text(), "Got Name");
+  mock_app->title.changed.emit(mock_app->title_);
+  EXPECT_EQ(mock_icon->tooltip_text(), "New Name");
 }
 
-TEST_F(TestApplicationLauncherIcon, InvalidIconUpdatesOnRunning)
+TEST_F(TestApplicationLauncherIcon, IconUpdates)
 {
   ASSERT_EQ(mock_icon->icon_name(), DEFAULT_EMPTY_ICON);
   mock_app->icon_ = "icon-name";
 
   ASSERT_EQ(mock_icon->icon_name(), DEFAULT_EMPTY_ICON);
 
-  mock_app->SetRunState(true);
+  mock_app->icon.changed.emit(mock_app->icon_);
   EXPECT_EQ(mock_icon->icon_name(), "icon-name");
 
-  mock_app->SetRunState(false);
   mock_app->icon_ = "new-icon-name";
-  mock_app->SetRunState(true);
-  EXPECT_EQ(mock_icon->icon_name(), "icon-name");
+  mock_app->icon.changed.emit(mock_app->icon_);
+  EXPECT_EQ(mock_icon->icon_name(), "new-icon-name");
 }
 
 TEST_F(TestApplicationLauncherIcon, PerformScrollTowardsTheUser)
 
=== modified file 'tests/test_switcher_controller.h'
--- tests/test_switcher_controller.h	2013-05-17 22:53:57 +0000
+++ tests/test_switcher_controller.h	2013-06-27 10:54:34 +0000
@@ -54,8 +54,6 @@
 public:
   FakeApplicationWindow(Window xid, uint64_t active_number = 0);
 
-  std::string title() const;
-  virtual std::string icon() const;
   virtual std::string type() const;
 
   virtual Window window_id() const;
 
=== modified file 'tests/test_switcher_controller_class.cpp'
--- tests/test_switcher_controller_class.cpp	2013-05-17 22:53:57 +0000
+++ tests/test_switcher_controller_class.cpp	2013-06-27 10:54:34 +0000
@@ -32,10 +32,11 @@
   auto standalone_window = std::make_shared<StandaloneWindow>(xid_);
   standalone_window->active_number = active_number;
   WM->AddStandaloneWindow(standalone_window);
+
+  title.SetGetterFunction([this] { return "FakeApplicationWindow"; });
+  icon.SetGetterFunction([this] { return ""; });
 }
 
-std::string FakeApplicationWindow::title() const { return "FakeApplicationWindow"; }
-std::string FakeApplicationWindow::icon() const { return ""; }
 std::string FakeApplicationWindow::type() const { return "mock"; }
 
 Window FakeApplicationWindow::window_id() const { return xid_; }
 
=== modified file 'unity-shared/ApplicationManager.h'
--- unity-shared/ApplicationManager.h	2012-12-19 22:34:22 +0000
+++ unity-shared/ApplicationManager.h	2013-06-27 10:54:34 +0000
@@ -46,8 +46,6 @@
 public:
   virtual ~ApplicationWindow() {}
 
-  virtual std::string title() const = 0;
-  virtual std::string icon() const = 0;
   virtual std::string type() const = 0; // 'window' or 'tab'
 
   virtual Window window_id() const = 0;
@@ -62,6 +60,9 @@
   // Closes the window, or the browser tab if a webapp.
   virtual void Quit() const = 0;
 
+  nux::ROProperty<std::string> title;
+  nux::ROProperty<std::string> icon;
+
   nux::ROProperty<bool> visible;
   nux::ROProperty<bool> active;
   nux::ROProperty<bool> urgent;
@@ -73,8 +74,6 @@
 public:
   virtual ~Application() {}
 
-  virtual std::string icon() const = 0;
-  virtual std::string title() const = 0;
   virtual std::string desktop_file() const = 0;
   virtual std::string type() const = 0;
 
@@ -91,6 +90,9 @@
   // Calls quit on all the Windows for this application.
   virtual void Quit() const = 0;
 
+  nux::ROProperty<std::string> title;
+  nux::ROProperty<std::string> icon;
+
   // Considering using a property for the "unity-seen" quark
   nux::RWProperty<bool> seen;
   nux::RWProperty<bool> sticky;
 
=== modified file 'unity-shared/BamfApplicationManager.cpp'
--- unity-shared/BamfApplicationManager.cpp	2013-06-01 02:10:49 +0000
+++ unity-shared/BamfApplicationManager.cpp	2013-06-27 10:54:34 +0000
@@ -44,12 +44,12 @@
 {
 }
 
-std::string View::title() const
+std::string View::GetTitle() const
 {
   return glib::String(bamf_view_get_name(bamf_view_)).Str();
 }
 
-std::string View::icon() const
+std::string View::GetIcon() const
 {
   return glib::String(bamf_view_get_icon(bamf_view_)).Str();
 }
@@ -81,15 +81,32 @@
 }
 
 
+std::string WindowBase::type() const
+{
+  return View::type();
+}
+
 WindowBase::WindowBase(ApplicationManager const& manager,
                        glib::Object<BamfView> const& window)
   : View(manager, window)
 {
+  title.SetGetterFunction(sigc::mem_fun(this, &View::GetTitle));
+  icon.SetGetterFunction(sigc::mem_fun(this, &View::GetIcon));
   visible.SetGetterFunction(sigc::mem_fun(this, &View::GetVisible));
   active.SetGetterFunction(sigc::mem_fun(this, &View::GetActive));
   urgent.SetGetterFunction(sigc::mem_fun(this, &View::GetUrgent));
 
   glib::SignalBase* sig;
+  sig = new glib::Signal<void, BamfView*, const char*, const char*>(bamf_view_, "name-changed",
+                          [this] (BamfView*, const char*, const char* new_name) {
+                            this->title.changed.emit(glib::gchar_to_string(new_name));
+                          });
+  signals_.Add(sig);
+  sig = new glib::Signal<void, BamfView*, const char*>(bamf_view_, "icon-changed",
+                          [this] (BamfView*, const char* icon) {
+                            this->icon.changed.emit(glib::gchar_to_string(icon));
+                          });
+  signals_.Add(sig);
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view_, "user-visible-changed",
                           [this] (BamfView*, gboolean visible) {
                             this->visible.changed.emit(visible);
@@ -107,21 +124,6 @@
   signals_.Add(sig);
 }
 
-std::string WindowBase::title() const
-{
-  return View::title();
-}
-
-std::string WindowBase::icon() const
-{
-  return View::icon();
-}
-
-std::string WindowBase::type() const
-{
-  return View::type();
-}
-
 bool WindowBase::Focus() const
 {
   Window xid = window_id();
@@ -238,6 +240,8 @@
 void Application::HookUpEvents()
 {
   // Hook up the property set/get functions
+  title.SetGetterFunction(sigc::mem_fun(this, &View::GetTitle));
+  icon.SetGetterFunction(sigc::mem_fun(this, &View::GetIcon));
   seen.SetGetterFunction(sigc::mem_fun(this, &Application::GetSeen));
   seen.SetSetterFunction(sigc::mem_fun(this, &Application::SetSeen));
   sticky.SetGetterFunction(sigc::mem_fun(this, &Application::GetSticky));
@@ -247,7 +251,18 @@
   running.SetGetterFunction(sigc::mem_fun(this, &View::GetRunning));
   urgent.SetGetterFunction(sigc::mem_fun(this, &View::GetUrgent));
 
+  // Use signals_.Add....
   glib::SignalBase* sig;
+  sig = new glib::Signal<void, BamfView*, const char*, const char*>(bamf_view_, "name-changed",
+                          [this] (BamfView*, const char*, const char* new_name) {
+                            this->title.changed.emit(glib::gchar_to_string(new_name));
+                          });
+  signals_.Add(sig);
+  sig = new glib::Signal<void, BamfView*, const char*>(bamf_view_, "icon-changed",
+                          [this] (BamfView*, const char* icon) {
+                            this->icon.changed.emit(glib::gchar_to_string(icon));
+                          });
+  signals_.Add(sig);
   sig = new glib::Signal<void, BamfView*, gboolean>(bamf_view_, "user-visible-changed",
                           [this] (BamfView*, gboolean visible) {
                             LOG_TRACE(logger) << "user-visible-changed " << visible;
@@ -305,16 +320,6 @@
   signals_.Add(sig);
 }
 
-std::string Application::title() const
-{
-  return View::title();
-}
-
-std::string Application::icon() const
-{
-  return View::icon();
-}
-
 std::string Application::desktop_file() const
 {
   const gchar* file = bamf_application_get_desktop_file(bamf_app_);
 
=== modified file 'unity-shared/BamfApplicationManager.h'
--- unity-shared/BamfApplicationManager.h	2012-12-19 22:34:22 +0000
+++ unity-shared/BamfApplicationManager.h	2013-06-27 10:54:34 +0000
@@ -37,8 +37,8 @@
   View(ApplicationManager const& manager,
        glib::Object<BamfView> const& view);
 
-  std::string title() const;
-  std::string icon() const;
+  std::string GetTitle() const;
+  std::string GetIcon() const;
   std::string type() const;
 
   bool GetVisible() const;
@@ -59,8 +59,6 @@
              glib::Object<BamfView> const& window);
 
 public:
-  virtual std::string title() const;
-  virtual std::string icon() const;
   virtual std::string type() const; // 'window' or 'tab'
 
   virtual bool Focus() const;
@@ -110,8 +108,6 @@
   Application(ApplicationManager const& manager,
               glib::Object<BamfApplication> const& app);
 
-  virtual std::string title() const;
-  virtual std::string icon() const;
   virtual std::string desktop_file() const;
   virtual std::string type() const;