Merge lp:~abreu-alexandre/unity/dbus-stick-unstick-launcher-icon into lp:unity

[Alexandre Abreu]

Add method to Launcher dbus interface to stick/unstick a launcher icon.

Needed in order to fix:

https://bugs.launchpad.net/libunity-webapps/+bug/1061056

[Marco Trevisan (Treviño) (3v1n0)]

22 + std::string app_uri;
23 + if (desktop_path.empty())
24 + return app_uri;

You can just return "" in that case, without initializing app_uri.
Also at the end of the function returning a temporary it's fine anyway..

46 +Controller::Impl::OnLauncherUpdateIconStickyState(std::string const& icon_uri,
47 + bool sticky)

Please, keep everything in one-line.

61 + AbstractLauncherIcon::Ptr existing_icon_entry =
62 + GetIconByUri(target_uri);

One line, plus use const& (or auto const&).

72 + SortAndUpdate();

Please avoid to do this, if nothing has been actually changed.

86 + else
87 + favorite_store.RemoveFavorite(target_uri);

Add brackets here too please. However in that case, when can happen that a favorite has not a related application icon? Going in edge cases it's ok, anyway.

140 + ASSERT_FALSE(favorite_store.IsFavorite(app_icon->RemoteUri()));

This is quite redundant as it should be guarantee by other tests we have.

[Alexandre Abreu (abreu-alexandre)]

> 22 + std::string app_uri;
> 23 + if (desktop_path.empty())
> 24 + return app_uri;
>
> You can just return "" in that case, without initializing app_uri.

Done.

> Also at the end of the function returning a temporary it's fine anyway..

mmh, it's returned by value & copy constructed ...

> 46 +Controller::Impl::OnLauncherUpdateIconStickyState(std::string const&
> icon_uri,
> 47 + bool sticky)
>
> Please, keep everything in one-line.

Done.

>
>
> 61 + AbstractLauncherIcon::Ptr existing_icon_entry =
> 62 + GetIconByUri(target_uri);
>
> One line, plus use const& (or auto const&).

Done.

>
>
> 72 + SortAndUpdate();
>
> Please avoid to do this, if nothing has been actually changed.

Good catch, done.

>
>
> 86 + else
> 87 + favorite_store.RemoveFavorite(target_uri);
>
> Add brackets here too please. However in that case, when can happen that a
> favorite has not a related application icon? Going in edge cases it's ok,
> anyway.

Done.

>
>
> 140 + ASSERT_FALSE(favorite_store.IsFavorite(app_icon->RemoteUri()));
>
> This is quite redundant as it should be guarantee by other tests we have.

Yeah, but it is somewhat consistent w/ the test's logic & strict internals here imo,

[Alexandre Abreu (abreu-alexandre)]

ping :)

[Marco Trevisan (Treviño) (3v1n0)]

Ops, sorry I missed, this... About app_uri variable I meant to remove it at all, but it's not a big thing, so we can stay with it, considering that I can clean it when merging this with lp:~3v1n0/unity/gdbus-server-use, so approved! ;)

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'launcher/LauncherController.cpp'
--- launcher/LauncherController.cpp	2013-02-14 21:01:58 +0000
+++ launcher/LauncherController.cpp	2013-02-27 18:35:24 +0000
@@ -68,6 +68,11 @@
   "      <arg type='s' name='aptdaemon_task' direction='in'/>"
   "    </method>"
   ""
+  "    <method name='UpdateLauncherIconFavoriteState'>"
+  "      <arg type='s' name='icon_uri' direction='in'/>"
+  "      <arg type='b' name='is_sticky' direction='in'/>"
+  "    </method>"
+  ""
   "  </interface>"
   "</node>";
 }
@@ -89,6 +94,16 @@
   const std::string RUNNING_APPS_URI = FavoriteStore::URI_PREFIX_UNITY + "running-apps";
   const std::string DEVICES_URI = FavoriteStore::URI_PREFIX_UNITY + "devices";
 }
+std::string CreateAppUriNameFromDesktopPath(const std::string &desktop_path)
+{
+  std::string app_uri;
+  if (desktop_path.empty())
+    return "";
+
+  app_uri = FavoriteStore::URI_PREFIX_APP +
+    DesktopUtilities::GetDesktopID(desktop_path);
+  return app_uri;
+}
 }
 
 GDBusInterfaceVTable Controller::Impl::interface_vtable =
@@ -348,7 +363,7 @@
   if (icon_uri.find(FavoriteStore::URI_PREFIX_FILE) == 0)
   {
     auto const& desktop_path = icon_uri.substr(FavoriteStore::URI_PREFIX_FILE.length());
-    app_uri = FavoriteStore::URI_PREFIX_APP + DesktopUtilities::GetDesktopID(desktop_path);
+    app_uri = local::CreateAppUriNameFromDesktopPath(desktop_path);
   }
 
   auto const& icon = GetIconByUri(app_uri.empty() ? icon_uri : app_uri);
@@ -433,6 +448,58 @@
 }
 
 void
