Merge lp:~azzar1/unity/fix-994163 into lp:unity

[Andrea Azzarone]

Remove internal partitions after they have been unmounted.

== Problem ==
Unity launcher shows internal partitions after they have been unmounted

== Fix ==
Use "changed" signal instead of "mount-added"/"mount-removed".

== Test ==
Manual test

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

16 + typedef glib::Signal<void, GVolume*> VolumeSignal;
17 + sig_manager_.Add(new VolumeSignal(volume_, "changed", sigc::mem_fun(this, &DeviceLauncherIcon::OnVolumeChanged)));

Since DeviceLauncherIcon just uses one signal I think that you can avoid to use the SignalManager and save some overhead by just adding a glib::Signal class member and using its Connect method...

[Andrea Azzarone (azzar1)]

> 16 + typedef glib::Signal<void, GVolume*> VolumeSignal;
> 17 + sig_manager_.Add(new VolumeSignal(volume_, "changed",
> sigc::mem_fun(this, &DeviceLauncherIcon::OnVolumeChanged)));
>
> Since DeviceLauncherIcon just uses one signal I think that you can avoid to
> use the SignalManager and save some overhead by just adding a glib::Signal
> class member and using its Connect method...

Done.

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

Code looks good, fix works.

review: Approve

=== modified file 'launcher/DeviceLauncherIcon.cpp'
--- launcher/DeviceLauncherIcon.cpp	2012-06-15 02:03:31 +0000
+++ launcher/DeviceLauncherIcon.cpp	2012-06-20 06:00:31 +0000
@@ -40,12 +40,16 @@
 
 nux::logging::Logger logger("unity.launcher");
 
+const unsigned int volume_changed_timeout =  500;
+
 }
 
 DeviceLauncherIcon::DeviceLauncherIcon(glib::Object<GVolume> const& volume)
   : SimpleLauncherIcon()
   , volume_(volume)
 {
+  signal_volume_changed_.Connect(volume, "changed", sigc::mem_fun(this, &DeviceLauncherIcon::OnVolumeChanged));
+
   DevicesSettings::GetDefault().changed.connect(sigc::mem_fun(this, &DeviceLauncherIcon::OnSettingsChanged));
 
   // Checks if in favorites!
@@ -59,6 +63,42 @@
   UpdateVisibility();
 }
 
+void DeviceLauncherIcon::OnVolumeChanged(GVolume* volume)
+{
+  if (!G_IS_VOLUME(volume))
+    return;
+
+  changed_timeout_.reset(new glib::Timeout(volume_changed_timeout, [this]() {
+    UpdateDeviceIcon();
+    UpdateVisibility();
+    return false;
+  }));
+}
+
+void DeviceLauncherIcon::UpdateVisibility()
+{
+  switch (DevicesSettings::GetDefault().GetDevicesOption())
+  {
+    case DevicesSettings::NEVER:
+      SetQuirk(QUIRK_VISIBLE, false);
+      break;
+    case DevicesSettings::ONLY_MOUNTED:
+      if (keep_in_launcher_)
+      {
+        SetQuirk(QUIRK_VISIBLE, true);
+      }
+      else
+      {
+        glib::Object<GMount> mount(g_volume_get_mount(volume_));
+        SetQuirk(QUIRK_VISIBLE, mount);
+      }
+      break;
+    case DevicesSettings::ALWAYS:
+      SetQuirk(QUIRK_VISIBLE, true);
+      break;
+  }
+}
+
 void DeviceLauncherIcon::UpdateDeviceIcon()
 {
   name_ = glib::String(g_volume_get_name(volume_)).Str();
@@ -395,34 +435,6 @@
                NULL);
 }
 
-void DeviceLauncherIcon::UpdateVisibility(int visibility)
-{
-  switch (DevicesSettings::GetDefault().GetDevicesOption())
-  {
-    case DevicesSettings::NEVER:
-      SetQuirk(QUIRK_VISIBLE, false);
-      break;
-    case DevicesSettings::ONLY_MOUNTED:
-      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, visibility);
-      }
-      break;
-    case DevicesSettings::ALWAYS:
-      SetQuirk(QUIRK_VISIBLE, true);
-      break;
-  }
-}
-
 void DeviceLauncherIcon::OnSettingsChanged()
 {
   // Checks if in favourites!

=== modified file 'launcher/DeviceLauncherIcon.h'
--- launcher/DeviceLauncherIcon.h	2012-06-15 02:03:31 +0000
+++ launcher/DeviceLauncherIcon.h	2012-06-20 06:00:31 +0000
@@ -22,6 +22,8 @@
 
 #include <gio/gio.h>
 #include <UnityCore/GLibWrapper.h>
+#include <UnityCore/GLibSignal.h>
+
 #include "SimpleLauncherIcon.h"
 
 namespace unity
@@ -35,17 +37,17 @@
 public:
   DeviceLauncherIcon(glib::Object<GVolume> const& volume);
 
-  void UpdateVisibility(int visibility = -1);
   void OnRemoved();
   bool CanEject();
   void Eject();
 
 protected:
   std::list<DbusmenuMenuitem*> GetMenus();
+  std::string GetName() const;
+
+private:
+  void UpdateVisibility();
   void UpdateDeviceIcon();
-  std::string GetName() const;
-
-private:
   void ActivateLauncherIcon(ActionArg arg);
   void ShowMount(GMount* mount);
   void Unmount();
@@ -59,11 +61,15 @@
   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);
