Merge lp:~3v1n0/unity/keep-priority-launcher-model into lp:unity

[Marco Trevisan (Treviño)]

LauncherModel: rewrite the Reordering functions to keep the icon priority deltas

The priority of the icons added to the model was reset every time the model
was reordered; we should otherwise keep the priority deltas between icons
so that we can safely set them without be worried that a reorder would completely
reset them.
Also add IconIndex method to get the positio of a given icon.

The unity LauncherModel used the IconPriority value to sort icons, however it was resetting this value every time the model was reordered, and was not possible to keep the priority deltas between the icons.
I've rewritten the model functions to make them change the priority to a relative value, instead of absolute ones.

This work is preliminary to the work needed for bug #761155

Unit tests added and updated.

[Andrea Azzarone (azzar1)]

LGTM... Discussed and reviewed on IRC.

review: Approve

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1181/console reported an error when processing this lp:~3v1n0/unity/keep-priority-launcher-model branch.
Not merging it.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1203/console reported an error when processing this lp:~3v1n0/unity/keep-priority-launcher-model branch.
Not merging it.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1209/console reported an error when processing this lp:~3v1n0/unity/keep-priority-launcher-model branch.
Not merging it.

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-unity/1212/console reported an error when processing this lp:~3v1n0/unity/keep-priority-launcher-model branch.
Not merging it.

=== modified file 'launcher/Launcher.cpp'
--- launcher/Launcher.cpp	2012-09-03 08:47:49 +0000
+++ launcher/Launcher.cpp	2012-09-04 16:43:33 +0000
@@ -1981,7 +1981,8 @@
 
   // FIXME: nux doesn't give nux::GetEventButton (button_flags) there, relying
   // on an internal Launcher property then
-  if (drag_icon && _last_button_press == 1 && _model->IconHasSister(drag_icon))
+  bool can_drag = (_model->IconHasSister(drag_icon) || drag_icon->GetIconType() == AbstractLauncherIcon::IconType::DEVICE);
+  if (drag_icon && _last_button_press == 1 && can_drag)
   {
     SetActionState(ACTION_DRAG_ICON);
     StartIconDrag(drag_icon);
 
=== modified file 'launcher/LauncherController.cpp'
--- launcher/LauncherController.cpp	2012-08-30 21:09:02 +0000
+++ launcher/LauncherController.cpp	2012-09-04 16:43:33 +0000
@@ -688,10 +688,6 @@
   GList* apps, *l;
   BamfApplication* app;
 
-  // Sufficiently large number such that we ensure proper sorting
-  // (avoids case where first item gets tacked onto end rather than start)
-  int priority = 100;
-
   FavoriteList const& favs = FavoriteStore::Instance().GetFavorites();
 
   for (FavoriteList::const_iterator i = favs.begin(), end = favs.end();
@@ -701,9 +697,8 @@
 
     if (fav)
     {
-      fav->SetSortPriority(priority);
+      fav->SetSortPriority(sort_priority_++);
       RegisterIcon(fav);
-      priority++;
     }
   }
 
 
=== modified file 'launcher/LauncherModel.cpp'
--- launcher/LauncherModel.cpp	2012-08-31 19:35:46 +0000
+++ launcher/LauncherModel.cpp	2012-09-04 16:43:33 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
- * Copyright (C) 2010 Canonical Ltd
+ * Copyright (C) 2010-2012 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -15,6 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Jason Smith <[e-mail]>
+ *              Marco Trevisan <[e-mail]>
  */
 
 #include "LauncherModel.h"
@@ -30,8 +31,7 @@
 
 LauncherModel::LauncherModel()
   : selection_(0)
-{
-}
+{}
 
 std::string LauncherModel::GetName() const
 {
@@ -54,12 +54,12 @@
   return introspection_results_;
 }
 
-bool LauncherModel::IconShouldShelf(AbstractLauncherIcon::Ptr icon) const
+bool LauncherModel::IconShouldShelf(AbstractLauncherIcon::Ptr const& icon) const
 {
   return icon->GetIconType() == AbstractLauncherIcon::IconType::TRASH;
 }
 
-bool LauncherModel::CompareIcons(AbstractLauncherIcon::Ptr first, AbstractLauncherIcon::Ptr second)
+bool LauncherModel::CompareIcons(AbstractLauncherIcon::Ptr const& first, AbstractLauncherIcon::Ptr const& second)
 {
   if (first->GetIconType() < second->GetIconType())
     return true;
@@ -69,36 +69,43 @@
   return first->SortPriority() < second->SortPriority();
 }
 
-bool
-LauncherModel::Populate()
+void LauncherModel::PopulatePart(iterator begin, iterator end)
+{
+  AbstractLauncherIcon::Ptr prev_icon;
+  for (auto it = begin; it != end; ++it)
+  {
+    auto const& icon = *it;
+    _inner.push_back(icon);
+
+    if (prev_icon)
+    {
+      // Ensuring that the current icon has higher priority than previous one
+      if (icon->SortPriority() < prev_icon->SortPriority())
+      {
+        int new_priority = prev_icon->SortPriority() + 1;
+        icon->SetSortPriority(new_priority);
+      }
+    }
+
+    prev_icon = icon;
+  }
+}
+
+bool LauncherModel::Populate()
 {
   Base copy = _inner;
-
   _inner.clear();
-
-  iterator it, it2;
-
-  int i = 0;
-  for (it = main_begin(); it != main_end(); ++it)
-  {
-    _inner.push_back(*it);
-    (*it)->SetSortPriority(i);
-    ++i;
-  }
-
-  for (it = shelf_begin(); it != shelf_end(); ++it)
-  {
-    _inner.push_back(*it);
-    (*it)->SetSortPriority(i);
-    ++i;
-  }
+  PopulatePart(main_begin(), main_end());
+  PopulatePart(shelf_begin(), shelf_end());
 
   return copy.size() == _inner.size() && !std::equal(begin(), end(), copy.begin());
 }
 
-void
-LauncherModel::AddIcon(AbstractLauncherIcon::Ptr icon)
+void LauncherModel::AddIcon(AbstractLauncherIcon::Ptr const& icon)
 {
+  if (!icon || std::find(begin(), end(), icon) != end())
+    return;
+
   if (IconShouldShelf(icon))
     _inner_shelf.push_back(icon);
   else
@@ -113,8 +120,7 @@
   icon->on_icon_removed_connection = icon->remove.connect(sigc::mem_fun(this, &LauncherModel::OnIconRemove));
 }
 
-void
-LauncherModel::RemoveIcon(AbstractLauncherIcon::Ptr icon)
+void LauncherModel::RemoveIcon(AbstractLauncherIcon::Ptr const& icon)
 {
   size_t size;
 
@@ -130,23 +136,20 @@
   }
 }
 