+Controller::Impl::OnLauncherUpdateIconStickyState(std::string const& icon_uri, bool sticky)
+{
+  if (icon_uri.empty())
+    return;
+
+  std::string target_uri = icon_uri;
+  if (icon_uri.find(FavoriteStore::URI_PREFIX_FILE) == 0)
+  {
+    auto const& desktop_path =
+      icon_uri.substr(FavoriteStore::URI_PREFIX_FILE.length());
+
+    // app uri instead
+    target_uri = local::CreateAppUriNameFromDesktopPath(desktop_path);
+  }
+  auto const& existing_icon_entry =
+    GetIconByUri(target_uri);
+
+  if (existing_icon_entry)
+    {
+      // use the backgroung mechanism of model updates & propagation
+      bool should_update = (existing_icon_entry->IsSticky() != sticky);
+      if (should_update)
+        {
+          if (sticky)
+            existing_icon_entry->Stick(true);
+          else
+            existing_icon_entry->UnStick();
+          
+          SortAndUpdate();
+        }
+    }
+    else
+    {
+      FavoriteStore& favorite_store = FavoriteStore::Instance();
+
+      bool should_update = (favorite_store.IsFavorite(target_uri) != sticky);
+      if (should_update)
+        {
+          if (sticky)
+            {
+              favorite_store.AddFavorite(target_uri, -1);
+              RegisterIcon(CreateFavoriteIcon(target_uri));
+            }
+          else
+            {
+              favorite_store.RemoveFavorite(target_uri);
+            }
+        }
+    }
+}
+
+void
 Controller::Impl::OnLauncherAddRequestSpecial(std::string const& path,
                                               std::string const& aptdaemon_trans_id,
                                               std::string const& icon_path,
@@ -1499,6 +1566,17 @@
 
     g_dbus_method_invocation_return_value(invocation, nullptr);
   }
+  else if (g_strcmp0(method_name, "UpdateLauncherIconFavoriteState") == 0)
+  {
+    auto self = static_cast<Controller::Impl*>(user_data);
+    gboolean is_sticky;
+    glib::String icon_uri;
+    g_variant_get(parameters, "(sb)", &icon_uri, &is_sticky);
+
+    self->OnLauncherUpdateIconStickyState(icon_uri.Str(), is_sticky);
+
+    g_dbus_method_invocation_return_value(invocation, nullptr);
+  }
 }
 
 } // namespace launcher
 
=== modified file 'launcher/LauncherControllerPrivate.h'
--- launcher/LauncherControllerPrivate.h	2013-02-09 15:16:41 +0000
+++ launcher/LauncherControllerPrivate.h	2013-02-27 18:35:24 +0000
@@ -71,6 +71,7 @@
   void OnLauncherAddRequest(std::string const& icon_uri, AbstractLauncherIcon::Ptr const& before);
   void OnLauncherAddRequestSpecial(std::string const& path, std::string const& aptdaemon_trans_id,
                                    std::string const& icon_path, int icon_x, int icon_y, int icon_size);
+  void OnLauncherUpdateIconStickyState(std::string const& desktop_file, bool sticky);
   void OnLauncherRemoveRequest(AbstractLauncherIcon::Ptr const& icon);
 
   void OnLauncherEntryRemoteAdded(LauncherEntryRemote::Ptr const& entry);
 
=== modified file 'tests/test_launcher_controller.cpp'
--- tests/test_launcher_controller.cpp	2013-02-14 19:30:15 +0000
+++ tests/test_launcher_controller.cpp	2013-02-27 18:35:24 +0000
@@ -1692,4 +1692,49 @@
   Utils::WaitUntil(check_fn, true);
 }
 
+TEST_F(TestLauncherController, SetExistingLauncherIconAsFavorite)
+{
+  const char * desktop_file = "normal-icon.desktop";
+  MockApplicationLauncherIcon::Ptr
+    app_icon(new NiceMock<MockApplicationLauncherIcon>(true, desktop_file));
+  lc.Impl()->RegisterIcon(app_icon);
+  ASSERT_FALSE(favorite_store.IsFavorite(app_icon->RemoteUri()));
+
+  const std::string icon_uri = FavoriteStore::URI_PREFIX_APP + desktop_file;
+  lc.Impl()->OnLauncherUpdateIconStickyState(icon_uri, true);
+
+  ASSERT_TRUE(app_icon->IsSticky());
+  EXPECT_TRUE(favorite_store.IsFavorite(app_icon->RemoteUri()));
+}
+
+TEST_F(TestLauncherController, SetExistingLauncherIconAsNonFavorite)
+{
+  const char * desktop_file = "normal-icon.desktop";
+  MockApplicationLauncherIcon::Ptr
+    app_icon(new NiceMock<MockApplicationLauncherIcon>(true, desktop_file));
+  lc.Impl()->RegisterIcon(app_icon);
+  ASSERT_FALSE(favorite_store.IsFavorite(app_icon->RemoteUri()));
+  app_icon->Stick(true);
+
+  EXPECT_CALL(*app_icon, UnStick());
+
+  const std::string icon_uri = FavoriteStore::URI_PREFIX_APP + desktop_file;
+  lc.Impl()->OnLauncherUpdateIconStickyState(icon_uri, false);
+}
+
+TEST_F(TestLauncherController, SetNonExistingLauncherIconAsFavorite)
+{
+  std::string desktop = app::BZR_HANDLE_PATCH;
+  std::string icon_uri = FavoriteStore::URI_PREFIX_APP + DesktopUtilities::GetDesktopID(desktop);
+
+  lc.Impl()->OnLauncherUpdateIconStickyState(icon_uri, true);
+
+  // Make sure that the icon now exists and is sticky
+  EXPECT_TRUE(favorite_store.IsFavorite(icon_uri));
+
+  auto const& icon = lc.Impl()->GetIconByUri(icon_uri);
+  ASSERT_TRUE(icon.IsValid());
+  ASSERT_TRUE(icon->IsSticky());
+}
+
 }