+  void OnVolumeChanged(GVolume* volume);
   void OnSettingsChanged();
   void ShowNotification(std::string const&, unsigned, glib::Object<GdkPixbuf> const&, std::string const&);
 
 private:
+  glib::Signal<void, GVolume*> signal_volume_changed_;
+  glib::Source::UniquePtr changed_timeout_;
   glib::Object<GVolume> volume_;
+
   std::string name_;
   bool keep_in_launcher_;
 };

=== modified file 'launcher/DeviceLauncherSection.cpp'
--- launcher/DeviceLauncherSection.cpp	2012-06-12 06:32:09 +0000
+++ launcher/DeviceLauncherSection.cpp	2012-06-20 06:00:31 +0000
@@ -30,10 +30,6 @@
   sig_manager_.Add(new VolumeSignal(monitor_, "volume-added", sigc::mem_fun(this, &DeviceLauncherSection::OnVolumeAdded)));
   sig_manager_.Add(new VolumeSignal(monitor_, "volume-removed", sigc::mem_fun(this, &DeviceLauncherSection::OnVolumeRemoved)));
 
-  typedef glib::Signal<void, GVolumeMonitor*, GMount*> MountSignal;
-  sig_manager_.Add(new MountSignal(monitor_, "mount-added", sigc::mem_fun(this, &DeviceLauncherSection::OnMountAdded)));
-  sig_manager_.Add(new MountSignal(monitor_, "mount-pre-unmount", sigc::mem_fun(this, &DeviceLauncherSection::OnMountPreUnmount)));
-
   device_populate_idle_.Run([&] () {
     PopulateEntries();
     return false;
@@ -88,31 +84,5 @@
   }
 }
 
-/* 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)
-{
-  glib::Object<GVolume> volume(g_mount_get_volume(mount));
-
-  auto volume_it = map_.find(volume);
-
-  if (volume_it != map_.end())
-    volume_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)
-{
-  glib::Object<GVolume> volume(g_mount_get_volume(mount));
-
-  auto volume_it = map_.find(volume);
-
-  if (volume_it != map_.end())
-    volume_it->second->UpdateVisibility(0);
-}
-
 } // namespace launcher
 } // namespace unity

=== modified file 'launcher/DeviceLauncherSection.h'
--- launcher/DeviceLauncherSection.h	2012-06-12 06:32:09 +0000
+++ launcher/DeviceLauncherSection.h	2012-06-20 06:00:31 +0000
@@ -50,8 +50,6 @@
 
   void OnVolumeAdded(GVolumeMonitor* monitor, GVolume* volume);
   void OnVolumeRemoved(GVolumeMonitor* monitor, GVolume* volume);
-  void OnMountAdded(GVolumeMonitor* monitor, GMount* mount);
-  void OnMountPreUnmount(GVolumeMonitor* monitor, GMount* mount);
 
 private:
   std::map<GVolume*, DeviceLauncherIcon*> map_;

=== modified file 'manual-tests/Launcher.txt'
--- manual-tests/Launcher.txt	2012-06-15 02:03:31 +0000
+++ manual-tests/Launcher.txt	2012-06-20 06:00:31 +0000
@@ -562,3 +562,18 @@
 Expected Results:
  * There are no icons doubled up in the Launcher
  * All icons are present in the Launcher
+
+
+Test launcher removes device icons
+------------------------------------------------------------
+This test makes sure that Unity doesn't shows internal partitions after
+they have been unmounted.
+
+Actions:
+ * Open Nautilus
+ * Mount an internal partition using the sidebar
+ * Make sure the launcher has the launcher icon for the partition
+ * Unmount that partition using Nautilus
+
+Expected Results:
+ * The device launcher icon has been removed.