From 3033506f2c266115a00ff43daaad14e59e3215c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 5 Dec 2017 22:41:17 +0100 Subject: [PATCH 1/4] 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 634a7d6d7..8483e89fc 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.display.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.display.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.19.0