-void
-LauncherModel::OnIconRemove(AbstractLauncherIcon::Ptr icon)
+void LauncherModel::OnIconRemove(AbstractLauncherIcon::Ptr const& icon)
 {
-  timeouts_.AddTimeout(1000, [&, icon] {
+  timeouts_.AddTimeout(1000, [this, icon] {
     RemoveIcon(icon);
     return false;
   });
 }
 
-void
-LauncherModel::Save()
+void LauncherModel::Save()
 {
   saved.emit();
 }
 
-void
-LauncherModel::Sort()
+void LauncherModel::Sort()
 {
   std::stable_sort(_inner_shelf.begin(), _inner_shelf.end(), &LauncherModel::CompareIcons);
   std::stable_sort(_inner_main.begin(), _inner_main.end(), &LauncherModel::CompareIcons);
@@ -155,15 +158,14 @@
     order_changed.emit();
 }
 
-bool
-LauncherModel::IconHasSister(AbstractLauncherIcon::Ptr icon) const
+bool LauncherModel::IconHasSister(AbstractLauncherIcon::Ptr const& icon) const
 {
+  if (!icon)
+    return false;
+
   const_iterator it;
   const_iterator end;
 
-  if (icon && icon->GetIconType() == AbstractLauncherIcon::IconType::DEVICE)
-    return true;
-
   if (IconShouldShelf(icon))
   {
     it = _inner_shelf.begin();
@@ -177,144 +179,125 @@
 
   for (; it != end; ++it)
   {
-    AbstractLauncherIcon::Ptr iter_icon = *it;
-    if ((iter_icon  != icon)
-        && iter_icon->GetIconType() == icon->GetIconType())
+    AbstractLauncherIcon::Ptr const& iter_icon = *it;
+
+    if (iter_icon != icon && iter_icon->GetIconType() == icon->GetIconType())
       return true;
   }
 
   return false;
 }
 
-void
-LauncherModel::ReorderAfter(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other)
-{
-  if (icon == other || icon.IsNull() || other.IsNull())
-    return;
-
-  if (icon->GetIconType() != other->GetIconType())
-    return;
-
-  int i = 0;
-  for (LauncherModel::iterator it = begin(); it != end(); ++it)
-  {
-    if ((*it) == icon)
-      continue;
-
-    if ((*it) == other)
-    {
-      (*it)->SetSortPriority(i);
-      ++i;
-
-      icon->SetSortPriority(i);
-      ++i;
-    }
-    else
-    {
-      (*it)->SetSortPriority(i);
-      ++i;
-    }
-  }
-
-  Sort();
-}
-
-void
-LauncherModel::ReorderBefore(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other, bool save)
-{
-  if (icon == other || icon.IsNull() || other.IsNull())
-    return;
-
-  if (icon->GetIconType() != other->GetIconType())
-    return;
-
-  int i = 0;
-  int j = 0;
-  for (auto icon_it : _inner)
-  {
-    if (icon_it == icon)
-    {
-      j++;
-      continue;
-    }
-
-    if (icon_it == other)
-    {
-      icon->SetSortPriority(i);
-      if (i != j && save)
-        icon_it->SaveCenter();
-      i++;
-
-      icon_it->SetSortPriority(i);
-      if (i != j && save)
-        icon_it->SaveCenter();
-      i++;
-    }
-    else
-    {
-      icon_it->SetSortPriority(i);
-      if (i != j && save)
-        icon_it->SaveCenter();
-      i++;
-    }
-    j++;
-  }
-
-  Sort();
-}
-
-void
-LauncherModel::ReorderSmart(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other, bool save)
-{
-  if (icon == other || icon.IsNull() || other.IsNull())
-    return;
-
-  if (icon->GetIconType() != other->GetIconType())
-    return;
-
-  int i = 0;
-  int j = 0;
-  bool skipped = false;
-  for (auto icon_it : _inner)
-  {
-    if (icon_it == icon)
-    {
-      skipped = true;
-      j++;
-      continue;
-    }
-
-    if (icon_it == other)
-    {
-      if (!skipped)
-      {
-        icon->SetSortPriority(i);
-        if (i != j && save)
-          icon_it->SaveCenter();
-        i++;
-      }
-
-      icon_it->SetSortPriority(i);
-      if (i != j && save)
-        icon_it->SaveCenter();
-      i++;
-
-      if (skipped)
-      {
-        icon->SetSortPriority(i);
-        if (i != j && save)
-          icon_it->SaveCenter();
-        i++;
-      }
-    }
-    else
-    {
-      icon_it->SetSortPriority(i);
-      if (i != j && save)
-        icon_it->SaveCenter();
-      i++;
-    }
-    j++;
+void LauncherModel::ReorderAfter(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other)
+{
+  if (icon == other || icon.IsNull() || other.IsNull())
+    return;
+
+  if (icon->GetIconType() != other->GetIconType())
+    return;
+
+  icon->SetSortPriority(other->SortPriority() + 1);
+
+  for (auto it = std::next(std::find(begin(), end(), other)); it != end(); ++it)
+  {
+    // Increasing the priority of the icons next to the other one
+    auto const& icon_it = *it;
+    int new_priority = icon_it->SortPriority() + 1;
+    icon_it->SetSortPriority(new_priority);
+  }
+
+  Sort();
+}
+
+void LauncherModel::ReorderBefore(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other, bool animate)
+{
+  if (icon == other || icon.IsNull() || other.IsNull())
+    return;
+
+  if (icon->GetIconType() != other->GetIconType())
+    return;
+
+  bool found_target = false;
+  bool center = false;
+
+  for (auto const& icon_it : _inner)
+  {
+    if (icon_it == icon)
+    {
+      center = !center;
+      continue;
+    }
+
+    int new_priority = icon_it->SortPriority() + (found_target ? 1 : -1);
+    icon_it->SetSortPriority(new_priority);
+
+    if (icon_it == other)
+    {
+      if (animate && center)
+        icon_it->SaveCenter();
+
+      center = !center;
+      new_priority = new_priority - 1;
+      icon->SetSortPriority(new_priority);
+
+      if (animate && center)
+        icon_it->SaveCenter();
+
+      found_target = true;
+    }
+    else
+    {
+      if (animate && center)
+        icon_it->SaveCenter();
+    }
+  }
+
+  Sort();
+}
+
+void LauncherModel::ReorderSmart(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other, bool animate)
+{
+  if (icon == other || icon.IsNull() || other.IsNull())
+    return;
+
+  if (icon->GetIconType() != other->GetIconType())
+    return;
+
+  bool found_icon = false;
+  bool found_target = false;
+  bool center = false;
+
+  for (auto const& icon_it : _inner)
+  {
+    if (icon_it == icon)
+    {
+      found_icon = true;
+      center = !center;
+      continue;
+    }
+
+    int new_priority = icon_it->SortPriority() + (found_target ? 1 : -1);
+    icon_it->SetSortPriority(new_priority);
+
+    if (icon_it == other)
+    {
+      if (animate && center)
+        icon_it->SaveCenter();
+
+      center = !center;
+      new_priority = new_priority + (found_icon ? 1 : -1);
+      icon->SetSortPriority(new_priority);
+
+      if (animate && center)
+        icon_it->SaveCenter();
+
+      found_target = true;
+    }
+    else
+    {
+      if (animate && center)
+        icon_it->SaveCenter();
+    }
   }
 
   Sort();
@@ -357,7 +340,7 @@
     if (temp >= Size())
       temp = 0;
 
-    if (_inner[temp]->GetQuirk(AbstractLauncherIcon::Quirk::VISIBLE))
+    if (_inner[temp]->IsVisible())
     {
       selection_ = temp;
       selection_changed.emit(Selection());
@@ -377,7 +360,7 @@
     if (temp < 0)
       temp = Size() - 1;
 
-    if (_inner[temp]->GetQuirk(AbstractLauncherIcon::Quirk::VISIBLE))
+    if (_inner[temp]->IsVisible())
     {
       selection_ = temp;
       selection_changed.emit(Selection());
@@ -387,7 +370,7 @@
   }
 }
 
-AbstractLauncherIcon::Ptr LauncherModel::GetClosestIcon(AbstractLauncherIcon::Ptr icon, bool& is_before) const
+AbstractLauncherIcon::Ptr LauncherModel::GetClosestIcon(AbstractLauncherIcon::Ptr const& icon, bool& is_before) const
 {
   AbstractLauncherIcon::Ptr prev, next;
   bool found_target = false;
@@ -423,22 +406,38 @@
   return is_before ? prev : next;
 }
 
+int LauncherModel::IconIndex(AbstractLauncherIcon::Ptr const& target) const
+{
+  int pos = 0;
+  bool found = false;
+
+  for (auto const& icon : _inner)
+  {
+    if (icon == target)
+    {
+      found = true;
+      break;
+    }
+
+    ++pos;
+  }
+
+  return found ? pos : -1;
+}
+
 /* iterators */
 
-LauncherModel::iterator
-LauncherModel::begin()
+LauncherModel::iterator LauncherModel::begin()
 {
   return _inner.begin();
 }
 
-LauncherModel::iterator
-LauncherModel::end()
+LauncherModel::iterator LauncherModel::end()
 {
   return _inner.end();
 }
 
-LauncherModel::iterator
-LauncherModel::at(int index)
+LauncherModel::iterator LauncherModel::at(int index)
 {
   LauncherModel::iterator it;
   int i;
@@ -453,62 +452,52 @@
   return (LauncherModel::iterator)NULL;
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::rbegin()
+LauncherModel::reverse_iterator LauncherModel::rbegin()
 {
   return _inner.rbegin();
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::rend()
+LauncherModel::reverse_iterator LauncherModel::rend()
 {
   return _inner.rend();
 }
 
-LauncherModel::iterator
-LauncherModel::main_begin()
+LauncherModel::iterator LauncherModel::main_begin()
 {
   return _inner_main.begin();
 }
 
-LauncherModel::iterator
-LauncherModel::main_end()
+LauncherModel::iterator LauncherModel::main_end()
 {
   return _inner_main.end();
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::main_rbegin()
+LauncherModel::reverse_iterator LauncherModel::main_rbegin()
 {
   return _inner_main.rbegin();
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::main_rend()
+LauncherModel::reverse_iterator LauncherModel::main_rend()
 {
   return _inner_main.rend();
 }
 
-LauncherModel::iterator
-LauncherModel::shelf_begin()
+LauncherModel::iterator LauncherModel::shelf_begin()
 {
   return _inner_shelf.begin();
 }
 
-LauncherModel::iterator
-LauncherModel::shelf_end()
+LauncherModel::iterator LauncherModel::shelf_end()
 {
   return _inner_shelf.end();
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::shelf_rbegin()
+LauncherModel::reverse_iterator LauncherModel::shelf_rbegin()
 {
   return _inner_shelf.rbegin();
 }
 
-LauncherModel::reverse_iterator
-LauncherModel::shelf_rend()
+LauncherModel::reverse_iterator LauncherModel::shelf_rend()
 {
   return _inner_shelf.rend();
 }
 
=== modified file 'launcher/LauncherModel.h'
--- launcher/LauncherModel.h	2012-08-31 19:35:46 +0000
+++ launcher/LauncherModel.h	2012-09-04 16:43:33 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
- * Copyright (C) 2010 Canonical Ltd
+ * Copyright (C) 2010-2012 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -15,6 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Jason Smith <[e-mail]>
+ *              Marco Trevisan <[e-mail]>
  */
 
 #ifndef LAUNCHERMODEL_H
@@ -41,20 +42,18 @@
 
   LauncherModel();
 
-  void AddIcon(AbstractLauncherIcon::Ptr icon);
-  void RemoveIcon(AbstractLauncherIcon::Ptr icon);
+  void AddIcon(AbstractLauncherIcon::Ptr const& icon);
+  void RemoveIcon(AbstractLauncherIcon::Ptr const& icon);
   void Save();
   void Sort();
   int  Size() const;
 
-  void OnIconRemove(AbstractLauncherIcon::Ptr icon);
-
-  bool IconHasSister(AbstractLauncherIcon::Ptr icon) const;
-
-  void ReorderAfter(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other);
-  void ReorderBefore(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other, bool save);
-
-  void ReorderSmart(AbstractLauncherIcon::Ptr icon, AbstractLauncherIcon::Ptr other, bool save);
+  bool IconHasSister(AbstractLauncherIcon::Ptr const& icon) const;
+  int IconIndex(AbstractLauncherIcon::Ptr const& icon) const;
+
+  void ReorderAfter(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other);
+  void ReorderBefore(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other, bool animate);
+  void ReorderSmart(AbstractLauncherIcon::Ptr const& icon, AbstractLauncherIcon::Ptr const& other, bool animate);
 
   AbstractLauncherIcon::Ptr Selection() const;
   int SelectionIndex() const;
@@ -62,7 +61,7 @@
   void SelectNext();
   void SelectPrevious();
 
-  AbstractLauncherIcon::Ptr GetClosestIcon(AbstractLauncherIcon::Ptr icon, bool& is_before) const;
+  AbstractLauncherIcon::Ptr GetClosestIcon(AbstractLauncherIcon::Ptr const& icon, bool& is_before) const;
 
   iterator begin();
   iterator end();
@@ -80,17 +79,17 @@
   reverse_iterator shelf_rbegin();
   reverse_iterator shelf_rend();
 
-  sigc::signal<void, AbstractLauncherIcon::Ptr> icon_added;
-  sigc::signal<void, AbstractLauncherIcon::Ptr> icon_removed;
+  sigc::signal<void, AbstractLauncherIcon::Ptr const&> icon_added;
+  sigc::signal<void, AbstractLauncherIcon::Ptr const&> icon_removed;
+  sigc::signal<void, AbstractLauncherIcon::Ptr const&> selection_changed;
   sigc::signal<void> order_changed;
   sigc::signal<void> saved;
-  sigc::signal<void, AbstractLauncherIcon::Ptr> selection_changed;
 
-  IntrospectableList GetIntrospectableChildren();
 protected:
   // Introspectable methods
   std::string GetName() const;
   void AddProperties(GVariantBuilder* builder);
+  IntrospectableList GetIntrospectableChildren();
 
 private:
   Base             _inner;
@@ -101,10 +100,10 @@
   glib::SourceManager timeouts_;
 
   bool Populate();
-
-  bool IconShouldShelf(AbstractLauncherIcon::Ptr icon) const;
-
-  static bool CompareIcons(AbstractLauncherIcon::Ptr first, AbstractLauncherIcon::Ptr second);
+  void PopulatePart(iterator begin, iterator end);
+  void OnIconRemove(AbstractLauncherIcon::Ptr const& icon);
+  bool IconShouldShelf(AbstractLauncherIcon::Ptr const& icon) const;
+  static bool CompareIcons(AbstractLauncherIcon::Ptr const& first, AbstractLauncherIcon::Ptr const& second);
 
   /* Template Methods */
 public:
 
=== modified file 'launcher/MockLauncherIcon.h'
--- launcher/MockLauncherIcon.h	2012-08-31 18:00:29 +0000
+++ launcher/MockLauncherIcon.h	2012-09-04 16:43:33 +0000
@@ -44,12 +44,12 @@
 {
   NUX_DECLARE_OBJECT_TYPE(MockLauncherIcon, AbstractLauncherIcon);
 public:
-  MockLauncherIcon()
-    : icon_(0)
+  MockLauncherIcon(IconType type = IconType::APPLICATION)
+    : type_(type)
+    , sort_priority_(0)
+    , icon_(0)
   {
     tooltip_text = "Mock Icon";
-    sort_priority_ = 0;
-    type_ = IconType::APPLICATION;
 
     for (unsigned i = 0; i < unsigned(Quirk::LAST); ++i)
     {
@@ -58,7 +58,7 @@
   }
 
   std::string GetName() const { return "MockLauncherIcon"; }
-  
+
   void AddProperties(GVariantBuilder* builder) {}
 
   void HideTooltip() {}
@@ -293,9 +293,9 @@
   bool IsVisible() const { return false; }
 
   void AboutToRemove() {}
-  
+
   void Stick(bool save = true) {}
-  
+
   void UnStick() {}
 
 private:
@@ -346,9 +346,9 @@
     return result;
   }
 
+  IconType type_;
+  int sort_priority_;
   nux::BaseTexture* icon_;
-  int sort_priority_;
-  IconType type_;
   bool quirks_[unsigned(Quirk::LAST)];
   timespec quirk_times_[unsigned(Quirk::LAST)];
   std::map<int, nux::Point3> center_;
 
=== modified file 'tests/test_launcher.cpp'
--- tests/test_launcher.cpp	2012-09-01 14:09:20 +0000
+++ tests/test_launcher.cpp	2012-09-04 16:43:33 +0000
@@ -210,10 +210,6 @@
   MockMockLauncherIcon::Ptr icon2(new MockMockLauncherIcon);
   MockMockLauncherIcon::Ptr icon3(new MockMockLauncherIcon);
 
-  icon1->SetSortPriority(0);
-  icon2->SetSortPriority(1);
-  icon3->SetSortPriority(2);
-
   model_->AddIcon(icon1);
   model_->AddIcon(icon2);
   model_->AddIcon(icon3);
 
=== modified file 'tests/test_launcher_model.cpp'
--- tests/test_launcher_model.cpp	2012-08-31 19:35:46 +0000
+++ tests/test_launcher_model.cpp	2012-09-04 16:43:33 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Canonical Ltd.
+ * Copyright 2011-2012 Canonical Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 3, as published
@@ -15,7 +15,7 @@
  * <http://www.gnu.org/licenses/>
  *
  * Authored by: Jason Smith <[e-mail]>
- *
+ *              Marco Trevisan <[e-mail]>
  */
 
 #include <gtest/gtest.h>
@@ -32,229 +32,308 @@
 namespace
 {
 
-class EventListener
+class TestLauncherModel : public testing::Test
 {
-  public:
-    EventListener()
-    {
-      icon_added = false;
-      icon_removed = false;
-    }
-
-    void OnIconAdded (AbstractLauncherIcon::Ptr icon)
-    {
-      icon_added = true;
-    }
-
-    void OnIconRemoved (AbstractLauncherIcon::Ptr icon)
-    {
-      icon_removed = true;
-    }
-
-    bool icon_added;
-    bool icon_removed; 
+public:
+  TestLauncherModel()
+    : icon1(new MockLauncherIcon())
+    , icon2(new MockLauncherIcon())
+    , icon3(new MockLauncherIcon())
+    , icon4(new MockLauncherIcon())
+  {}
+
+  AbstractLauncherIcon::Ptr icon1;
+  AbstractLauncherIcon::Ptr icon2;
+  AbstractLauncherIcon::Ptr icon3;
+  AbstractLauncherIcon::Ptr icon4;
+
+  LauncherModel model;
 };
-//bool seen_result;
-
-TEST(TestLauncherModel, TestConstructor)
-{
-  LauncherModel::Ptr model(new LauncherModel());
-  EXPECT_EQ(model->Size(), 0);
-}
-
-TEST(TestLauncherModel, TestAdd)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  LauncherModel::Ptr model(new LauncherModel());
-
-  EXPECT_EQ(model->Size(), 0);
-  model->AddIcon(first);
-  EXPECT_EQ(model->Size(), 1);
-}
-
-TEST(TestLauncherModel, TestRemove)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  LauncherModel::Ptr model(new LauncherModel());
-
-  EXPECT_EQ(model->Size(), 0);
-  model->AddIcon(first);
-  EXPECT_EQ(model->Size(), 1);
-  model->RemoveIcon(first);
-  EXPECT_EQ(model->Size(), 0);
-}
-
-TEST(TestLauncherModel, TestAddSignal)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  LauncherModel::Ptr model(new LauncherModel());
-
-  EventListener *listener = new EventListener();
-
-  model->icon_added.connect(sigc::mem_fun(listener, &EventListener::OnIconAdded));
-  model->AddIcon(first);
-  EXPECT_EQ(listener->icon_added, true);
-
-  delete listener;
-}
-
-TEST(TestLauncherModel, TestRemoveSignal)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  LauncherModel::Ptr model(new LauncherModel());
-
-  EventListener *listener = new EventListener();
-
-  model->icon_removed.connect(sigc::mem_fun(listener, &EventListener::OnIconRemoved));
-  model->AddIcon(first);
-  EXPECT_EQ(listener->icon_removed, false);
-  model->RemoveIcon(first);
-  EXPECT_EQ(listener->icon_removed, true);
-
-  delete listener;
-}
-
-TEST(TestLauncherModel, TestSort)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr second(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr third(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr fourth(new MockLauncherIcon());
-
-  LauncherModel::Ptr model(new LauncherModel());
-
-  third->SetSortPriority(0);
-  model->AddIcon(third);
-
-  first->SetSortPriority(-1);
-  model->AddIcon(first);
-
-  fourth->SetSortPriority(2);
-  model->AddIcon(fourth);
-
-  second->SetSortPriority(0);
-  model->AddIcon(second);
-
-  LauncherModel::iterator it;
-  it = model->begin();
-
-  EXPECT_EQ(first, *it);
-  it++;
-  EXPECT_EQ(second, *it);
-  it++;
-  EXPECT_EQ(third, *it);
-  it++;
-  EXPECT_EQ(fourth, *it);
-}
-
-TEST(TestLauncherModel, TestReorderBefore)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr second(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr third(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr fourth(new MockLauncherIcon());
-
-  LauncherModel::Ptr model(new LauncherModel());
-
-  first->SetSortPriority(0);
-  second->SetSortPriority(1);
-  third->SetSortPriority(2);
-  fourth->SetSortPriority(3);
-
-  model->AddIcon(first);
-  model->AddIcon(second);
-  model->AddIcon(third);
-  model->AddIcon(fourth);
-
-  model->ReorderBefore(third, second, false);
-
-  LauncherModel::iterator it;
-  it = model->begin();
-
-  EXPECT_EQ(first, *it);
-  it++;
-  EXPECT_EQ(third, *it);
-  it++;
-  EXPECT_EQ(second, *it);
-  it++;
-  EXPECT_EQ(fourth, *it);
-}
-
-TEST(TestLauncherModel, TestReorderSmart)
-{
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr second(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr third(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr fourth(new MockLauncherIcon());
-
-  LauncherModel::Ptr model(new LauncherModel());
-
-  first->SetSortPriority(0);
-  second->SetSortPriority(1);
-  third->SetSortPriority(2);
-  fourth->SetSortPriority(3);
-
-  model->AddIcon(first);
-  model->AddIcon(second);
-  model->AddIcon(third);
-  model->AddIcon(fourth);
-
-  model->ReorderSmart(third, second, false);
-
-  LauncherModel::iterator it;
-  it = model->begin();
-
-  EXPECT_EQ(first, *it);
-  it++;
-  EXPECT_EQ(third, *it);
-  it++;
-  EXPECT_EQ(second, *it);
-  it++;
-  EXPECT_EQ(fourth, *it);
-}
-
-TEST(TestLauncherModel, TestGetClosestIcon)
-{
-  LauncherModel::Ptr model(new LauncherModel());
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr second(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr third(new MockLauncherIcon());
-  AbstractLauncherIcon::Ptr fourth(new MockLauncherIcon());
-
-  first->SetSortPriority(0);
-  second->SetSortPriority(1);
-  third->SetSortPriority(2);
-  fourth->SetSortPriority(3);
-
-  model->AddIcon(first);
-  model->AddIcon(second);
-  model->AddIcon(third);
-  model->AddIcon(fourth);
+
+TEST_F(TestLauncherModel, Constructor)
+{
+  EXPECT_EQ(model.Size(), 0);
+}
+
+TEST_F(TestLauncherModel, Add)
+{
+  model.AddIcon(icon1);
+  EXPECT_EQ(model.Size(), 1);
+
+  model.AddIcon(icon1);
+  EXPECT_EQ(model.Size(), 1);
+
+  model.AddIcon(AbstractLauncherIcon::Ptr());
+  EXPECT_EQ(model.Size(), 1);
+}
+
+TEST_F(TestLauncherModel, Remove)
+{
+  model.AddIcon(icon1);
+  EXPECT_EQ(model.Size(), 1);
+
+  model.RemoveIcon(icon1);
+  EXPECT_EQ(model.Size(), 0);
+}
+
+TEST_F(TestLauncherModel, AddSignal)
+{
+  bool icon_added = false;
+  model.icon_added.connect([&icon_added] (AbstractLauncherIcon::Ptr) { icon_added = true; });
+
+  model.AddIcon(icon1);
+  EXPECT_TRUE(icon_added);
+
+  icon_added = false;
+  model.AddIcon(icon1);
+  EXPECT_FALSE(icon_added);
+
+  icon_added = false;
+  model.AddIcon(AbstractLauncherIcon::Ptr());
+  EXPECT_FALSE(icon_added);
+}
+
+TEST_F(TestLauncherModel, RemoveSignal)
+{
+  bool icon_removed = false;
+  model.icon_removed.connect([&icon_removed] (AbstractLauncherIcon::Ptr) { icon_removed = true; });
+
+  model.AddIcon(icon1);
+  EXPECT_FALSE(icon_removed);
+  model.RemoveIcon(icon1);
+  EXPECT_TRUE(icon_removed);
+}
+
+TEST_F(TestLauncherModel, Sort)
+{
+  icon2->SetSortPriority(0);
+  model.AddIcon(icon2);
+
+  icon1->SetSortPriority(-1);
+  model.AddIcon(icon1);
+
+  icon4->SetSortPriority(2);
+  model.AddIcon(icon4);
+
+  icon3->SetSortPriority(0);
+  model.AddIcon(icon3);
+
+  LauncherModel::iterator it;
+  it = model.begin();
+
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+}
+
+TEST_F(TestLauncherModel, ModelKeepsPriorityDeltas)
+{
+  icon2->SetSortPriority(0);
+  icon1->SetSortPriority(-1);
+  icon4->SetSortPriority(2);
+  icon3->SetSortPriority(0);
+
+  model.AddIcon(icon2);
+  model.AddIcon(icon1);
+  model.AddIcon(icon4);
+  model.AddIcon(icon3);
+
+  auto it = model.begin();
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+
+  EXPECT_GT(icon2->SortPriority(), icon1->SortPriority());
+  EXPECT_EQ(icon2->SortPriority(), icon3->SortPriority());
+  EXPECT_LT(icon3->SortPriority(), icon4->SortPriority());
+}
+
+TEST_F(TestLauncherModel, ReorderBefore)
+{
+  icon1->SetSortPriority(0);
+  icon2->SetSortPriority(1);
+  icon3->SetSortPriority(2);
+  icon4->SetSortPriority(3);
+
+  model.AddIcon(icon1);
+  model.AddIcon(icon2);
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
+
+  model.ReorderBefore(icon3, icon2, false);
+
+  LauncherModel::iterator it;
+  it = model.begin();
+
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+}
+
+TEST_F(TestLauncherModel, ReorderAfter)
+{
+  model.AddIcon(icon1);
+  model.AddIcon(icon3);
+  model.AddIcon(icon2);
+  model.AddIcon(icon4);
+
+  model.ReorderAfter(icon3, icon2);
+
+  LauncherModel::iterator it;
+  it = model.begin();
+
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+}
+
+TEST_F(TestLauncherModel, ReorderSmart)
+{
+  icon1->SetSortPriority(0);
+  icon2->SetSortPriority(1);
+  icon3->SetSortPriority(2);
+  icon4->SetSortPriority(3);
+
+  model.AddIcon(icon1);
+  model.AddIcon(icon2);
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
+
+  model.ReorderSmart(icon3, icon2, false);
+
+  LauncherModel::iterator it;
+  it = model.begin();
+
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+}
+
+TEST_F(TestLauncherModel, OrderByType)
+{
+  AbstractLauncherIcon::Ptr icon1(new MockLauncherIcon(AbstractLauncherIcon::IconType::HOME));
+  AbstractLauncherIcon::Ptr icon2(new MockLauncherIcon(AbstractLauncherIcon::IconType::APPLICATION));
+  AbstractLauncherIcon::Ptr icon3(new MockLauncherIcon(AbstractLauncherIcon::IconType::EXPO));
+  AbstractLauncherIcon::Ptr icon4(new MockLauncherIcon(AbstractLauncherIcon::IconType::DEVICE));
+  AbstractLauncherIcon::Ptr icon5(new MockLauncherIcon(AbstractLauncherIcon::IconType::TRASH));
+
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
+  model.AddIcon(icon2);
+  model.AddIcon(icon1);
+  model.AddIcon(icon5);
+
+  auto it = model.begin();
+  EXPECT_EQ(icon1, *it);
+  it++;
+  EXPECT_EQ(icon2, *it);
+  it++;
+  EXPECT_EQ(icon3, *it);
+  it++;
+  EXPECT_EQ(icon4, *it);
+  it++;
+  EXPECT_EQ(icon5, *it);
+  it++;
+  EXPECT_EQ(it, model.end());
+
+  auto it_main = model.main_begin();
+  EXPECT_EQ(icon1, *it_main);
+  it_main++;
+  EXPECT_EQ(icon2, *it_main);
+  it_main++;
+  EXPECT_EQ(icon3, *it_main);
+  it_main++;
+  EXPECT_EQ(icon4, *it_main);
+  it_main++;
+  EXPECT_EQ(it_main, model.main_end());
+
+  auto it_shelf = model.shelf_begin();
+  EXPECT_EQ(icon5, *it_shelf);
+  it_shelf++;
+  EXPECT_EQ(it_shelf, model.shelf_end());
+}
+
+TEST_F(TestLauncherModel, GetClosestIcon)
+{
+  model.AddIcon(icon1);
+  model.AddIcon(icon2);
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
 
   bool before;
-  EXPECT_EQ(model->GetClosestIcon(first, before), second);
+  EXPECT_EQ(model.GetClosestIcon(icon1, before), icon2);
   EXPECT_FALSE(before);
 
-  EXPECT_EQ(model->GetClosestIcon(second, before), first);
-  EXPECT_TRUE(before);
-
-  EXPECT_EQ(model->GetClosestIcon(third, before), second);
-  EXPECT_TRUE(before);
-
-  EXPECT_EQ(model->GetClosestIcon(fourth, before), third);
+  EXPECT_EQ(model.GetClosestIcon(icon2, before), icon1);
+  EXPECT_TRUE(before);
+
+  EXPECT_EQ(model.GetClosestIcon(icon3, before), icon2);
+  EXPECT_TRUE(before);
+
+  EXPECT_EQ(model.GetClosestIcon(icon4, before), icon3);
   EXPECT_TRUE(before);
 }
 
-TEST(TestLauncherModel, TestGetClosestIconWithOneIcon)
+TEST_F(TestLauncherModel, GetClosestIconWithOneIcon)
 {
-  LauncherModel::Ptr model(new LauncherModel());
-  AbstractLauncherIcon::Ptr first(new MockLauncherIcon());
-
-  model->AddIcon(first);
+  model.AddIcon(icon1);
 
   bool before;
-  EXPECT_EQ(model->GetClosestIcon(first, before), nullptr);
+  EXPECT_EQ(model.GetClosestIcon(icon1, before), nullptr);
   EXPECT_TRUE(before);
 }
 
+TEST_F(TestLauncherModel, IconIndex)
+{
+  model.AddIcon(icon1);
+  model.AddIcon(icon2);
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
+
+  EXPECT_EQ(model.IconIndex(icon1), 0);
+  EXPECT_EQ(model.IconIndex(icon2), 1);
+  EXPECT_EQ(model.IconIndex(icon3), 2);
+  EXPECT_EQ(model.IconIndex(icon4), 3);
+
+  AbstractLauncherIcon::Ptr icon5(new MockLauncherIcon());
+  EXPECT_EQ(model.IconIndex(icon5), -1);
+}
+
+TEST_F(TestLauncherModel, IconHasSister)
+{
+  model.AddIcon(icon1);
+  EXPECT_FALSE(model.IconHasSister(icon1));
+
+  model.AddIcon(icon2);
+  model.AddIcon(icon3);
+  model.AddIcon(icon4);
+
+  EXPECT_TRUE(model.IconHasSister(icon1));
+  EXPECT_TRUE(model.IconHasSister(icon2));
+  EXPECT_TRUE(model.IconHasSister(icon3));
+  EXPECT_TRUE(model.IconHasSister(icon4));
+
+  EXPECT_FALSE(AbstractLauncherIcon::Ptr());
+}
+
 }