Merge lp:~azzar1/unity/launcher-devices-improvement into lp:unity

[Andrea Azzarone]

First of all let me show what changes I have done:
* I moved launcher devices options from GSettings to CompizConfig Settings Manager.
* I added a "Keep in launcher" item also for the devices. Since it have no sense having the quicklist item for "Never" and "Always" mode it is enabled only in "Only Mounted" one.
* I changed a bit the signals manager for GVolumeMonitor. Inter alia I moved all volume signals from DeviceLauncherIcon to DeviceLauncherSignal (according to me is less confusing!).
* Use std::map instad of g_hash_table.
* Use GLibWrapper to avoid memory leaks.
* Porting to new code style.

I have tested this work on three different computers. Thanks to Alessandro Cutilli, Nicolò Turatello and Andrea Virgillito for testing. I hope there is not error (code style or weird behavior).

Please pardon me in this case.

Sorry for my poor English.

[Tim Penhey (thumper)]

I notice that your editor is using tab stops. Can you configure it to use
spaces instead of tabs? See the diff for
plugins/unityshell/src/DeviceLauncherSection.cpp to see what I see.

If you are going to the trouble of adding an anonymous namespace, then you
should replace the #define:

plugins/unityshell/src/DeviceLauncherIcon.cpp

namespace {
const char* DEFAULT_ICON = "drive-removable-media";
}

  DeviceLauncherIcon::DeviceLauncherIcon(Launcher *launcher, GVolume *volume)
should be:
  DeviceLauncherIcon::DeviceLauncherIcon(Launcher* launcher, GVolume* volume)

Keep the pointer with the type not the variable name.

> void DeviceLauncherIcon::UpdateDeviceIcon()

The extra scoping you have in this method don't really help the readability,
nor are they required for early destruction of the objects. Can you remove
them please?

