From 2510bb362557918e4e0df083a5add688462d0494 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Sun, 24 Sep 2023 18:31:39 +0200
Subject: [PATCH] extensions: Stop using run_dispose()

It is considered bad practice, and mainly a lazy way of disconnecting
signal handlers without tracking individual handler IDs.

We can do better by using connectObject(), which provides the same
level of convenience without the dodginess of getting behind the
garbage collector's back.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell-extensions/-/merge_requests/275>
---
 extensions/auto-move-windows/extension.js | 10 ++++------
 extensions/places-menu/placeDisplay.js    | 10 ++++------
 extensions/user-theme/extension.js        |  5 +++--
 extensions/user-theme/prefs.js            |  2 +-
 extensions/window-list/extension.js       | 10 ++++++----
 5 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/extensions/auto-move-windows/extension.js b/extensions/auto-move-windows/extension.js
index 06a75fe1..4f972cd7 100644
--- a/extensions/auto-move-windows/extension.js
+++ b/extensions/auto-move-windows/extension.js
@@ -22,7 +22,8 @@ class WindowMover {
         this._appSystem.connectObject('installed-changed',
             () => this._updateAppData(), this);
 
-        this._settings.connect('changed', this._updateAppConfigs.bind(this));
+        this._settings.connectObject('changed',
+            this._updateAppConfigs.bind(this), this);
         this._updateAppConfigs();
     }
 
@@ -58,11 +59,8 @@ class WindowMover {
 
     destroy() {
         this._appSystem.disconnectObject(this);
-
-        if (this._settings) {
-            this._settings.run_dispose();
-            this._settings = null;
-        }
+        this._settings.disconnectObject(this);
+        this._settings = null;
 
         this._appConfigs.clear();
         this._updateAppData();
diff --git a/extensions/places-menu/placeDisplay.js b/extensions/places-menu/placeDisplay.js
index 07007dfc..930f92c6 100644
--- a/extensions/places-menu/placeDisplay.js
+++ b/extensions/places-menu/placeDisplay.js
@@ -141,8 +141,8 @@ class RootInfo extends PlaceInfo {
                 return;
 
             this._proxy = obj;
-            this._proxy.connect('g-properties-changed',
-                this._propertiesChanged.bind(this));
+            this._proxy.connectObject('g-properties-changed',
+                this._propertiesChanged.bind(this), this);
             this._propertiesChanged(obj);
         });
     }
@@ -161,10 +161,8 @@ class RootInfo extends PlaceInfo {
     }
 
     destroy() {
-        if (this._proxy) {
-            this._proxy.run_dispose();
-            this._proxy = null;
-        }
+        this._proxy?.disconnectObject(this);
+        this._proxy = null;
         super.destroy();
     }
 }
diff --git a/extensions/user-theme/extension.js b/extensions/user-theme/extension.js
index 69a02cf1..fd7a7b43 100644
--- a/extensions/user-theme/extension.js
+++ b/extensions/user-theme/extension.js
@@ -21,12 +21,13 @@ const SETTINGS_KEY = 'name';
 export default class ThemeManager extends Extension {
     enable() {
         this._settings = this.getSettings();
-        this._settings.connect(`changed::${SETTINGS_KEY}`, this._changeTheme.bind(this));
+        this._settings.connectObject(`changed::${SETTINGS_KEY}`,
+            this._changeTheme.bind(this), this);
         this._changeTheme();
     }
 
     disable() {
-        this._settings?.run_dispose();
+        this._settings?.disconnectObject();
         this._settings = null;
 
         Main.setThemeStylesheet(null);
diff --git a/extensions/user-theme/prefs.js b/extensions/user-theme/prefs.js
index b386a2be..9b4cf951 100644
--- a/extensions/user-theme/prefs.js
+++ b/extensions/user-theme/prefs.js
@@ -36,7 +36,7 @@ class UserThemePrefsWidget extends Adw.PreferencesGroup {
         this._actionGroup.add_action(
             this._settings.create_action('name'));
 
-        this.connect('destroy', () => this._settings.run_dispose());
+        this.connect('destroy', () => (this._settings = null));
 
         this._rows = new Map();
         this._addTheme(''); // default
diff --git a/extensions/window-list/extension.js b/extensions/window-list/extension.js
index a071afff..2247dbb8 100644
--- a/extensions/window-list/extension.js
+++ b/extensions/window-list/extension.js
@@ -844,8 +844,8 @@ class WindowList extends St.Widget {
         this._dndWindow = null;
 
         this._settings = settings;
-        this._settings.connect('changed::grouping-mode',
-            () => this._groupingModeChanged());
+        this._settings.connectObject('changed::grouping-mode',
+            () => this._groupingModeChanged(), this);
         this._grouped = undefined;
         this._groupingModeChanged();
     }
@@ -1092,7 +1092,8 @@ class WindowList extends St.Widget {
 
         this._stopMonitoringDrag();
 
-        this._settings.run_dispose();
+        this._settings.disconnectObject();
+        this._settings = null;
 
         let windows = global.get_window_actors();
         for (let i = 0; i < windows.length; i++)
@@ -1144,8 +1145,9 @@ export default class WindowListExtension extends Extension {
         if (!this._windowLists)
             return;
 
-        this._settings.disconnectObject(this);
         Main.layoutManager.disconnectObject(this);
+        this._settings.disconnectObject(this);
+        this._settings = null;
 
         this._windowLists.forEach(windowList => {
             windowList.hide();