From fc4f9f61fa6ffc1bf284339bb387a0ab0fe98cea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Sun, 6 Mar 2022 00:30:16 +0100
Subject: [PATCH] signalTracker: Explicitly register destroyable types

We currently assume that any '::destroy' signal on a GObject type
has the semantics of the ClutterActor/GtkWidget signal, and should
therefore result in all signals being disconnected.

But we already have a case where the assumption doesn't hold: ShellWM
uses '::destroy' for the closing animation of windows, and the ShellWM
object itself remains very valid after the emission.

So rather than making assumptions about '::destroy', check objects
against a list of destroyable types that are explicitly registered
as such.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2226>
---
 js/misc/signalTracker.js    | 22 ++++++++++++++++++++--
 js/ui/environment.js        |  2 ++
 js/ui/messageTray.js        |  3 +++
 tests/unit/signalTracker.js |  3 ++-
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/js/misc/signalTracker.js b/js/misc/signalTracker.js
index 3e83be734..bcfa72de9 100644
--- a/js/misc/signalTracker.js
+++ b/js/misc/signalTracker.js
@@ -1,14 +1,15 @@
 /* exported TransientSignalHolder, addObjectSignalMethods */
 const { GObject } = imports.gi;
 
+const destroyableTypes = [];
+
 /**
  * @private
  * @param {Object} obj - an object
  * @returns {bool} - true if obj has a 'destroy' GObject signal
  */
 function _hasDestroySignal(obj) {
-    return obj instanceof GObject.Object &&
-        GObject.signal_lookup('destroy', obj);
+    return destroyableTypes.some(type => obj instanceof type);
 }
 
 var TransientSignalHolder = GObject.registerClass(
@@ -28,6 +29,7 @@ class TransientSignalHolder extends GObject.Object {
         this.emit('destroy');
     }
 });
+registerDestroyableType(TransientSignalHolder);
 
 class SignalManager {
     /**
@@ -230,3 +232,19 @@ function addObjectSignalMethods(proto) {
     };
     proto['disconnect_object'] = proto['disconnectObject'];
 }
+
+/**
+ * Register a GObject type as having a 'destroy' signal
+ * that should disconnect all handlers
+ *
+ * @param {GObject.Type} gtype - a GObject type
+ */
+function registerDestroyableType(gtype) {
+    if (!GObject.type_is_a(gtype, GObject.Object))
+        throw new Error(`${gtype} is not a GObject subclass`);
+
+    if (!GObject.signal_lookup('destroy', gtype))
+        throw new Error(`${gtype} does not have a destroy signal`);
+
+    destroyableTypes.push(gtype);
+}
diff --git a/js/ui/environment.js b/js/ui/environment.js
index 2d3baf722..7bd848f1a 100644
--- a/js/ui/environment.js
+++ b/js/ui/environment.js
@@ -345,6 +345,8 @@ function init() {
         SignalTracker.addObjectSignalMethods(prototype);
     };
 
+    SignalTracker.registerDestroyableType(Clutter.Actor);
+
     // Miscellaneous monkeypatching
     _patchContainerClass(St.BoxLayout);
 
diff --git a/js/ui/messageTray.js b/js/ui/messageTray.js
index 862ade6ef..e001f2202 100644
--- a/js/ui/messageTray.js
+++ b/js/ui/messageTray.js
@@ -10,6 +10,7 @@ const GnomeSession = imports.misc.gnomeSession;
 const Layout = imports.ui.layout;
 const Main = imports.ui.main;
 const Params = imports.misc.params;
+const SignalTracker = imports.misc.signalTracker;
 
 const SHELL_KEYBINDINGS_SCHEMA = 'org.gnome.shell.keybindings';
 
@@ -493,6 +494,7 @@ var Notification = GObject.registerClass({
         this.run_dispose();
     }
 });
+SignalTracker.registerDestroyableType(Notification);
 
 var NotificationBanner = GObject.registerClass({
     Signals: {
@@ -795,6 +797,7 @@ var Source = GObject.registerClass({
         }
     }
 });
+SignalTracker.registerDestroyableType(Source);
 
 var MessageTray = GObject.registerClass({
     Signals: {
diff --git a/tests/unit/signalTracker.js b/tests/unit/signalTracker.js
index 40398c0b8..a6f50e257 100644
--- a/tests/unit/signalTracker.js
+++ b/tests/unit/signalTracker.js
@@ -8,13 +8,14 @@ const JsUnit = imports.jsUnit;
 const Signals = imports.signals;
 
 const Environment = imports.ui.environment;
-const { TransientSignalHolder } = imports.misc.signalTracker;
+const { TransientSignalHolder, registerDestroyableType } = imports.misc.signalTracker;
 
 Environment.init();
 
 const Destroyable = GObject.registerClass({
     Signals: { 'destroy': {} },
 }, class Destroyable extends GObject.Object {});
+registerDestroyableType(Destroyable);
 
 class PlainEmitter {}
 Signals.addSignalMethods(PlainEmitter.prototype);