Launcher: fix GLib.Binding memory leak

src/AppSystem/Launcher.vala

@@ -52,8 +52,6 @@ public class Dock.Launcher : BaseItem {
         }
     }
 
-    private Binding current_count_binding;
-
     private int drag_offset_x = 0;
     private int drag_offset_y = 0;
 
@@ -263,19 +261,7 @@ public class Dock.Launcher : BaseItem {
 
         app.notify["count-visible"].connect (update_badge_revealed);
         update_badge_revealed ();
-        current_count_binding = app.bind_property ("current_count", badge, "label", SYNC_CREATE,
-            (binding, srcval, ref targetval) => {
-                var src = (int64) srcval;
-
-                if (src > 0) {
-                    targetval.set_string ("%lld".printf (src));
-                } else {
-                    targetval.set_string ("!");
-                }
-
-                return true;
-            }, null
-        );
+        app.bind_property ("current_count", badge, "label", SYNC_CREATE, app_badge_count_to_badge_string);
 
         if (notify_settings != null) {
             notify_settings.changed["do-not-disturb"].connect (update_badge_revealed);
@@ -316,7 +302,8 @@ public class Dock.Launcher : BaseItem {
         bounce_down = null;
         bounce_up = null;
         shake = null;
-        current_count_binding.unbind ();
+        badge_fade = null;
+        badge_scale = null;
         remove_dnd_cycle ();
     }
 
@@ -497,4 +484,10 @@ public class Dock.Launcher : BaseItem {
             multiple_windows_open = app.windows.length > 1;
         }
     }
+
+    private static bool app_badge_count_to_badge_string (Binding binding, Value from_value, ref Value to_value) {
+        var src = from_value.get_int64 ();
+        to_value.set_string (src > 0 ? "%lld".printf (src) : "!");
+        return true;
+    }
 }

src/BaseIconGroup.vala

