[glib/wip/Jehan/issue-2096-SHGetFileInfoW: 2/2] Issue #2096: SHGetFileInfoW() is not reliable (time-wise).



commit 8e30e3e83278d01b0c3d2b2b0ee503d7bc9383d5
Author: Jehan <jehan girinstud io>
Date:   Mon Apr 27 22:42:02 2020 +0200

    Issue #2096: SHGetFileInfoW() is not reliable (time-wise).
    
    For the same reasons as the previous commit, g_mount_get_icon() is
    modified to not run SHGetFileInfoW() directly but in a thread, while
    returning valid defaults at first, rather than specific icon as given by
    the Windows API. Once the icon is actually modified, the "changed"
    signal will be emitted.

 gio/gmount.c      | 10 ++++++-
 gio/gwin32mount.c | 80 +++++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 69 insertions(+), 21 deletions(-)
---
diff --git a/gio/gmount.c b/gio/gmount.c
index 667f9ce7d..b33d8fa83 100644
--- a/gio/gmount.c
+++ b/gio/gmount.c
@@ -206,7 +206,15 @@ g_mount_get_name (GMount *mount)
  * @mount: a #GMount.
  * 
  * Gets the icon for @mount.
- * 
+ *
+ * Note that on some system (Windows at least), the returned value may not be
+ * the most specific icon, but calling g_mount_get_icon() will still return an
+ * acceptable generic icon and trigger specific icon request in a thread (it
+ * can be very slow in some cases, such as unresponsive or network mounts). If
+ * you care about displaying the most adapted icon, you may want to initially
+ * use the returned icon, then connect to the "changed" signal to later update
+ * code where it is used.
+ *
  * Returns: (transfer full): a #GIcon.
  *      The returned object should be unreffed with 
  *      g_object_unref() when no longer needed.
diff --git a/gio/gwin32mount.c b/gio/gwin32mount.c
index 4d50f4ded..e0427d80a 100644
--- a/gio/gwin32mount.c
+++ b/gio/gwin32mount.c
@@ -79,10 +79,8 @@ g_win32_mount_finalize (GObject *object)
 #endif
   /* TODO: g_warn_if_fail (volume->volume == NULL); */
 
-  if (mount->icon != NULL)
-    g_object_unref (mount->icon);
-  if (mount->symbolic_icon != NULL)
-    g_object_unref (mount->symbolic_icon);
+  g_clear_object (&mount->icon);
+  g_clear_object (&mount->symbolic_icon);
 
   g_free (mount->name);
   g_free (mount->mount_path);
@@ -227,31 +225,73 @@ _win32_drive_type_to_icon (int type, gboolean use_symbolic)
   }
 }
 
+static void
+_win32_get_icon (GTask        *task,
+                 gpointer      source_object,
+                 gpointer      task_data,
+                 GCancellable *cancellable)
+{
+  const char  *drive = task_data;
+  gunichar2   *wdrive = g_utf8_to_utf16 (drive, -1, NULL, NULL, NULL);
+  GIcon       *icon = NULL;
+  SHFILEINFOW  sfi;
+
+  if (SHGetFileInfoW (wdrive, 0, &sfi, sizeof (sfi), SHGFI_ICONLOCATION))
+    {
+      gchar *name = g_utf16_to_utf8 (sfi.szDisplayName, -1, NULL, NULL, NULL);
+      gchar *id   = g_strdup_printf ("%s,%i", name, sfi.iIcon);
+
+      icon = g_themed_icon_new (id);
+      g_free (name);
+      g_free (id);
+    }
+  g_free (wdrive);
+
+  g_task_return_pointer (task, icon, g_object_unref);
+}
+
+static void
+_win32_get_icon_finish (GObject      *source_object,
+                        GAsyncResult *res,
+                        gpointer      user_data)
+{
+  GWin32Mount *mount = G_WIN32_MOUNT (source_object);
+  GIcon       *icon  = g_task_propagate_pointer (G_TASK (res), NULL);
+
+  if (icon)
+    {
+      g_clear_object (&mount->icon);
+      mount->icon = icon;
+      g_signal_emit_by_name (mount, "changed");
+    }
+}
+
 static GIcon *
 g_win32_mount_get_icon (GMount *mount)
 {
   GWin32Mount *win32_mount = G_WIN32_MOUNT (mount);
+  GIcon       *icon        = win32_mount->icon;
 
   g_return_val_if_fail (win32_mount->mount_path != NULL, NULL);
 
   /* lazy creation */
-  if (!win32_mount->icon)
+  if (! icon)
     {
-      SHFILEINFOW shfi;
-      wchar_t *wfn = g_utf8_to_utf16 (win32_mount->mount_path, -1, NULL, NULL, NULL);
-
-      if (SHGetFileInfoW (wfn, 0, &shfi, sizeof (shfi), SHGFI_ICONLOCATION))
-        {
-         gchar *name = g_utf16_to_utf8 (shfi.szDisplayName, -1, NULL, NULL, NULL);
-         gchar *id = g_strdup_printf ("%s,%i", name, shfi.iIcon);
-         win32_mount->icon = g_themed_icon_new (id);
-         g_free (name);
-         g_free (id);
-       }
-      else
-        {
-          win32_mount->icon = g_themed_icon_new_with_default_fallbacks (_win32_drive_type_to_icon 
(win32_mount->drive_type, FALSE));
-       }
+      GTask *task;
+
+      /* On Windows, this request can be expensive, especially with remote
+       * mounts, or worse unresponsive mounts.
+       * So only request the icon when the info is needed and run it in
+       * a thread. Caller is advised to monitor "changed" signal.
+       */
+      task = g_task_new (mount, NULL, _win32_get_icon_finish, NULL);
+      g_task_set_priority (task, G_PRIORITY_LOW);
+      g_task_set_source_tag (task, g_win32_mount_get_icon);
+      g_task_set_task_data (task, g_strdup (win32_mount->mount_path), g_free);
+      g_task_run_in_thread (task, _win32_get_icon);
+      g_object_unref (task);
+
+      icon = g_themed_icon_new_with_default_fallbacks (_win32_drive_type_to_icon (win32_mount->drive_type, 
FALSE));
     }
 
   return g_object_ref (win32_mount->icon);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]