From 1c838205dcd99a0a2a901f7449197da3df7b3954 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Date: Tue, 5 Dec 2017 22:41:17 +0100

Subject: [PATCH 1/6] dnd: Nullify _dragActor after we've destroyed it, and

 avoid invalid access

We need to avoid that we use the _dragActor instance after that it has

been destroyed or we'll get errors. We now set it to null when this

happens, protecting any access to that.

Add a DragState enum-like object to keep track of the state

instead of using booleans.

Remove duplicated handler on 'destroy' and just use a generic one.

https://bugzilla.gnome.org/show_bug.cgi?id=791233

---

 js/ui/dnd.js | 65 +++++++++++++++++++++++++++++++---------------------

 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/js/ui/dnd.js b/js/ui/dnd.js

index a38607c24..431c60d6c 100644

--- a/js/ui/dnd.js

+++ b/js/ui/dnd.js

@@ -27,6 +27,12 @@ var DragMotionResult = {

     CONTINUE:  3

 };

+var DragState = {

+    INIT:      0,

+    DRAGGING:  1,

+    CANCELLED: 2,

+};

+

 var DRAG_CURSOR_MAP = {

     0: Meta.Cursor.DND_UNSUPPORTED_TARGET,

     1: Meta.Cursor.DND_COPY,

@@ -78,6 +84,8 @@ var _Draggable = new Lang.Class({

                                         dragActorOpacity: undefined });

         this.actor = actor;

+        this._dragState = DragState.INIT;

+

         if (!params.manualMode) {

             this.actor.connect('button-press-event',

                                this._onButtonPress.bind(this));

@@ -88,7 +96,7 @@ var _Draggable = new Lang.Class({

         this.actor.connect('destroy', () => {

             this._actorDestroyed = true;

-            if (this._dragInProgress && this._dragCancellable)

+            if (this._dragState == DragState.DRAGGING && this._dragCancellable)

                 this._cancelDrag(global.get_current_time());

             this.disconnectAll();

         });

@@ -100,7 +108,6 @@ var _Draggable = new Lang.Class({

         this._dragActorOpacity = params.dragActorOpacity;

         this._buttonDown = false; // The mouse button has been pressed and has not yet been released.

-        this._dragInProgress = false; // The drag has been started, and has not been dropped or cancelled yet.

         this._animationInProgress = false; // The drag is over and the item is in the process of animating to its original position (snapping back or reverting).

         this._dragCancellable = true;

@@ -206,9 +213,10 @@ var _Draggable = new Lang.Class({

             (event.type() == Clutter.EventType.TOUCH_END &&

              global.display.is_pointer_emulating_sequence(event.get_event_sequence()))) {

             this._buttonDown = false;

-            if (this._dragInProgress) {

+            if (this._dragState == DragState.DRAGGING) {

                 return this._dragActorDropped(event);

-            } else if (this._dragActor != null && !this._animationInProgress) {

+            } else if ((this._dragActor != null || this._dragState == DragState.CANCELLED) &&

+                       !this._animationInProgress) {

                 // Drag must have been cancelled with Esc.

                 this._dragComplete();

                 return Clutter.EVENT_STOP;

@@ -222,14 +230,14 @@ var _Draggable = new Lang.Class({

         } else if (event.type() == Clutter.EventType.MOTION ||

                    (event.type() == Clutter.EventType.TOUCH_UPDATE &&

                     global.display.is_pointer_emulating_sequence(event.get_event_sequence()))) {

-            if (this._dragInProgress) {

+            if (this._dragActor && this._dragState == DragState.DRAGGING) {

                 return this._updateDragPosition(event);

-            } else if (this._dragActor == null) {

+            } else if (this._dragActor == null && this._dragState != DragState.CANCELLED) {

                 return this._maybeStartDrag(event);

             }

         // We intercept KEY_PRESS event so that we can process Esc key press to cancel

         // dragging and ignore all other key presses.

-        } else if (event.type() == Clutter.EventType.KEY_PRESS && this._dragInProgress) {

+        } else if (event.type() == Clutter.EventType.KEY_PRESS && this._dragState == DragState.DRAGGING) {

             let symbol = event.get_key_symbol();

             if (symbol == Clutter.Escape) {

                 this._cancelDrag(event.get_time());

@@ -265,7 +273,7 @@ var _Draggable = new Lang.Class({

      */

     startDrag(stageX, stageY, time, sequence) {

         currentDraggable = this;

-        this._dragInProgress = true;

+        this._dragState = DragState.DRAGGING;

         // Special-case St.Button: the pointer grab messes with the internal

         // state, so force a reset to a reasonable state here

@@ -342,6 +350,13 @@ var _Draggable = new Lang.Class({

             Shell.util_set_hidden_from_pick(this._dragActor, true);

         }

+        this._dragActorDestroyId = this._dragActor.connect('destroy', () => {

+            // Cancel ongoing animation (if any)

+            this._finishAnimation();

+

+            this._dragActor = null;

+            this._dragState = DragState.CANCELLED;

+        });

         this._dragOrigOpacity = this._dragActor.opacity;

         if (this._dragActorOpacity != undefined)

             this._dragActor.opacity = this._dragActorOpacity;

@@ -500,7 +515,7 @@ var _Draggable = new Lang.Class({

                                                 event.get_time())) {

                     // If it accepted the drop without taking the actor,

                     // handle it ourselves.

-                    if (this._dragActor.get_parent() == Main.uiGroup) {

+                    if (this._dragActor && this._dragActor.get_parent() == Main.uiGroup) {

                         if (this._restoreOnSuccess) {

                             this._restoreDragActor(event.get_time());

                             return true;

@@ -508,7 +523,7 @@ var _Draggable = new Lang.Class({

                             this._dragActor.destroy();

                     }

-                    this._dragInProgress = false;

+                    this._dragState = DragState.INIT;

                     global.screen.set_cursor(Meta.Cursor.DEFAULT);

                     this.emit('drag-end', event.get_time(), true);

                     this._dragComplete();

@@ -557,20 +572,22 @@ var _Draggable = new Lang.Class({

     _cancelDrag(eventTime) {

         this.emit('drag-cancelled', eventTime);

-        this._dragInProgress = false;

-        let [snapBackX, snapBackY, snapBackScale] = this._getRestoreLocation();

+        let wasCancelled = (this._dragState == DragState.CANCELLED);

+        this._dragState = DragState.CANCELLED;

-        if (this._actorDestroyed) {

+        if (this._actorDestroyed || wasCancelled) {

             global.screen.set_cursor(Meta.Cursor.DEFAULT);

             if (!this._buttonDown)

                 this._dragComplete();

             this.emit('drag-end', eventTime, false);

-            if (!this._dragOrigParent)

+            if (!this._dragOrigParent && this._dragActor)

                 this._dragActor.destroy();

             return;

         }

+        let [snapBackX, snapBackY, snapBackScale] = this._getRestoreLocation();

+

         this._animateDragEnd(eventTime,

                              { x: snapBackX,

                                y: snapBackY,

@@ -581,7 +598,7 @@ var _Draggable = new Lang.Class({

     },

     _restoreDragActor(eventTime) {

-        this._dragInProgress = false;

+        this._dragState = DragState.INIT;

         let [restoreX, restoreY, restoreScale] = this._getRestoreLocation();

         // fade the actor back in at its original location

@@ -596,12 +613,6 @@ var _Draggable = new Lang.Class({

     _animateDragEnd(eventTime, params) {

         this._animationInProgress = true;

-        // finish animation if the actor gets destroyed

-        // during it

-        this._dragActorDestroyId =

-            this._dragActor.connect('destroy',

-                                    this._finishAnimation.bind(this));

-

         params['opacity']          = this._dragOrigOpacity;

         params['transition']       = 'easeOutQuad';

         params['onComplete']       = this._onAnimationComplete;

@@ -624,9 +635,6 @@ var _Draggable = new Lang.Class({

     },

     _onAnimationComplete(dragActor, eventTime) {

-        dragActor.disconnect(this._dragActorDestroyId);

-        this._dragActorDestroyId = 0;

-

         if (this._dragOrigParent) {

             Main.uiGroup.remove_child(this._dragActor);

             this._dragOrigParent.add_actor(this._dragActor);

@@ -641,7 +649,7 @@ var _Draggable = new Lang.Class({

     },

     _dragComplete() {

-        if (!this._actorDestroyed)

+        if (!this._actorDestroyed && this._dragActor)

             Shell.util_set_hidden_from_pick(this._dragActor, false);

         this._ungrabEvents();

@@ -652,7 +660,12 @@ var _Draggable = new Lang.Class({

             this._updateHoverId = 0;

         }

-        this._dragActor = undefined;

+        if (this._dragActor) {

+            this._dragActor.disconnect(this._dragActorDestroyId);

+            this._dragActor = null;

+        }

+

+        this._dragState = DragState.INIT;

         currentDraggable = null;

     }

 });

-- 

2.20.1

From 98fd633f3b124f72f71aa5da38df9c69121fd4ed Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Jonas=20=C3=85dahl?= <jadahl@gmail.com>

Date: Thu, 3 Jan 2019 11:53:13 +0100

Subject: [PATCH 2/6] dnd: Repick target actor if destroyed mid iteration

The picked target actor may be destroyed (e.g. hover style change

resulting in the ClutterTexture to be destroyed). If we don't handle

this, GJS will abort when it sees the exception caused by Javascript

code trying to access the destroyed target actor.

To handle it, listen on the 'destroy' signal on the target actor, and

repick, so a valid actor is passed to the next motion callback.

Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/issues/632

---

 js/ui/dnd.js | 22 ++++++++++++++++++++--

 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/js/ui/dnd.js b/js/ui/dnd.js

index 431c60d6c..9e961a186 100644

--- a/js/ui/dnd.js

+++ b/js/ui/dnd.js

@@ -411,10 +411,15 @@ var _Draggable = new Lang.Class({

         return true;

     },

+    _pickTargetActor() {

+        return this._dragActor.get_stage().get_actor_at_pos(Clutter.PickMode.ALL,

+                                                            this._dragX, this._dragY);

+    },

+

     _updateDragHover() {

         this._updateHoverId = 0;

-        let target = this._dragActor.get_stage().get_actor_at_pos(Clutter.PickMode.ALL,

-                                                                  this._dragX, this._dragY);

+        let target = this._pickTargetActor();

+

         let dragEvent = {

             x: this._dragX,

             y: this._dragY,

@@ -422,6 +427,18 @@ var _Draggable = new Lang.Class({

             source: this.actor._delegate,

             targetActor: target

         };

+

+        let targetActorDestroyHandlerId;

+        let handleTargetActorDestroyClosure;

+        handleTargetActorDestroyClosure = () => {

+            target = this._pickTargetActor();

+            dragEvent.targetActor = target;

+            targetActorDestroyHandlerId =

+                target.connect('destroy', handleTargetActorDestroyClosure);

+        };

+        targetActorDestroyHandlerId =

+            target.connect('destroy', handleTargetActorDestroyClosure);

+

         for (let i = 0; i < dragMonitors.length; i++) {

             let motionFunc = dragMonitors[i].dragMotion;

             if (motionFunc) {

@@ -432,6 +449,7 @@ var _Draggable = new Lang.Class({

                 }

             }

         }

+        dragEvent.targetActor.disconnect(targetActorDestroyHandlerId);

         while (target) {

             if (target._delegate && target._delegate.handleDragOver) {

-- 

2.20.1

From 2dfae7213f0d3849dfcc38bb2fbaeaf80d4e8b19 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Date: Wed, 4 Jul 2018 15:56:25 +0200

Subject: [PATCH 3/6] messageList: stop syncing if closeButton has been

 destroyed

The _sync function for Message only updates the close button visibility,

so we can safely stop doing that if the close button get get destroyed earlier

(as it happens when clicking on it).

https://bugzilla.gnome.org/show_bug.cgi?id=791233

---

 js/ui/messageList.js | 3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/js/ui/messageList.js b/js/ui/messageList.js

index aff201ed6..2d397c1d7 100644

--- a/js/ui/messageList.js

+++ b/js/ui/messageList.js

@@ -362,7 +362,8 @@ var Message = new Lang.Class({

         this.setBody(body);

         this._closeButton.connect('clicked', this.close.bind(this));

-        this.actor.connect('notify::hover', this._sync.bind(this));

+        let actorHoverId = this.actor.connect('notify::hover', this._sync.bind(this));

+        this._closeButton.connect('destroy', this.actor.disconnect.bind(this.actor, actorHoverId));

         this.actor.connect('clicked', this._onClicked.bind(this));

         this.actor.connect('destroy', this._onDestroy.bind(this));

         this._sync();

-- 

2.20.1

From 41bde330384ba7021c96ea3d4575bfa12295dff1 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Date: Wed, 4 Jul 2018 16:55:28 +0200

Subject: [PATCH 4/6] automountManager: remove allowAutorun expire timeout on

 volume removal

If the volume is removed before AUTORUN_EXPIRE_TIMEOUT_SECS seconds, we can stop

the timeout earlier as there's nothing to unset, while the volume instance

won't be valid anymore.

https://bugzilla.gnome.org/show_bug.cgi?id=791233

---

 js/ui/components/automountManager.js | 6 ++++++

 1 file changed, 6 insertions(+)

diff --git a/js/ui/components/automountManager.js b/js/ui/components/automountManager.js

index 2d8f3f8fb..a6cd85792 100644

--- a/js/ui/components/automountManager.js

+++ b/js/ui/components/automountManager.js

@@ -210,6 +210,10 @@ var AutomountManager = new Lang.Class({

     },

     _onVolumeRemoved(monitor, volume) {

+        if (volume._allowAutorunExpireId && volume._allowAutorunExpireId > 0) {

+            Mainloop.source_remove(volume._allowAutorunExpireId);

+            delete volume._allowAutorunExpireId;

+        }

         this._volumeQueue = 

             this._volumeQueue.filter(element => (element != volume));

     },

@@ -234,8 +238,10 @@ var AutomountManager = new Lang.Class({

     _allowAutorunExpire(volume) {

         let id = Mainloop.timeout_add_seconds(AUTORUN_EXPIRE_TIMEOUT_SECS, () => {

             volume.allowAutorun = false;

+            delete volume._allowAutorunExpireId;

             return GLib.SOURCE_REMOVE;

         });

+        volume._allowAutorunExpireId = id;

         GLib.Source.set_name_by_id(id, '[gnome-shell] volume.allowAutorun');

     }

 });

-- 

2.20.1

From f0d608cea903538683b2eaafadbdefe7ff41475e Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>

Date: Mon, 9 Jul 2018 13:31:26 +0200

Subject: [PATCH 5/6] calendar: chain up to parent on _onDestroy

---

 js/ui/calendar.js | 2 ++

 1 file changed, 2 insertions(+)

diff --git a/js/ui/calendar.js b/js/ui/calendar.js

index a46017ad0..65133d9f7 100644

--- a/js/ui/calendar.js

+++ b/js/ui/calendar.js

@@ -802,6 +802,8 @@ var NotificationMessage = new Lang.Class({

     },

     _onDestroy() {

+        this.parent();

+

         if (this._updatedId)

             this.notification.disconnect(this._updatedId);

         this._updatedId = 0;

-- 

2.20.1

From dc75a4e785e5f19f61bfd8f58e079c0bdfd0fca8 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>

Date: Tue, 5 Dec 2017 02:41:50 +0100

Subject: [PATCH 6/6] tweener: Save handlers on target and remove them on

 destroy

Saving handlers we had using the wrapper as a property of the object and delete

them when resetting the object state.

Without doing this an handler could be called on a destroyed target when this

happens on the onComplete callback.

https://bugzilla.gnome.org/show_bug.cgi?id=791233

---

 js/ui/tweener.js | 63 ++++++++++++++++++++++++++++++++++++++----------

 1 file changed, 50 insertions(+), 13 deletions(-)

diff --git a/js/ui/tweener.js b/js/ui/tweener.js

index 1a85e2fb1..22818ba4b 100644

--- a/js/ui/tweener.js

+++ b/js/ui/tweener.js

@@ -69,30 +69,67 @@ function _getTweenState(target) {

     return target.__ShellTweenerState;

 }

+function _ensureHandlers(target) {

+    if (!target.__ShellTweenerHandlers)

+        target.__ShellTweenerHandlers = {};

+    return target.__ShellTweenerHandlers;

+}

+

 function _resetTweenState(target) {

     let state = target.__ShellTweenerState;

     if (state) {

-        if (state.destroyedId)

+        if (state.destroyedId) {

             state.actor.disconnect(state.destroyedId);

+            delete state.destroyedId;

+        }

     }

+    _removeHandler(target, 'onComplete', _tweenCompleted);

     target.__ShellTweenerState = {};

 }

 function _addHandler(target, params, name, handler) {

-    if (params[name]) {

-        let oldHandler = params[name];

-        let oldScope = params[name + 'Scope'];

-        let oldParams = params[name + 'Params'];

-        let eventScope = oldScope ? oldScope : target;

-

-        params[name] = () => {

-            oldHandler.apply(eventScope, oldParams);

-            handler(target);

-        };

-    } else

-        params[name] = () => { handler(target); };

+    let wrapperNeeded = false;

+    let tweenerHandlers = _ensureHandlers(target);

+

+    if (!(name in tweenerHandlers)) {

+        tweenerHandlers[name] = [];

+        wrapperNeeded = true;

+    }

+

+    let handlers = tweenerHandlers[name];

+    handlers.push(handler);

+

+    if (wrapperNeeded) {

+        if (params[name]) {

+            let oldHandler = params[name];

+            let oldScope = params[name + 'Scope'];

+            let oldParams = params[name + 'Params'];

+            let eventScope = oldScope ? oldScope : target;

+

+            params[name] = () => {

+                oldHandler.apply(eventScope, oldParams);

+                handlers.forEach((h) => h(target));

+            };

+        } else {

+            params[name] = () => { handlers.forEach((h) => h(target)); };

+        }

+    }

+}

+

+function _removeHandler(target, name, handler) {

+    let tweenerHandlers = _ensureHandlers(target);

+

+    if (name in tweenerHandlers) {

+        let handlers = tweenerHandlers[name];

+        let handlerIndex = handlers.indexOf(handler);

+

+        while (handlerIndex > -1) {

+            handlers.splice(handlerIndex, 1);

+            handlerIndex = handlers.indexOf(handler);

+        }

+    }

 }

 function _actorDestroyed(target) {

-- 

2.20.1