@@ -9,10 +9,12 @@ public abstract class Dock.BaseIconGroup : ContainerItem {
 
     public ListModel icons { get; construct; }
 
+    private Gtk.FlowBox flow_box;
+
     construct {
         var slice = new Gtk.SliceListModel (icons, 0, MAX_N_CHILDREN);
 
-        var flow_box = new Gtk.FlowBox () {
+        flow_box = new Gtk.FlowBox () {
             max_children_per_line = MAX_IN_ROW,
             min_children_per_line = MAX_IN_ROW,
             selection_mode = NONE,
@@ -24,13 +26,19 @@ public abstract class Dock.BaseIconGroup : ContainerItem {
         child = flow_box;
     }
 
+    /**
+    * {@inheritDoc}
+    */
+   public override void cleanup () {
+       base.cleanup ();
+
+       // remove all flowbox children and release references from bindings
+       flow_box.bind_model (null, null);
+   }
+
     private Gtk.Widget create_flow_box_child (Object? item) {
         var image = new Gtk.Image.from_gicon ((Icon) item);
-        bind_property ("icon-size", image, "pixel-size", SYNC_CREATE, (binding, from_value, ref to_value) => {
-            var icon_size = from_value.get_int ();
-            to_value.set_int (get_pixel_size (icon_size));
-            return true;
-        });
+        bind_property ("icon-size", image, "pixel-size", SYNC_CREATE, icon_size_to_pixel_size);
         // We use margin instead of grid spacing because grid spacing in combination with
         // min children per line causes the flow box to request the grid spacing as additional width
         // even when there is only one child making it off center.
@@ -46,6 +54,12 @@ public abstract class Dock.BaseIconGroup : ContainerItem {
         };
     }
 
+    private static bool icon_size_to_pixel_size (Binding binding, Value from_value, ref Value to_value) {
+        var icon_size = from_value.get_int ();
+        to_value.set_int (get_pixel_size (icon_size));
+        return true;
+    }
+
     private static bool icon_size_to_margin (Binding binding, Value from_value, ref Value to_value) {
         var icon_size = from_value.get_int ();
         var pixel_size = get_pixel_size (icon_size);

src/ItemGroup.vala

@@ -6,8 +6,6 @@
  */
 
  public class Dock.ItemGroup : Gtk.Fixed {
-    private const string OBJECT_DATA_KEY = "item-group-obj";
-
     [CCode (has_target = false)]
     public delegate BaseItem CreateBaseItemFunc (Object obj);
 
@@ -23,8 +21,11 @@
 
     private bool relayout_queued = false;
 
-    private HashTable<Object, BaseItem> cached_items;
-    private uint clear_cache_id = 0;
+    private HashTable<Object, BaseItem> obj_to_item_table = new HashTable<Object, BaseItem> (null, null);
+    private HashTable<BaseItem, Object> item_to_obj_table = new HashTable<BaseItem, Object> (null, null);
+
+    private GenericSet<BaseItem> items_marked_for_removal = new GenericSet<BaseItem> (null, null);
+    private uint remove_items_id = 0;
 
     public ItemGroup (ListModel items, CreateBaseItemFunc create_item_func) {
         Object (items: items, create_item_func: create_item_func);
@@ -51,8 +52,6 @@
         on_items_changed (0, 0, items.get_n_items ());
 
         overflow = VISIBLE;
-
-        cached_items = new HashTable<Object, BaseItem> (null, null);
     }
 
     private void queue_relayout () {
@@ -101,41 +100,64 @@
     private void on_items_changed (uint position, uint removed, uint added) {
         var start_iter = item_store.get_iter_at_pos ((int) position);
         var end_iter = start_iter.move ((int) removed);
-        start_iter.foreach_range (end_iter, cache_item);
-        start_iter.foreach_range (end_iter, remove_item);
+        start_iter.foreach_range (end_iter, mark_item_for_removal);
         start_iter.remove_range (end_iter);
 
         var insert_iter = item_store.get_iter_at_pos ((int) position);
         for (int i = (int) position; i < position + added; i++) {
             var item = get_or_create_item (items.get_item (i));
             insert_iter.insert_before (item);
 
+            unmark_item_for_removal (item);
             add_item (i, item);
         }
     }
 
-    // Make sure that if an item is removed and added again in the same
-    // mainloop iteration it gets mapped to the same BaseItem
-    private void cache_item (BaseItem item) {
-        cached_items[item.get_data<Object> (OBJECT_DATA_KEY)] = item;
+    /*
+     * During drag-and-drop we get 2 separate item_changed signals:
+     * 1 for removing the dragged item, and 1 for adding it back into new place.
+     * To avoid removing this item, we remove it in Idle.
+     */
+    private void mark_item_for_removal (BaseItem item) {
+        items_marked_for_removal.add (item);
+
+        item.revealed_done.connect (remove_item);
+        item.set_revealed (false);
+
+        if (remove_items_id == 0) {
+            remove_items_id = Idle.add_once (remove_pending_items);
+        }
+    }
+
+    private void remove_pending_items () {
+        remove_items_id = 0;
+
+        foreach (var item in items_marked_for_removal.get_values ()) {
+            item.cleanup ();
+            items_marked_for_removal.remove (item);
 
-        if (clear_cache_id == 0) {
-            clear_cache_id = Idle.add_once (clear_cache);
+            var obj = item_to_obj_table[item];
+            obj_to_item_table.remove (obj);
+            item_to_obj_table.remove (item);
         }
     }
 
-    private void clear_cache () {
-        cached_items.remove_all ();
-        clear_cache_id = 0;
+    private void unmark_item_for_removal (BaseItem item) {
+        items_marked_for_removal.remove (item);
+
+        item.revealed_done.disconnect (remove_item);
+        item.set_revealed (true);
+
     }
 
     private BaseItem get_or_create_item (Object obj) {
-        if (obj in cached_items) {
-            return cached_items[obj];
+        if (obj in obj_to_item_table) {
+            return obj_to_item_table[obj];
         }
 
         var base_item = create_item_func (obj);
-        base_item.set_data<Object> (OBJECT_DATA_KEY, obj);
+        obj_to_item_table[obj] = base_item;
+        item_to_obj_table[base_item] = obj;
         return base_item;
     }
 
@@ -144,7 +166,7 @@
             // The item was already in this group and is currently being removed
             // so immediately finish the removal and add it as if it was new
             // This happens when the items are repositioned via dnd
-            finish_remove (item);
+            remove_item (item);
         }
 
         item.visible = false;
@@ -157,14 +179,7 @@
     }
 
     private void remove_item (BaseItem item) {
-        item.revealed_done.connect (finish_remove);
-        item.set_revealed (false);
-
-        cached_items[item.get_data<Object> ("dock-obj")] = item;
-    }
-
-    private void finish_remove (BaseItem item) {
-        item.revealed_done.disconnect (finish_remove);
+        item.revealed_done.disconnect (remove_item);
 
         remove (item);
 

src/WorkspaceSystem/WorkspaceIconGroup.vala

@@ -13,9 +13,7 @@ public class Dock.WorkspaceIconGroup : BaseIconGroup, WorkspaceItem {
     public WorkspaceIconGroup (Workspace workspace) {
         var additional_icons = new GLib.ListStore (typeof (GLib.Icon));
 
-        var workspace_icons = new Gtk.MapListModel (workspace.windows, (window) => {
-            return ((Window) window).icon;
-        });
+        var workspace_icons = new Gtk.MapListModel (workspace.windows, (window) => ((Window) window).icon);
 
         var icon_sources_list_store = new GLib.ListStore (typeof (GLib.ListModel));
         icon_sources_list_store.append (additional_icons);
@@ -32,11 +30,7 @@ public class Dock.WorkspaceIconGroup : BaseIconGroup, WorkspaceItem {
     }
 
     construct {
-        workspace.bind_property ("is-active-workspace", this, "state", SYNC_CREATE, (binding, from_value, ref to_value) => {
-            var new_val = from_value.get_boolean () ? State.ACTIVE : State.HIDDEN;
-            to_value.set_enum (new_val);
-            return true;
-        });
+        workspace.bind_property ("is-active-workspace", this, "state", SYNC_CREATE, is_active_workspace_to_state);
 
         gesture_click.button = Gdk.BUTTON_PRIMARY;
         gesture_click.released.connect (workspace.activate);
@@ -55,4 +49,10 @@ public class Dock.WorkspaceIconGroup : BaseIconGroup, WorkspaceItem {
         additional_icons.remove_all ();
         unset_state_flags (DROP_ACTIVE);
     }
+
+    private static bool is_active_workspace_to_state (Binding binding, Value from_value, ref Value to_value) {
+        var new_val = from_value.get_boolean () ? State.ACTIVE : State.HIDDEN;
+        to_value.set_enum (new_val);
+        return true;
+    }
 }