There is no point at all in adding the "inline" keyword to a function in the
source file. Inlining only works if the compiler knows at the time the other
source file is being compiled (at least I'm fairly sure about that).

> std::list<DbusmenuMenuitem *> DeviceLauncherIcon::GetMenus()

I'm not a fan of returning containers from methods. Normally there is a
better way to structure the program. You don't need to do anything about this
at this stage, just somethingn to consider for the future.

Although I just noticed that the list is containing things that you explicitly
create. This means that every time you call this method you need to remember
to explicitly free all of the objects. Hmm... I also see that you didn't
create this method. I'll chase this up elsewhere.

> void DeviceLauncherIcon::ShowMount(GMount *mount)

Please keep the pointer with the type.

> g_mount_unmount_with_operation(mount.RawPtr(),

This should work without the .RawPtr() call. The object implements an
operator OBJECT* method, and since the g_mount_unmount_with_operation expects
a GMount, the method is used.

Same for g_drive_stop.
Also try for the g_signal_connect. It should work. Let me know if it
doesn't.

std::map<GVolume *, DeviceLauncherIcon *> is screaming for a typedef. The
whole using a map based on pointers makes me queezy. I don't feel comfortable
that the contained values are destroyed consistently. The are newed when
added to the map, but removal just calls erase. I don't see a corresponding
delete. Instead of storing raw pointers, could we key it off a consistent
name perhaps? And store smart pointers?

review: Needs Fixing

[Andrea Azzarone (azzar1)]

> I notice that your editor is using tab stops. Can you configure it to use
> spaces instead of tabs? See the diff for
> plugins/unityshell/src/DeviceLauncherSection.cpp to see what I see.

Of course.

> If you are going to the trouble of adding an anonymous namespace, then you
> should replace the #define:
>
> plugins/unityshell/src/DeviceLauncherIcon.cpp
>
> namespace {
> const char* DEFAULT_ICON = "drive-removable-media";
> }

Of course.

> DeviceLauncherIcon::DeviceLauncherIcon(Launcher *launcher, GVolume *volume)
> should be:
> DeviceLauncherIcon::DeviceLauncherIcon(Launcher* launcher, GVolume* volume)
>
> Keep the pointer with the type not the variable name.

I don't like it, but it will be done ;)

> > void DeviceLauncherIcon::UpdateDeviceIcon()
>
> The extra scoping you have in this method don't really help the readability,
> nor are they required for early destruction of the objects. Can you remove
> them please?
>

Sure. They are not my method.

> There is no point at all in adding the "inline" keyword to a function in the
> source file. Inlining only works if the compiler knows at the time the other
> source file is being compiled (at least I'm fairly sure about that).
>

Ops...

> > std::list<DbusmenuMenuitem *> DeviceLauncherIcon::GetMenus()
>
> I'm not a fan of returning containers from methods. Normally there is a
> better way to structure the program. You don't need to do anything about this
> at this stage, just somethingn to consider for the future.
>
> Although I just noticed that the list is containing things that you explicitly
> create. This means that every time you call this method you need to remember
> to explicitly free all of the objects. Hmm... I also see that you didn't
> create this method. I'll chase this up elsewhere.
>

I can do nothing here. This is the quicklist manager if i am not wrong. Let me know
what can i do here.

> > void DeviceLauncherIcon::ShowMount(GMount *mount)
>
> Please keep the pointer with the type.
>
> > g_mount_unmount_with_operation(mount.RawPtr(),
>
> This should work without the .RawPtr() call. The object implements an
> operator OBJECT* method, and since the g_mount_unmount_with_operation expects
> a GMount, the method is used.
>

I use this because sometimes i got warnings.

> Same for g_drive_stop.
> Also try for the g_signal_connect. It should work. Let me know if it
> doesn't.
>

I will try...

> std::map<GVolume *, DeviceLauncherIcon *> is screaming for a typedef. The
> whole using a map based on pointers makes me queezy. I don't feel comfortable
> that the contained values are destroyed consistently. The are newed when
> added to the map, but removal just calls erase. I don't see a corresponding
> delete. Instead of storing raw pointers, could we key it off a consistent
> name perhaps? And store smart pointers?

Instead of <GVolume*> we could use a uuid but according to me is too slow... For the key
(GVolume *) we don't need a g_object_unref(...).
For what regard the value (DeviceLauncherIcon *) we cannot use a smart pointer (i used a boost one) because when we OnVolumeRemoved is called, we call "self->map_[volume]->OnRemoved();" that call Ico...

[Andrea Azzarone (azzar1)]

I haven't tested what happens removing RawPtr in other computers (just on mine).

[Tim Penhey (thumper)]

> > > void DeviceLauncherIcon::UpdateDeviceIcon()
> >
> > The extra scoping you have in this method don't really help the readability,
> > nor are they required for early destruction of the objects. Can you remove
> > them please?
> >
>
> Sure. They are not my method.

Aah. You are right. I can now see that you just replaced the gchar with the
glib::String. The old scopes were there to show the lifetime of the glib
created data, and they are no longer needed. Thanks for cleaning this up.

> > > std::list<DbusmenuMenuitem *> DeviceLauncherIcon::GetMenus()
> I can do nothing here. This is the quicklist manager if i am not wrong. Let me
> know what can i do here.

I did say that I can see this wasn't created by you :-) I will chase up the
design.

> > > void DeviceLauncherIcon::ShowMount(GMount *mount)
> >
> > Please keep the pointer with the type.
> >
> > > g_mount_unmount_with_operation(mount.RawPtr(),
> >
> > This should work without the .RawPtr() call. The object implements an
> > operator OBJECT* method, and since the g_mount_unmount_with_operation
> expects
> > a GMount, the method is used.
> >
>
> I use this because sometimes i got warnings.

There would be warnings in the places where it wasn't clear to the compiler
what you were wanting. But for most places it should work without the
.RawPtr() call.

> > std::map<GVolume *, DeviceLauncherIcon *> is screaming for a typedef. The
> > whole using a map based on pointers makes me queezy. I don't feel
> comfortable
> > that the contained values are destroyed consistently. The are newed when
> > added to the map, but removal just calls erase. I don't see a corresponding
> > delete. Instead of storing raw pointers, could we key it off a consistent
> > name perhaps? And store smart pointers?
>
> Instead of <GVolume*> we could use a uuid but according to me is too slow...
> For the key
> (GVolume *) we don't need a g_object_unref(...).
> For what regard the value (DeviceLauncherIcon *) we cannot use a smart pointer
> (i used a boost one) because when we OnVolumeRemoved is called, we call
> "self->map_[volume]->OnRemoved();" that call IconLauncher::Remove() that emit
> a signal that is handled by LancherController.cpp or LauncherModels.cpp (I
> don't remember! :) ) that unref the icon... If we use a smartpointer we
> "unref" it twice... Correct me if I'm wrong.

You see... here is my problem. None of this is obvious to me (the reader of
the code). I'll leave that logic part to Neil to verify.

= New stuff =

You have a checked_ member variable. "Checked" I think could be changed to
something a little more descriptive. "keep_in_launcher_" perhaps?

The constructor sets:
  checked_ = !(pos != favorites.end());
this is more simply written:
  checked_ = pos == favorites.end();

Which then makes me wonder, what is the purpose of "checked_".

This same double negative logic is found in
DeviceLauncherIcon::OnSettingsChanged

In summary, the only things I'd like to hear back about is the checked_
member, and the double negative logic.

Thanks for the updates.

review: Needs Fixing

[Andrea Azzarone (azzar1)]

* I changed 'checked_' in `keep_in_launcher_` and also its behavior to make it more readable.
* I ported it to 4.0 trunk.

[Tim Penhey (thumper)]

I'm happy with these changes. I'd like Neil to just confirm the lifecycle of the DeviceLauncherIcon you mentioned above. I've copied it here to make it easy for Neil :-)

> Instead of <GVolume*> we could use a uuid but according to me is too slow... For the key
> (GVolume *) we don't need a g_object_unref(...).
> For what regard the value (DeviceLauncherIcon *) we cannot use a smart pointer (i used a boost one)
> because when we OnVolumeRemoved is called, we call "self->map_[volume]->OnRemoved();" that call
> IconLauncher::Remove() that emit a signal that is handled by LancherController.cpp or LauncherModels.cpp
> (I don't remember! :) ) that unref the icon... If we use a smartpointer we "unref" it
> twice... Correct me if I'm wrong.

review: Approve

[Andrea Azzarone (azzar1)]

@Tim, if the destructor of the DeviceLauncherIcon is called, it means that it works right?

[Andrea Azzarone (azzar1)]

@Tim I think that in glib::String::Str() there is a little problem:

=================================================================
std::string String::Str() const
{
  return std::string(string_);
}
=================================================================

In fact if string_ is NULL an exception is thrown (at least this happens on
my computer). I noticed this because this branch crash if we have
a gvolume without uuid (e.g. a cd-rom). This fix works good but i don't know
if this is the better way to fix:

=================================================================
std::string String::Str() const
{
  if (string_)
    return std::string(string_);
  else
    return std::string("");
}
=================================================================

You are the expert, so let me know if this is a good fix :)

[Tim Penhey (thumper)]

On Sat, 25 Jun 2011 20:18:31 you wrote:
> @Tim I think that in glib::String::Str() there is a little problem:
>
> =================================================================
> std::string String::Str() const
> {
> return std::string(string_);
> }
> =================================================================
>
> In fact if string_ is NULL an exception is thrown (at least this happens on
> my computer). I noticed this because this branch crash if we have
> a gvolume without uuid (e.g. a cd-rom). This fix works good but i don't
> know if this is the better way to fix:
>
> =================================================================
> std::string String::Str() const
> {
> if (string_)
> return std::string(string_);
> else
> return std::string("");
> }
> =================================================================
>
> You are the expert, so let me know if this is a good fix :)

Yeah. I think that fix is fine.

[Neil J. Patel (njpatel)]

Hey, sorry for the late merge. This looks good and works well. However it has a merge-conflict with trunk. If you could update it I'll merge it as soon as it's ready.

Nice work!

review: Approve

[Omer Akram (om26er)]

Could you also change the string 'springboard' to 'launcher' when fixing the merge conflict. Not many people are aware of what springboard is

[Andrea Azzarone (azzar1)]

Done.

[Neil J. Patel (njpatel)]

Thanks Andrea, could you please ping me tomorrow as there's a few things I need to check with you before committing the code, thanks!

review: Approve

[Andrea Azzarone (azzar1)]

Of course, see you tomorrow...

[Neil J. Patel (njpatel)]

Hey Andy, so I cleaned out my system and tested again on a fresh Nux and Unity install on Oneiric. Adding/removing USB/external harddrives/cd-rom's etc works perfectly. And the setting in ccsm works perfectly too, nice one!

However I never see the "Keep in Launcher" option on any device (when I have "Only Mounted" showing) :/ Is there something else that needs to change for me to see that? I've updated my system-wide unity schema with the one from your branch so it shouldn't be that...

[Andrea Azzarone (azzar1)]

"Keep in launcher" option should be visible only for un-removable devices (when "Only Mounted" is checked). I will work on it.

[Andrea Azzarone (azzar1)]

Hey Neil, maybe you have a "automounted configuration" in /etc/fstab (or something like that). I know, it's a my mistake, but if it is the problem, last commit should resolve the problem. Let me know!

[Neil J. Patel (njpatel)]

Hey Andrea, fstab has no automount configuration for these drives (one is CD-ROM and other is 4G USB key). I tried your latest branch but still doesn't show the Keep In Launcher :( Are you on Oneiric? I wonder if something has changed...maybe tomorrow if you ping me we can debug this together, I can play around with the branch and get some more debug information for you.

[Tim Penhey (thumper)]

Neil,

Why would you want a "Keep in Launcher" option for a removable device?

[Andrea Azzarone (azzar1)]

Neil, i am on Oneiric...
I'll ping you as soon as possible, by the way you can printf the value of:
DevicesSettings::GetDefault().GetDevicesOption() == DevicesSettings::ONLY_MOUNTED

and of:
g_drive_is_media_removable (drive)

in DeviceLauncherIcons::GetMenus

[Neil J. Patel (njpatel)]

I don't, but it was part of the design I believe.

@andyrock, will try and do that tomorrow morning (am not at computer today)

Sent from my iPhone

On 7 Jul 2011, at 22:56, Tim Penhey <email address hidden> wrote:

> Neil,
>
> Why would you want a "Keep in Launcher" option for a removable device?
>
> --
> https://code.launchpad.net/~andyrock/unity/launcher-devices-improvement/+merge/65537
> You are reviewing the proposed merge of lp:~andyrock/unity/launcher-devices-improvement into lp:unity.

[Andrea Azzarone (azzar1)]

===================================================================
1) It should be possible to remove specific file system icons from the Launcher by un-checking the “Keep in Launcher” option (or dragging to the trash).
===================================================================

This branch use the "Keep in Launcher" option but not the dragging to the trash, since there is another bug report (#764905), and my merge proposal, with a different behavior: «Drag and drop a USB key into the trash should eject the USB key».

===================================================================
2) In the case of non-removable file systems, the system should remember these settings so that rebooting will not return these icons to the Launcher.
===================================================================
Ok, this branch does it (at least I hope :)

===================================================================
3) In the case of removable file systems (USB keys, SD cards, etc...), if a removable file system is removed and then re-attached it should re-appear in the Launcher. If the removable file system is not removed and the user has removed it's icon from the Launcher, the icon should not return to the Launcher even if the computer is re-booted.
===================================================================

According to me, this is absurd and make no sense and not so easy to do! :)

===================================================================
4) All 'specific file system' icons that the user has removed from the Launcher should be displayed in the Dash file Lens in the “Favourite Folders/Folders” category. This allows the user to re-add these icons to the Launcher at a later date
===================================================================
According to me this is in part a unity-file-place (???) bug... So first we should add the "specific file system" icons in the Dash file Lens and i cannot (i hate python!) do this.

=== modified file 'UnityCore/GLibWrapper.cpp'
--- UnityCore/GLibWrapper.cpp	2011-07-21 14:59:25 +0000
+++ UnityCore/GLibWrapper.cpp	2011-07-26 12:37:20 +0000
@@ -79,7 +79,10 @@
 
 std::string String::Str() const
 {
-  return std::string(string_);
+  if (string_)
+    return std::string(string_);
+  else
+    return std::string("");
 }
 
 

=== modified file 'com.canonical.Unity.gschema.xml'
--- com.canonical.Unity.gschema.xml	2011-07-21 09:54:39 +0000
+++ com.canonical.Unity.gschema.xml	2011-07-26 12:37:20 +0000
@@ -4,11 +4,6 @@
     <value nick="Desktop" value="1" />
     <value nick="Netbook" value="2" />
   </enum>
-  <enum id="devices-option-enum">
-    <value nick="Never" value="0" />
-    <value nick="OnlyMounted" value="1" />
-    <value nick="Always" value="2" />
-  </enum>
   <enum id="home-expanded-enum">
     <value nick="Not Expanded" value="0" />
     <value nick="Expanded" value="1" />
@@ -51,10 +46,10 @@
     </key>
   </schema>
   <schema path="/desktop/unity/devices/" id="com.canonical.Unity.Devices" gettext-domain="unity">
-    <key enum="devices-option-enum" name="devices-option">
-        <default>"OnlyMounted"</default>
-        <summary>The devices that will be shown on launcher</summary>
-        <description>Key for setting the devices that will be shown on launcher. </description>
+    <key type="as" name="favorites">
+        <default>[]</default>
+        <summary>List of device uuid for favorites on the launcher.</summary>
+        <description>These devices are shown in the Launcher by default.</description>
     </key>
     </schema>
   <schema path="/desktop/unity/runner/" id="com.canonical.Unity.Runner" gettext-domain="unity">

=== modified file 'plugins/unityshell/src/DeviceLauncherIcon.cpp'
--- plugins/unityshell/src/DeviceLauncherIcon.cpp	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DeviceLauncherIcon.cpp	2011-07-26 12:37:20 +0000
@@ -19,428 +19,397 @@
 
 #include "DeviceLauncherIcon.h"
 
+#include <algorithm>
+#include <list>
+
+#include <glib/gi18n-lib.h>
+#include <libnotify/notify.h>
+
+#include "DevicesSettings.h"
 #include "ubus-server.h"
 #include "UBusMessages.h"
 
-#include <glib/gi18n-lib.h>
-#include <libnotify/notify.h>
-
-#define DEFAULT_ICON "drive-removable-media"
+namespace unity {
 
 DeviceLauncherIcon::DeviceLauncherIcon(Launcher* launcher, GVolume* volume)
-  : SimpleLauncherIcon(launcher),
-    _volume(volume)
+  : SimpleLauncherIcon(launcher)
+  , volume_(volume)
 {
-
-  DevicesSettings::GetDefault()->changed.connect(sigc::mem_fun(this, &DeviceLauncherIcon::OnSettingsChanged));
-
-  _on_removed_handler_id = g_signal_connect(_volume,
-                                            "removed",
-                                            G_CALLBACK(&DeviceLauncherIcon::OnRemoved),
-                                            this);
-
-  _on_changed_handler_id = g_signal_connect(_volume,
-                                            "changed",
-                                            G_CALLBACK(&DeviceLauncherIcon::OnChanged),
-                                            this);
-
+  DevicesSettings::GetDefault().changed.connect(sigc::mem_fun(this, &DeviceLauncherIcon::OnSettingsChanged));
+
+  // Checks if in favourites!
+  glib::String uuid(g_volume_get_identifier(volume_, G_VOLUME_IDENTIFIER_KIND_UUID));
+  DeviceList favorites = DevicesSettings::GetDefault().GetFavorites();
+  DeviceList::iterator pos = std::find(favorites.begin(), favorites.end(), uuid.Str());
+
+  keep_in_launcher_ = pos != favorites.end();
+  
   UpdateDeviceIcon();
-
   UpdateVisibility();
-
-}
-
-DeviceLauncherIcon::~DeviceLauncherIcon()
-{
-  if (_on_removed_handler_id != 0)
-    g_signal_handler_disconnect((gpointer) _volume, _on_removed_handler_id);
-
-  if (_on_changed_handler_id != 0)
-    g_signal_handler_disconnect((gpointer) _volume, _on_changed_handler_id);
-}
-
-void
-DeviceLauncherIcon::UpdateDeviceIcon()
-{
-  {
-    gchar* name;
-    name = g_volume_get_name(_volume);
-    tooltip_text = name;
-
-    g_free(name);
-  }
-
-  {
-    GIcon* icon;
-    gchar* icon_string;
-
-    icon = g_volume_get_icon(_volume);
-    icon_string = g_icon_to_string(icon);
-
-    SetIconName(icon_string);
-
-    g_object_unref(icon);
-    g_free(icon_string);
-  }
-
-  SetQuirk(QUIRK_VISIBLE, true);
+}
+
+void DeviceLauncherIcon::UpdateDeviceIcon()
+{
+  glib::String name(g_volume_get_name(volume_));
+  glib::Object<GIcon> icon(g_volume_get_icon(volume_));
+  glib::String icon_string(g_icon_to_string(icon));
+
+  tooltip_text = name.Value();
+  SetIconName(icon_string.Value());
+  
+  SetIconType(TYPE_DEVICE);
   SetQuirk(QUIRK_RUNNING, false);
-  SetIconType(TYPE_DEVICE);
-}
-
-nux::Color
-DeviceLauncherIcon::BackgroundColor()
-{
-  return nux::Color(0xFF333333);
-}
-
-nux::Color
-DeviceLauncherIcon::GlowColor()
-{
-  return nux::Color(0xFF333333);
 }
 
 bool
 DeviceLauncherIcon::CanEject()
 {
-  return g_volume_can_eject(_volume);
-}
-
-std::list<DbusmenuMenuitem*>
-DeviceLauncherIcon::GetMenus()
-{
-  std::list<DbusmenuMenuitem*>  result;
-  DbusmenuMenuitem*              menu_item;
-  GDrive*                        drive;
-
+  return g_volume_can_eject(volume_);
+}
+
+nux::Color DeviceLauncherIcon::BackgroundColor()
+{
+  return nux::Color(0xFF333333);
+}
+
+nux::Color DeviceLauncherIcon::GlowColor()
+{
+  return nux::Color(0xFF333333);
+}
+
+std::list<DbusmenuMenuitem*> DeviceLauncherIcon::GetMenus()
+{
+  std::list<DbusmenuMenuitem*> result;
+  DbusmenuMenuitem* menu_item;
+  glib::Object<GDrive> drive(g_volume_get_drive(volume_));
+
+  // "Keep in launcher" item
+  if (DevicesSettings::GetDefault().GetDevicesOption() == DevicesSettings::ONLY_MOUNTED
+      && drive && !g_drive_is_media_removable (drive))
+  {
+    menu_item = dbusmenu_menuitem_new();
+
+    dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Keep in launcher"));
+    dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_TOGGLE_TYPE, DBUSMENU_MENUITEM_TOGGLE_CHECK);
+    dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
+    dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+    dbusmenu_menuitem_property_set_int(menu_item, DBUSMENU_MENUITEM_PROP_TOGGLE_STATE, !keep_in_launcher_);
+    
+    g_signal_connect(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
+                     G_CALLBACK(&DeviceLauncherIcon::OnTogglePin), this);
+
+    result.push_back(menu_item);
+  }
+  
+  // "Open" item
   menu_item = dbusmenu_menuitem_new();
+
   dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Open"));
   dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
   dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+
   g_signal_connect(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                    G_CALLBACK(&DeviceLauncherIcon::OnOpen), this);
+                   
   result.push_back(menu_item);
 
-  if (g_volume_can_eject(_volume))
+  // "Eject" item
+  if (g_volume_can_eject(volume_))
   {
     menu_item = dbusmenu_menuitem_new();
+
     dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Eject"));
     dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
     dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+
     g_signal_connect(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                      G_CALLBACK(&DeviceLauncherIcon::OnEject), this);
+
     result.push_back(menu_item);
   }
 
-  drive = g_volume_get_drive(_volume);
+  // "Safely Remove" item (FIXME: Should it be "Safely remove"?)
   if (drive && g_drive_can_stop(drive))
   {
     menu_item = dbusmenu_menuitem_new();
+
     dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Safely Remove"));
     dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
     dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+
     g_signal_connect(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                      G_CALLBACK(&DeviceLauncherIcon::OnDriveStop), this);
+                     
     result.push_back(menu_item);
   }
-  if (drive)
-  {
-    g_object_unref(drive);
-  }
-
-  if (!g_volume_can_eject(_volume))   // Don't need Unmount if can Eject
-  {
-    GMount* mount = g_volume_get_mount(_volume);
+
+  // "Unmount" item
+  if (!g_volume_can_eject(volume_))  // Don't need Unmount if can Eject
+  {
+    glib::Object<GMount> mount(g_volume_get_mount(volume_));
+
     if (mount && g_mount_can_unmount(mount))
     {
       menu_item = dbusmenu_menuitem_new();
+
       dbusmenu_menuitem_property_set(menu_item, DBUSMENU_MENUITEM_PROP_LABEL, _("Unmount"));
       dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_ENABLED, true);
       dbusmenu_menuitem_property_set_bool(menu_item, DBUSMENU_MENUITEM_PROP_VISIBLE, true);
+
       g_signal_connect(menu_item, DBUSMENU_MENUITEM_SIGNAL_ITEM_ACTIVATED,
                        G_CALLBACK(&DeviceLauncherIcon::OnUnmount), this);
+
       result.push_back(menu_item);
     }
-    if (mount)
-    {
-      g_object_unref(mount);
-    }
   }
 
   return result;
 }
 
-void
-DeviceLauncherIcon::ShowMount(GMount* mount)
+void DeviceLauncherIcon::ShowMount(GMount* mount)
 {
-  gchar*  name;
-
-  name = g_volume_get_name(_volume);
+  glib::String name(g_volume_get_name(volume_));
+  
   if (G_IS_MOUNT(mount))
   {
-    GFile* root;
+    glib::Object<GFile> root(g_mount_get_root(mount));
 
-    root = g_mount_get_root(mount);
-    if (G_IS_FILE(root))
+    if (G_IS_FILE(root.RawPtr()))
     {
-      gchar*  uri;
-      GError* error = NULL;
-
-      uri = g_file_get_uri(root);
-
-      g_app_info_launch_default_for_uri(uri, NULL, &error);
+      glib::String uri(g_file_get_uri(root));
+      glib::Error error;
+
+      g_app_info_launch_default_for_uri(uri.Value(), NULL, error.AsOutParam());
+
       if (error)
-      {
-        g_warning("Cannot open volume '%s': Unable to show %s: %s", name, uri, error->message);
-        g_error_free(error);
-      }
-
-      g_free(uri);
-      g_object_unref(root);
+        g_warning("Cannot open volume '%s': Unable to show %s: %s", name.Value(), uri.Value(), error.Message().c_str());
     }
     else
     {
-      g_warning("Cannot open volume '%s': Mount has no root", name);
+      g_warning ("Cannot open volume '%s': Mount has no root", name.Value());
     }
   }
   else
   {
-    g_warning("Cannot open volume '%s': Mount-point is invalid", name);
+    g_warning ("Cannot open volume '%s': Mount-point is invalid", name.Value());
   }
-
-  g_free(name);
 }
 
-void
-DeviceLauncherIcon::ActivateLauncherIcon(ActionArg arg)
+void DeviceLauncherIcon::ActivateLauncherIcon(ActionArg arg)
 {
-  GMount* mount;
-  gchar*  name;
-
   SetQuirk(QUIRK_STARTING, true);
+  
+  glib::Object<GMount> mount(g_volume_get_mount(volume_));
 
-  name = g_volume_get_name(_volume);
-  mount = g_volume_get_mount(_volume);
-  if (G_IS_MOUNT(mount))
-  {
+  if (G_IS_MOUNT(mount.RawPtr()))
     ShowMount(mount);
-    g_object_unref(mount);
-  }
   else
-  {
-    g_volume_mount(_volume,
+    g_volume_mount(volume_,
                    (GMountMountFlags)0,
                    NULL,
                    NULL,
                    (GAsyncReadyCallback)&DeviceLauncherIcon::OnMountReady,
                    this);
-  }
-
-  g_free(name);
 }
 
-void
-DeviceLauncherIcon::OnMountReady(GObject* object, GAsyncResult* result, DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnMountReady(GObject* object,
+                                      GAsyncResult* result,
+                                      DeviceLauncherIcon* self)
 {
-  GError* error = NULL;
+  glib::Error error;
 
-  if (g_volume_mount_finish(self->_volume, result, &error))
+  if (g_volume_mount_finish(self->volume_, result, error.AsOutParam()))
   {
-    GMount* mount;
-
-    mount = g_volume_get_mount(self->_volume);
+    glib::Object<GMount> mount(g_volume_get_mount(self->volume_));
     self->ShowMount(mount);
-
-    g_object_unref(mount);
   }
   else
   {
-    gchar* name;
+    glib::String name(g_volume_get_name(self->volume_));
 
-    name = g_volume_get_name(self->_volume);
     g_warning("Cannot open volume '%s': %s",
-              name,
-              error ? error->message : "Mount operation failed");
-    g_error_free(error);
-
-    g_free(name);
+              name.Value(),
+              error ? error.Message().c_str() : "Mount operation failed");
   }
 }
 
-void
-DeviceLauncherIcon::OnEjectReady(GObject*            object,
-                                 GAsyncResult*       result,
-                                 DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnEjectReady(GObject* object,
+                                      GAsyncResult* result,
+                                      DeviceLauncherIcon* self)
 {
-  if (g_volume_eject_with_operation_finish(self->_volume, result, NULL))
+  if (g_volume_eject_with_operation_finish(self->volume_, result, NULL))
   {
     NotifyNotification* notification;
-    gchar*  name = g_volume_get_name(self->_volume);
+    glib::String name(g_volume_get_name(self->volume_));
 
-    notification = notify_notification_new(name,
+    notification = notify_notification_new(name.Value(),
                                            _("The drive has been successfully ejected"),
                                            "drive-removable-media-usb");
 
     notify_notification_show(notification, NULL);
-
-    g_free(name);
   }
 }
 
-void
-DeviceLauncherIcon::Eject()
+void DeviceLauncherIcon::Eject()
 {
-  GMountOperation* mount_op;
-
-  mount_op = gtk_mount_operation_new(NULL);
-
-  g_volume_eject_with_operation(_volume,
+  glib::Object<GMountOperation> mount_op(gtk_mount_operation_new(NULL));
+
+  g_volume_eject_with_operation(volume_,
                                 (GMountUnmountFlags)0,
                                 mount_op,
                                 NULL,
                                 (GAsyncReadyCallback)OnEjectReady,
                                 this);
-
-  g_object_unref(mount_op);
-}
-
-void
-DeviceLauncherIcon::OnOpen(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self)
+}
+
+void DeviceLauncherIcon::OnTogglePin(DbusmenuMenuitem* item,
+                                     int time,
+                                     DeviceLauncherIcon* self)
+{
+  glib::String uuid(g_volume_get_identifier(self->volume_, G_VOLUME_IDENTIFIER_KIND_UUID));
+  
+  self->keep_in_launcher_ = !self->keep_in_launcher_;
+
+  if (!self->keep_in_launcher_)
+  {
+    // If the volume is not mounted hide the icon
+    glib::Object<GMount> mount(g_volume_get_mount(self->volume_));
+
+    if (!mount)
+      self->SetQuirk(QUIRK_VISIBLE, false); 
+
+    // Remove from favorites
+    if (!uuid.Str().empty())
+      DevicesSettings::GetDefault().RemoveFavorite(uuid.Str());
+  }
+  else
+  {
+    if (!uuid.Str().empty())
+      DevicesSettings::GetDefault().AddFavorite(uuid.Str());
+  }
+}
+
+void DeviceLauncherIcon::OnOpen(DbusmenuMenuitem* item,
+                                int time,
+                                DeviceLauncherIcon* self)
 {
   self->ActivateLauncherIcon(ActionArg(ActionArg::OTHER, 0));
 }
 
-void
-DeviceLauncherIcon::OnEject(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnEject(DbusmenuMenuitem* item,
+                                 int time,
+                                 DeviceLauncherIcon* self)
 {
   self->Eject();
 }
 
-void
-DeviceLauncherIcon::OnUnmountReady(GObject*            object,
-                                   GAsyncResult*       result,
-                                   DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnUnmountReady(GObject* object,
+                                        GAsyncResult* result,
+                                        DeviceLauncherIcon* self)
 {
   if (G_IS_MOUNT(object))
-  {
     g_mount_unmount_with_operation_finish(G_MOUNT(object), result, NULL);
-  }
 }
 
-void
-DeviceLauncherIcon::Unmount()
+void DeviceLauncherIcon::Unmount()
 {
-  GMount* mount = g_volume_get_mount(_volume);
+  glib::Object<GMount> mount(g_volume_get_mount(volume_));
+  
   if (mount)
   {
-    GMountOperation* op = gtk_mount_operation_new(NULL);
+    glib::Object<GMountOperation> op(gtk_mount_operation_new(NULL));
+
     g_mount_unmount_with_operation(mount,
                                    (GMountUnmountFlags)0,
                                    op,
                                    NULL,
                                    (GAsyncReadyCallback)OnUnmountReady,
                                    this);
-    g_object_unref(op);
-    g_object_unref(mount);
   }
 }
 
-void
-DeviceLauncherIcon::OnUnmount(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnUnmount(DbusmenuMenuitem* item,
+                                   int time,
+                                   DeviceLauncherIcon* self)
 {
   self->Unmount();
 }
 
-void
-DeviceLauncherIcon::OnRemoved(GVolume* volume, DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnRemoved()
 {
-  self->_volume = NULL;
-  self->Remove();
+  Remove();
 }
 
-void
-DeviceLauncherIcon::OnDriveStop(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnDriveStop(DbusmenuMenuitem* item,
+                                     int time,
+                                     DeviceLauncherIcon* self)
 {
   self->StopDrive();
 }
 
-void
-DeviceLauncherIcon::StopDrive()
+void DeviceLauncherIcon::StopDrive()
 {
-  GDrive*          drive;
-  GMountOperation* mount_op;
+  glib::Object<GDrive> drive(g_volume_get_drive(volume_));
+  glib::Object<GMountOperation> mount_op(gtk_mount_operation_new(NULL));
 
-  drive = g_volume_get_drive(_volume);
-  mount_op = gtk_mount_operation_new(NULL);
   g_drive_stop(drive,
                (GMountUnmountFlags)0,
                mount_op,
                NULL,
                (GAsyncReadyCallback)OnStopDriveReady,
                this);
-  g_object_unref(drive);
-  g_object_unref(mount_op);
 }
 
-void
-DeviceLauncherIcon::OnStopDriveReady(GObject* object,
-                                     GAsyncResult* result,
-                                     DeviceLauncherIcon* self)
+void DeviceLauncherIcon::OnStopDriveReady(GObject* object,
+                                          GAsyncResult* result,
+                                          DeviceLauncherIcon* self)
 {
-  GDrive* drive;
 
-  if (!self || !G_IS_VOLUME(self->_volume))
-  {
+  if (!self || !G_IS_VOLUME(self->volume_))
     return;
-  }
 
-  drive = g_volume_get_drive(self->_volume);
+  glib::Object<GDrive> drive(g_volume_get_drive(self->volume_));
   g_drive_stop_finish(drive, result, NULL);
-  g_object_unref(drive);
-}
-
-void
-DeviceLauncherIcon::OnChanged(GVolume* volume, DeviceLauncherIcon* self)
-{
-  if (DevicesSettings::GetDefault()->GetDevicesOption() == DevicesSettings::ONLY_MOUNTED
-      && g_volume_get_mount(volume) == NULL)
-  {
-    self->SetQuirk(QUIRK_VISIBLE, false);
-  }
-}
-
-void
-DeviceLauncherIcon::UpdateVisibility()
-{
-  switch (DevicesSettings::GetDefault()->GetDevicesOption())
+}
+
+void DeviceLauncherIcon::UpdateVisibility(int visibility)
+{
+  switch (DevicesSettings::GetDefault().GetDevicesOption())
   {
     case DevicesSettings::NEVER:
       SetQuirk(QUIRK_VISIBLE, false);
       break;
     case DevicesSettings::ONLY_MOUNTED:
-    {
-      GMount* mount =  g_volume_get_mount(_volume);
-
-      if (mount == NULL)
-      {
-        SetQuirk(QUIRK_VISIBLE, false);
+      if (keep_in_launcher_)
+      {
+        SetQuirk(QUIRK_VISIBLE, true);
+      }
+      else if (visibility < 0)
+      {
+        glib::Object<GMount> mount(g_volume_get_mount(volume_));
+        SetQuirk(QUIRK_VISIBLE, mount.RawPtr() != NULL); 
       }
       else
       {
-        SetQuirk(QUIRK_VISIBLE, true);
-        g_object_unref(mount);
+        SetQuirk(QUIRK_VISIBLE, visibility); 
       }
       break;
-    }
     case DevicesSettings::ALWAYS:
       SetQuirk(QUIRK_VISIBLE, true);
       break;
   }
 }
 
-void
-DeviceLauncherIcon::OnSettingsChanged(DevicesSettings*     settings)
+void DeviceLauncherIcon::OnSettingsChanged()
 {
+  // Checks if in favourites!
+  glib::String uuid(g_volume_get_identifier(volume_, G_VOLUME_IDENTIFIER_KIND_UUID));
+  DeviceList favorites = DevicesSettings::GetDefault().GetFavorites();
+  DeviceList::iterator pos = std::find(favorites.begin(), favorites.end(), uuid.Str());
+
+  keep_in_launcher_ = pos != favorites.end();
+
   UpdateVisibility();
 }
 
+} // namespace unity

=== modified file 'plugins/unityshell/src/DeviceLauncherIcon.h'
--- plugins/unityshell/src/DeviceLauncherIcon.h	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DeviceLauncherIcon.h	2011-07-26 12:37:20 +0000
@@ -20,21 +20,23 @@
 #ifndef _DEVICE_LAUNCHER_ICON_H__H
 #define _DEVICE_LAUNCHER_ICON_H__H
 
+#include <gio/gio.h>
+#include <UnityCore/GLibWrapper.h>
+
 #include "SimpleLauncherIcon.h"
-#include "DevicesSettings.h"
 
-#include <gio/gio.h>
+namespace unity {
 
 class DeviceLauncherIcon : public SimpleLauncherIcon
 {
 
 public:
   DeviceLauncherIcon(Launcher* launcher, GVolume* volume);
-  ~DeviceLauncherIcon();
 
   virtual nux::Color BackgroundColor();
   virtual nux::Color GlowColor();
-  void UpdateVisibility();
+  void UpdateVisibility(int visibility = -1);
+  void OnRemoved();
   bool CanEject();
   void Eject();
 
@@ -47,22 +49,23 @@
   void ShowMount(GMount* mount);
   void Unmount();
   void StopDrive();
+  static void OnTogglePin(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self);
   static void OnOpen(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self);
   static void OnEject(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self);
   static void OnUnmount(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self);
-  static void OnRemoved(GVolume* volume, DeviceLauncherIcon* self);
   static void OnChanged(GVolume* volume, DeviceLauncherIcon* self);
   static void OnMountReady(GObject* object, GAsyncResult* result, DeviceLauncherIcon* self);
   static void OnEjectReady(GObject* object, GAsyncResult* result, DeviceLauncherIcon* self);
   static void OnUnmountReady(GObject* object, GAsyncResult* result, DeviceLauncherIcon* self);
   static void OnDriveStop(DbusmenuMenuitem* item, int time, DeviceLauncherIcon* self);
   static void OnStopDriveReady(GObject* object, GAsyncResult* result, DeviceLauncherIcon* self);
-  void OnSettingsChanged(DevicesSettings* settings);
+  void OnSettingsChanged();
 
 private:
-  GVolume* _volume;
-  gulong _on_removed_handler_id;
-  gulong _on_changed_handler_id;
+  GVolume* volume_;
+  bool keep_in_launcher_;
 };
 
+} // namespace unity
+
 #endif // _DEVICE_LAUNCHER_ICON_H__H

=== modified file 'plugins/unityshell/src/DeviceLauncherSection.cpp'
--- plugins/unityshell/src/DeviceLauncherSection.cpp	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DeviceLauncherSection.cpp	2011-07-26 12:37:20 +0000
@@ -16,108 +16,136 @@
  * Authored by: Neil Jagdish Patel <[e-mail]>
  */
 
-#include "DeviceLauncherIcon.h"
-
 #include "DeviceLauncherSection.h"
 
+namespace unity
+{
+
 DeviceLauncherSection::DeviceLauncherSection(Launcher* launcher)
-  : _launcher(launcher)
-{
-  _monitor = g_volume_monitor_get();
-  _ht = g_hash_table_new(g_direct_hash , g_direct_equal);
-
-  _on_volume_added_handler_id = g_signal_connect(_monitor,
+  : launcher_(launcher)
+  , monitor_(g_volume_monitor_get())
+{  
+  on_volume_added_handler_id_ = g_signal_connect(monitor_,
                                                  "volume-added",
                                                  G_CALLBACK(&DeviceLauncherSection::OnVolumeAdded),
                                                  this);
 
-  _on_volume_removed_handler_id = g_signal_connect(_monitor,
+  on_volume_removed_handler_id_ = g_signal_connect(monitor_,
                                                    "volume-removed",
                                                    G_CALLBACK(&DeviceLauncherSection::OnVolumeRemoved),
                                                    this);
-
-  _on_mount_added_handler_id = g_signal_connect(_monitor,
+    
+  on_mount_added_handler_id_ = g_signal_connect(monitor_,
                                                 "mount-added",
                                                 G_CALLBACK(&DeviceLauncherSection::OnMountAdded),
                                                 this);
+                                                
+  on_mount_pre_unmount_handler_id_ = g_signal_connect(monitor_,
+                                                      "mount-pre-unmount",
+                                                      G_CALLBACK(&DeviceLauncherSection::OnMountPreUnmount),
+                                                      this);
 
-  _on_device_populate_entry_id = g_idle_add((GSourceFunc)&DeviceLauncherSection::PopulateEntries, this);
+  on_device_populate_entry_id_ = g_idle_add((GSourceFunc)&DeviceLauncherSection::PopulateEntries, this);
 }
 
 DeviceLauncherSection::~DeviceLauncherSection()
 {
-  if (_on_volume_added_handler_id != 0)
-    g_signal_handler_disconnect((gpointer) _monitor,
-                                _on_volume_added_handler_id);
-
-  if (_on_volume_removed_handler_id != 0)
-    g_signal_handler_disconnect((gpointer) _monitor,
-                                _on_volume_removed_handler_id);
-
-  if (_on_mount_added_handler_id != 0)
-    g_signal_handler_disconnect((gpointer) _monitor,
-                                _on_mount_added_handler_id);
-
-  if (_on_device_populate_entry_id)
-    g_source_remove(_on_device_populate_entry_id);
-
-  g_object_unref(_monitor);
-  g_hash_table_unref(_ht);
+  if (on_volume_added_handler_id_)
+    g_signal_handler_disconnect((gpointer) monitor_,
+                                on_volume_added_handler_id_);
+
+  if (on_volume_removed_handler_id_)
+    g_signal_handler_disconnect((gpointer) monitor_,
+                                on_volume_removed_handler_id_);
+
+  if (on_mount_added_handler_id_)
+    g_signal_handler_disconnect((gpointer) monitor_,
+                                on_mount_added_handler_id_);
+
+  if (on_mount_pre_unmount_handler_id_)
+    g_signal_handler_disconnect((gpointer) monitor_,
+                                on_mount_pre_unmount_handler_id_);
+
+  if (on_device_populate_entry_id_)
+    g_source_remove(on_device_populate_entry_id_);
 }
 
-bool
-DeviceLauncherSection::PopulateEntries(DeviceLauncherSection* self)
+bool DeviceLauncherSection::PopulateEntries(DeviceLauncherSection* self)
 {
-  GList* volumes, *v;
+  GList* volumes = g_volume_monitor_get_volumes(self->monitor_);
 
-  volumes = g_volume_monitor_get_volumes(self->_monitor);
-  for (v = volumes; v; v = v->next)
+  for (GList* v = volumes; v; v = v->next)
   {
-    GVolume* volume = (GVolume*)v->data;
-    DeviceLauncherIcon* icon = new DeviceLauncherIcon(self->_launcher, volume);
+    glib::Object<GVolume> volume((GVolume* )v->data);
+    DeviceLauncherIcon* icon = new DeviceLauncherIcon(self->launcher_, volume);
 
+    self->map_[volume] = icon;
     self->IconAdded.emit(icon);
-
-    g_hash_table_insert(self->_ht, (gpointer) volume, (gpointer) icon);
-
-    g_object_unref(volume);
   }
 
   g_list_free(volumes);
-
-  self->_on_device_populate_entry_id = 0;
-
+  
+  self->on_device_populate_entry_id_ = 0;
+  
   return false;
 }
 
-void
-DeviceLauncherSection::OnVolumeAdded(GVolumeMonitor*        monitor,
-                                     GVolume*               volume,
-                                     DeviceLauncherSection* self)
+/* Uses a std::map to track all the volume icons shown and not shown.
+ * Keep in mind: when "volume-removed" is recevied we should erase
+ * the pair (GVolume - DeviceLauncherIcon) from the std::map to avoid leaks
+ */
+void DeviceLauncherSection::OnVolumeAdded(GVolumeMonitor* monitor,
+                                          GVolume* volume,
+                                          DeviceLauncherSection* self)
 {
-  DeviceLauncherIcon* icon = new DeviceLauncherIcon(self->_launcher, volume);
-
-  g_hash_table_insert(self->_ht, (gpointer) volume, (gpointer) icon);
-
+  DeviceLauncherIcon* icon = new DeviceLauncherIcon(self->launcher_, volume);
+  
+  self->map_[volume] = icon;
   self->IconAdded.emit(icon);
 }
 
-void
-DeviceLauncherSection::OnVolumeRemoved(GVolumeMonitor*        monitor,
-                                       GVolume*               volume,
-                                       DeviceLauncherSection* self)
-{
-  g_hash_table_remove(self->_ht, (gpointer) volume);
-}
-
-#include "SimpleLauncherIcon.h"
-void
-DeviceLauncherSection::OnMountAdded(GVolumeMonitor*        monitor,
-                                    GMount*                mount,
-                                    DeviceLauncherSection* self)
-{
-  GVolume* volume = g_mount_get_volume(mount);
-  DeviceLauncherIcon* icon = (DeviceLauncherIcon*) g_hash_table_lookup(self->_ht, (gpointer) volume);
-  if (icon)
-    icon->UpdateVisibility();
-}
+void DeviceLauncherSection::OnVolumeRemoved(GVolumeMonitor* monitor,
+                                            GVolume* volume,
+                                            DeviceLauncherSection* self)
+{
+  // It should not happen! Let me do the check anyway.
+  if (self->map_.find(volume) != self->map_.end())
+  {	
+    self->map_[volume]->OnRemoved();
+    self->map_.erase(volume);
+  }
+}
+
+/* Keep in mind: we could have a GMount without a related GVolume
+ * so check everything to avoid unwanted behaviors.
+ */
+void DeviceLauncherSection::OnMountAdded(GVolumeMonitor* monitor,
+                                         GMount* mount,
+                                         DeviceLauncherSection* self)
+{
+  std::map<GVolume* , DeviceLauncherIcon* >::iterator it;
+  glib::Object<GVolume> volume(g_mount_get_volume(mount));
+
+  it = self->map_.find(volume);
+
+  if (it != self->map_.end())
+    it->second->UpdateVisibility(1);
+}
+
+/* We don't use "mount-removed" signal since it is received after "volume-removed"
+ * signal. You should read also the comment above.
+ */
+void DeviceLauncherSection::OnMountPreUnmount(GVolumeMonitor* monitor,
+                                              GMount* mount,
+                                              DeviceLauncherSection* self)
+{
+  std::map<GVolume* , DeviceLauncherIcon* >::iterator it;
+  glib::Object<GVolume> volume(g_mount_get_volume(mount));
+
+  it = self->map_.find(volume);
+
+  if (it != self->map_.end())
+    it->second->UpdateVisibility(0);
+}
+
+} // namespace unity

=== modified file 'plugins/unityshell/src/DeviceLauncherSection.h'
--- plugins/unityshell/src/DeviceLauncherSection.h	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DeviceLauncherSection.h	2011-07-26 12:37:20 +0000
@@ -19,14 +19,19 @@
 #ifndef _DEVICE_LAUNCHER_SECTION_H_
 #define _DEVICE_LAUNCHER_SECTION_H_
 
+#include <map>
+
+#include <gio/gio.h>
 #include <sigc++/sigc++.h>
 #include <sigc++/signal.h>
-
-#include "Launcher.h"
-#include "LauncherIcon.h"
-#include "DevicesSettings.h"
-
-#include <gio/gio.h>
+#include <UnityCore/GLibWrapper.h>
+
+#include "DeviceLauncherIcon.h"
+
+class Launcher;
+class LauncherIcon;
+
+namespace unity {
 
 class DeviceLauncherSection : public sigc::trackable
 {
@@ -38,25 +43,34 @@
 
 private:
   static bool PopulateEntries(DeviceLauncherSection* self);
-  static void OnVolumeAdded(GVolumeMonitor*        monitor,
-                            GVolume*               volume,
+  
+  static void OnVolumeAdded(GVolumeMonitor* monitor,
+                            GVolume* volume,
                             DeviceLauncherSection* self);
-  static void OnVolumeRemoved(GVolumeMonitor*        monitor,
-                              GVolume*               volume,
+
+  static void OnVolumeRemoved(GVolumeMonitor* monitor,
+                              GVolume* volume,
                               DeviceLauncherSection* self);
-  static void OnMountAdded(GVolumeMonitor*        monitor,
-                           GMount*                mount,
+
+  static void OnMountAdded(GVolumeMonitor* monitor,
+                           GMount* mount,
                            DeviceLauncherSection* self);
-public:
-  Launcher*       _launcher;
-  GVolumeMonitor* _monitor;
-  GHashTable*     _ht;
+
+  static void OnMountPreUnmount(GVolumeMonitor* monitor,
+                                GMount* mount,
+                                DeviceLauncherSection* self);
 
 private:
-  gulong _on_volume_added_handler_id;
-  gulong _on_volume_removed_handler_id;
-  gulong _on_mount_added_handler_id;
-  gulong _on_device_populate_entry_id;
+  Launcher* launcher_;
+  glib::Object<GVolumeMonitor> monitor_;
+  std::map<GVolume*, DeviceLauncherIcon*> map_;
+  gulong on_volume_added_handler_id_;
+  gulong on_volume_removed_handler_id_;
+  gulong on_mount_added_handler_id_;
+  gulong on_mount_pre_unmount_handler_id_;
+  gulong on_device_populate_entry_id_;
 };
 
+} // namespace unity
+
 #endif // _DEVICE_LAUNCHER_SECTION_H_

=== modified file 'plugins/unityshell/src/DevicesSettings.cpp'
--- plugins/unityshell/src/DevicesSettings.cpp	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DevicesSettings.cpp	2011-07-26 12:37:20 +0000
@@ -1,69 +1,143 @@
 // -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
-* Copyright (C) 2010 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 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 warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-* GNU General Public License for more details.
-*
-* You should have received a copy of the GNU General Public License
-* along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*
-* Authored by: Andrea Azzarone <[e-mail]>
-*/
+ * Copyright (C) 2010 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 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 warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andrea Azzarone <[e-mail]>
+ */
 
 #include "DevicesSettings.h"
 
-static DevicesSettings* _devices_settings = NULL;
+#include <algorithm>
+
+namespace unity {
+
+namespace {
+
+const char* SETTINGS_NAME = "com.canonical.Unity.Devices";
+
+void on_settings_updated(GSettings* settings,
+                         const gchar* key,
+                         DevicesSettings* self);
+
+} // anonymous namespace
+
+DevicesSettings& DevicesSettings::GetDefault()
+{
+  static DevicesSettings instance;
+  return instance;
+}
 
 DevicesSettings::DevicesSettings()
-  : _settings(NULL),
-    _raw_devices_option(1),
-    _devices_option(ONLY_MOUNTED)
-{
-  _settings = g_settings_new("com.canonical.Unity.Devices");
-  g_signal_connect(_settings, "changed",
-                   (GCallback)(DevicesSettings::Changed), this);
-  Refresh();
-}
-
-DevicesSettings::~DevicesSettings()
-{
-  g_object_unref(_settings);
-}
-
-void
-DevicesSettings::Refresh()
-{
-  _raw_devices_option = g_settings_get_enum(_settings, "devices-option");
-
-  _devices_option = (DevicesOption)_raw_devices_option;
-
-  changed.emit(this);
-}
-
-void
-DevicesSettings::Changed(GSettings* settings, char* key, DevicesSettings* self)
-{
-  self->Refresh();
-}
-
-DevicesSettings*
-DevicesSettings::GetDefault()
-{
-  if (G_UNLIKELY(!_devices_settings))
-    _devices_settings = new DevicesSettings();
-
-  return _devices_settings;
-}
-
-DevicesSettings::DevicesOption
-DevicesSettings::GetDevicesOption()
-{
-  return _devices_option;
-}
+  : settings_(g_settings_new(SETTINGS_NAME))
+  , ignore_signals_(false)
+  , devices_option_(ONLY_MOUNTED)
+{
+
+  g_signal_connect(settings_, "changed", G_CALLBACK(on_settings_updated), this);
+
+  Refresh();
+}
+
+void DevicesSettings::Refresh()
+{
+  gchar** favs = g_settings_get_strv(settings_, "favorites");
+
+  favorites_.clear();
+
+  for (int i = 0; favs[i] != NULL; i++)
+    favorites_.push_back(favs[i]);
+
+  g_strfreev(favs);
+}
+
+void DevicesSettings::AddFavorite(std::string const& uuid)
+{
+  if (uuid.empty())
+    return;
+  
+  favorites_.push_back(uuid);
+
+  SaveFavorites(favorites_);
+  Refresh();
+}
+
+void DevicesSettings::RemoveFavorite(std::string const& uuid)
+{
+  if (uuid.empty())
+    return;
+
+  DeviceList::iterator pos = std::find(favorites_.begin(), favorites_.end(), uuid);
+  if (pos == favorites_.end())
+    return;
+
+  favorites_.erase(pos);
+  SaveFavorites(favorites_);
+  Refresh();
+}
+
+void DevicesSettings::SaveFavorites(DeviceList const& favorites)
+{
+  const int size = favorites.size();
+  const char* favs[size + 1];
+  favs[size] = NULL;
+
+  int index = 0;
+  for (DeviceList::const_iterator i = favorites.begin(), end = favorites.end();
+       i != end; ++i, ++index)
+  {
+    favs[index] = i->c_str();
+  }
+
+  ignore_signals_ = true;
+  if (!g_settings_set_strv(settings_, "favorites", favs))
+    g_warning("Saving favorites failed.");
+  ignore_signals_ = false;
+}
+
+
+void DevicesSettings::Changed(std::string const& key)
+{
+  if (ignore_signals_)
+    return;
+  
+  Refresh();
+  
+  changed.emit();
+}
+
+void DevicesSettings::SetDevicesOption(DevicesOption devices_option)
+{
+  if (devices_option == devices_option_)
+    return;
+
+  devices_option_ = devices_option;
+
+  changed.emit();
+}
+
+namespace {
+
+void on_settings_updated(GSettings* settings,
+                         const gchar* key,
+                         DevicesSettings* self)
+{
+  if (settings and key) {
+    self->Changed(key);
+  }
+}
+
+} // anonymous namespace
+
+} // namespace unity

=== modified file 'plugins/unityshell/src/DevicesSettings.h'
--- plugins/unityshell/src/DevicesSettings.h	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/DevicesSettings.h	2011-07-26 12:37:20 +0000
@@ -1,30 +1,38 @@
 // -*- Mode: C++; indent-tabs-mode: nil; tab-width: 2 -*-
 /*
-* Copyright (C) 2010 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 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 warranty of
-* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-* GNU General Public License for more details.
-*
-* You should have received a copy of the GNU General Public License
-* along with this program.  If not, see <http://www.gnu.org/licenses/>.
-*
-* Authored by: Andrea Azzarone <[e-mail]>
-*/
+ * Copyright (C) 2010 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 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 warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Andrea Azzarone <[e-mail]>
+ */
 
 #ifndef DEVICES_SETTINGS_H
 #define DEVICES_SETTINGS_H
 
+#include <list>
+#include <string>
+
 #include <gio/gio.h>
-#include <Nux/Nux.h>
-
-
-class DevicesSettings : public nux::Object
+#include <boost/utility.hpp>
+#include <sigc++/sigc++.h>
+#include <UnityCore/GLibWrapper.h>
+
+namespace unity {
+
+typedef std::list<std::string> DeviceList;
+
+class DevicesSettings : boost::noncopyable
 {
 public:
   typedef enum
@@ -36,22 +44,31 @@
   } DevicesOption;
 
   DevicesSettings();
-  ~DevicesSettings();
-
-  static DevicesSettings* GetDefault();
-
-  DevicesOption GetDevicesOption();
-
-  sigc::signal<void, DevicesSettings*> changed;
-
+  
+  static DevicesSettings& GetDefault();
+
+  void SetDevicesOption(DevicesOption devices_option);
+  DevicesOption GetDevicesOption() { return devices_option_; };
+
+  DeviceList const& GetFavorites() { return favorites_; };
+  void AddFavorite(std::string const& uuid);
+  void RemoveFavorite(std::string const& uuid);
+
+  void Changed(std::string const& key);
+  
+  // Signals
+  sigc::signal<void> changed;
+  
 private:
   void Refresh();
-  static void Changed(GSettings* settings, gchar* key, DevicesSettings* self);
+  void SaveFavorites(DeviceList const& favorites);
 
-private:
-  GSettings*    _settings;
-  int           _raw_devices_option;
-  DevicesOption _devices_option;
+  glib::Object<GSettings> settings_;
+  DeviceList favorites_;
+  bool ignore_signals_;
+  DevicesOption devices_option_;
 };
 
+} // namespace unity
+
 #endif // DEVICES_SETTINGS_H

=== modified file 'plugins/unityshell/src/LauncherController.h'
--- plugins/unityshell/src/LauncherController.h	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/LauncherController.h	2011-07-26 12:37:20 +0000
@@ -56,7 +56,7 @@
   LauncherModel*         _model;
   int                    _sort_priority;
   PlaceLauncherSection*  _place_section;
-  DeviceLauncherSection* _device_section;
+  unity::DeviceLauncherSection* _device_section;
   LauncherEntryRemoteModel* _remote_model;
   SimpleLauncherIcon*    _expoIcon;
   int                    _num_workspaces;

=== modified file 'plugins/unityshell/src/unityshell.cpp'
--- plugins/unityshell/src/unityshell.cpp	2011-07-21 14:59:25 +0000
+++ plugins/unityshell/src/unityshell.cpp	2011-07-26 12:37:20 +0000
@@ -30,6 +30,7 @@
 #include "LauncherController.h"
 #include "PlacesSettings.h"
 #include "GeisAdapter.h"
+#include "DevicesSettings.h"
 #include "PluginAdapter.h"
 #include "StartupNotifyService.h"
 #include "Timer.h"
@@ -146,6 +147,7 @@
   optionSetIconSizeNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
   optionSetAutohideAnimationNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
   optionSetDashBlurExperimentalNotify(boost::bind(&UnityScreen::optionChanged, this, _1, _2));
+  optionSetDevicesOptionNotify(boost::bind (&UnityScreen::optionChanged, this, _1, _2));
   optionSetShowLauncherInitiate(boost::bind(&UnityScreen::showLauncherKeyInitiate, this, _1, _2, _3));
   optionSetShowLauncherTerminate(boost::bind(&UnityScreen::showLauncherKeyTerminate, this, _1, _2, _3));
   optionSetKeyboardFocusInitiate(boost::bind(&UnityScreen::setKeyboardFocusKeyInitiate, this, _1, _2, _3));
@@ -967,18 +969,19 @@
       panelController->SetBFBSize(optionGetIconSize() + 18);
       launcher->SetIconSize(optionGetIconSize() + 6, optionGetIconSize());
       PlacesController::SetLauncherSize(optionGetIconSize() + 18);
-
       break;
     case UnityshellOptions::AutohideAnimation:
       launcher->SetAutoHideAnimation((Launcher::AutoHideAnimation) optionGetAutohideAnimation());
       break;
-
     case UnityshellOptions::DashBlurExperimental:
       PlacesSettings::GetDefault()->SetDashBlurType((PlacesSettings::DashBlurType)optionGetDashBlurExperimental());
       break;
     case UnityshellOptions::AutomaximizeValue:
       PluginAdapter::Default()->SetCoverageAreaBeforeAutomaximize(optionGetAutomaximizeValue() / 100.0f);
       break;
+    case UnityshellOptions::DevicesOption:
+      unity::DevicesSettings::GetDefault().SetDevicesOption((unity::DevicesSettings::DevicesOption) optionGetDevicesOption());
+      break;
     default:
       break;
   }

=== modified file 'plugins/unityshell/unityshell.xml.in'
--- plugins/unityshell/unityshell.xml.in	2011-07-16 02:30:03 +0000
+++ plugins/unityshell/unityshell.xml.in	2011-07-26 12:37:20 +0000
@@ -213,6 +213,7 @@
                         <_name>Static Blur</_name>
                     </desc>
                 </option>
+
                 <option name="automaximize_value" type="int">
                     <_short>Automaximize value</_short>
                     <_long>The minimum value to trigger automaximize.</_long>
@@ -220,6 +221,27 @@
                     <max>100</max>
                     <default>75</default>
                 </option>
+                
+                <option name="devices_option" type="int">
+                    <_short>Show Devices</_short>
+                    <_long>Show devices in the launcher</_long>
+                    <min>0</min>
+                    <max>2</max>
+                    <default>1</default>
+                    <desc>
+                        <value>0</value>
+                        <_name>Never</_name>
+                    </desc>
+                    <desc>
+                        <value>1</value>
+                        <_name>Only Mounted</_name>
+                    </desc>
+                    <desc>
+                        <value>2</value>
+                        <_name>Always</_name>
+                    </desc>
+                </option>
+
             </group>
         </options>
     </plugin>