From a518c9f57e5fe9c6b5ece5c6cb0534a83f0b2f2d Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 15 Jul 2019 13:52:58 -0400
Subject: [PATCH 1/8] appDisplay: Don't leak duplicate items in AppView
If an icon already exists in an app view with the same id, the
duplicate is not added on a call to addItem. Unfortunately,
since it's not added, the icon actor gets orphaned and leaked.
This commit address the problem by introducing a new hasItem
method and disallowing callers to call addItem with a duplicate
in the first place.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index a07db6573..fa22f47e0 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -143,10 +143,14 @@ class BaseAppView {
return this._allItems;
}
+ hasItem(id) {
+ return this._items[id] !== undefined;
+ }
+
addItem(icon) {
let id = icon.id;
- if (this._items[id] !== undefined)
- return;
+ if (this.hasItem(id))
+ throw new Error(`icon with id ${id} already added to view`)
this._allItems.push(icon);
this._items[id] = icon;
@@ -386,6 +390,8 @@ var AllView = class AllView extends BaseAppView {
let folders = this._folderSettings.get_strv('folder-children');
folders.forEach(id => {
+ if (this.hasItem(id))
+ return;
let path = this._folderSettings.path + 'folders/' + id + '/';
let icon = new FolderIcon(id, path, this);
icon.connect('name-changed', this._itemNameChanged.bind(this));
@@ -1165,7 +1171,10 @@ var FolderIcon = class FolderIcon {
let excludedApps = this._folder.get_strv('excluded-apps');
let appSys = Shell.AppSystem.get_default();
let addAppId = appId => {
- if (excludedApps.indexOf(appId) >= 0)
+ if (this.view.hasItem(appId))
+ return;
+
+ if (excludedApps.includes(appId))
return;
let app = appSys.lookup_app(appId);
--
2.23.0
From 2b6aa9aed98c4854c2ad015879ddcb8d2bf91e9e Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 22 Jul 2019 11:06:30 -0400
Subject: [PATCH 2/8] iconGrid: Clear meta_later callback on destruction
The IconGrid code sometimes sets up a callback to be invoked
later right before being destroyed.
This commit adds a destroy handler to cancel the callback.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/iconGrid.js | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/js/ui/iconGrid.js b/js/ui/iconGrid.js
index d51a443e8..1f05e67f3 100644
--- a/js/ui/iconGrid.js
+++ b/js/ui/iconGrid.js
@@ -210,6 +210,8 @@ var IconGrid = GObject.registerClass({
this.rightPadding = 0;
this.leftPadding = 0;
+ this._updateIconSizesLaterId = 0;
+
this._items = [];
this._clonesAnimating = [];
// Pulled from CSS, but hardcode some defaults here
@@ -227,6 +229,14 @@ var IconGrid = GObject.registerClass({
this.connect('actor-added', this._childAdded.bind(this));
this.connect('actor-removed', this._childRemoved.bind(this));
+ this.connect('destroy', this._onDestroy.bind(this));
+ }
+
+ _onDestroy() {
+ if (this._updateIconSizesLaterId) {
+ Meta.later_remove (this._updateIconSizesLaterId);
+ this._updateIconSizesLaterId = 0;
+ }
}
_keyFocusIn(actor) {
@@ -757,12 +767,14 @@ var IconGrid = GObject.registerClass({
this._updateSpacingForSize(availWidth, availHeight);
}
- Meta.later_add(Meta.LaterType.BEFORE_REDRAW,
- this._updateIconSizes.bind(this));
+ if (!this._updateIconSizesLaterId)
+ this._updateIconSizesLaterId = Meta.later_add(Meta.LaterType.BEFORE_REDRAW,
+ this._updateIconSizes.bind(this));
}
// Note that this is ICON_SIZE as used by BaseIcon, not elsewhere in IconGrid; it's a bit messed up
_updateIconSizes() {
+ this._updateIconSizesLaterId = 0;
let scale = Math.min(this._fixedHItemSize, this._fixedVItemSize) / Math.max(this._hItemSize, this._vItemSize);
let newIconSize = Math.floor(ICON_SIZE * scale);
for (let i in this._items) {
--
2.23.0
From 14a2650548a5104d6a3ec7a1174a23264d79030a Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 22 Jul 2019 11:02:10 -0400
Subject: [PATCH 3/8] appDisplay: Add AppFolderPopup destroy handler
At the moment AppFolderPopup calls popdown on destruction,
which leads to open-state-changed getting emitted after
the actor associated with the popup is destroyed.
This commit handles ungrabbing and closing from an
actor destroy handler to side-step the open-state-changed
signal.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index fa22f47e0..b75d095d5 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1329,6 +1329,15 @@ var AppFolderPopup = class AppFolderPopup {
});
this._grabHelper.addActor(Main.layoutManager.overviewGroup);
this.actor.connect('key-press-event', this._onKeyPress.bind(this));
+ this.actor.connect('destroy', this._onDestroy.bind(this));
+ }
+
+ _onDestroy() {
+ if (this._isOpen) {
+ this._isOpen = false;
+ this._grabHelper.ungrab({ actor: this.actor });
+ this._grabHelper = null;
+ }
}
_onKeyPress(actor, event) {
--
2.23.0
From c9fcb2d23141694ffa2182df20ba75687b01dacc Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Thu, 18 Jul 2019 10:06:38 -0400
Subject: [PATCH 4/8] appDisplay: Clear AllView reference to current popup when
destroyed
AllView contains a reference to the current popup that lingers after
the popup is destroyed.
This commit fixes that, by explicitly nullifying when appropriate.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index b75d095d5..dabf63bfd 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -300,6 +300,7 @@ var AllView = class AllView extends BaseAppView {
this._eventBlocker.add_action(this._clickAction);
this._displayingPopup = false;
+ this._currentPopupDestroyId = 0;
this._availWidth = 0;
this._availHeight = 0;
@@ -589,7 +590,22 @@ var AllView = class AllView extends BaseAppView {
this._stack.add_actor(popup.actor);
popup.connect('open-state-changed', (popup, isOpen) => {
this._eventBlocker.reactive = isOpen;
- this._currentPopup = isOpen ? popup : null;
+
+ if (this._currentPopup) {
+ this._currentPopup.actor.disconnect(this._currentPopupDestroyId);
+ this._currentPopupDestroyId = 0;
+ }
+
+ this._currentPopup = null;
+
+ if (isOpen) {
+ this._currentPopup = popup;
+ this._currentPopupDestroyId = popup.actor.connect('destroy', () => {
+ this._currentPopup = null;
+ this._currentPopupDestroyId = 0;
+ this._eventBlocker.reactive = false;
+ });
+ }
this._updateIconOpacities(isOpen);
if(!isOpen)
this._closeSpaceForPopup();
--
2.23.0
From b7a3fd7fa4527ba9411dcd18debe6ccf88c34dc0 Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Mon, 22 Jul 2019 10:57:57 -0400
Subject: [PATCH 5/8] appDisplay: Add destroy handler for FolderIcon
It is important that the FolderView of a FolderIcon always
gets destroyed before the AppFolderPopup, since the view
may or may not be in the popup, and the view should
get cleaned up exactly once in either case.
This commit adds a destroy handler on FolderIcon to ensure
things get taken down in the right order, and to make sure
the view isn't leaked if it's not yet part of the popup.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index dabf63bfd..5a8f4f1bf 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1156,6 +1156,7 @@ var FolderIcon = class FolderIcon {
this.view.actor.vscroll.adjustment.value = 0;
this._openSpaceForPopup();
});
+ this.actor.connect('destroy', this.onDestroy.bind(this));
this.actor.connect('notify::mapped', () => {
if (!this.actor.mapped && this._popup)
this._popup.popdown();
@@ -1165,6 +1166,13 @@ var FolderIcon = class FolderIcon {
this._redisplay();
}
+ onDestroy() {
+ this.view.actor.destroy();
+
+ if (this._popup)
+ this._popup.actor.destroy();
+ }
+
getAppIds() {
return this.view.getAllItems().map(item => item.id);
}
--
2.23.0
From a90d7a97d21ffa596747cc8ecd0e3f500cb8a77c Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Thu, 18 Jul 2019 14:49:30 -0400
Subject: [PATCH 6/8] appDisplay: Stop watching FolderIcon parent view when
destroyed
When a FolderIcon is opened, it asks the parent view to allocate
space for it, which takes time. Eventually, the space-ready
signal is emitted on the view and the icon can make use of the new
space with its popup. If the icon gets destroyed in the
interim, though, space-ready signal handler still fires.
This commit disconnects the signal handler so it doesn't get called
on a destroyed icon.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index 5a8f4f1bf..062ff222c 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1169,6 +1169,11 @@ var FolderIcon = class FolderIcon {
onDestroy() {
this.view.actor.destroy();
+ if (this._spaceReadySignalId) {
+ this._parentView.disconnect(this._spaceReadySignalId);
+ this._spaceReadySignalId = 0;
+ }
+
if (this._popup)
this._popup.actor.destroy();
}
@@ -1240,8 +1245,9 @@ var FolderIcon = class FolderIcon {
}
_openSpaceForPopup() {
- let id = this._parentView.connect('space-ready', () => {
- this._parentView.disconnect(id);
+ this._spaceReadySignalId = this._parentView.connect('space-ready', () => {
+ this._parentView.disconnect(this._spaceReadySignalId);
+ this._spaceReadySignalId = 0;
this._popup.popup();
this._updatePopupPosition();
});
--
2.23.0
From b57ab33dadf0f31c5bf2c800806593e94784050c Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Thu, 18 Jul 2019 10:19:13 -0400
Subject: [PATCH 7/8] appDisplay: Add open method to FolderIcon
At the moment the only way to open a folder icon is to click on it;
there's no API to open the icon programmatically.
This commits adds an open method and makes the click handler use
it.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index 062ff222c..c0c6e3663 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -1151,11 +1151,7 @@ var FolderIcon = class FolderIcon {
this.view = new FolderView();
- this.actor.connect('clicked', () => {
- this._ensurePopup();
- this.view.actor.vscroll.adjustment.value = 0;
- this._openSpaceForPopup();
- });
+ this.actor.connect('clicked', this.open.bind(this));
this.actor.connect('destroy', this.onDestroy.bind(this));
this.actor.connect('notify::mapped', () => {
if (!this.actor.mapped && this._popup)
@@ -1178,6 +1174,12 @@ var FolderIcon = class FolderIcon {
this._popup.actor.destroy();
}
+ open() {
+ this._ensurePopup();
+ this.view.actor.vscroll.adjustment.value = 0;
+ this._openSpaceForPopup();
+ }
+
getAppIds() {
return this.view.getAllItems().map(item => item.id);
}
--
2.23.0
From baacab7922a56957d041aa59944c419b82e7a7e1 Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Thu, 18 Jul 2019 11:13:27 -0400
Subject: [PATCH 8/8] appDisplay: Keep popup open on refresh
If the list of applications is refreshed we currently close
the open app folder.
This commit adds logic to reopen the app folder on reload.
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/628
---
js/ui/appDisplay.js | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index c0c6e3663..7fad02cd0 100644
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -345,6 +345,21 @@ var AllView = class AllView extends BaseAppView {
super.removeAll();
}
+ _redisplay() {
+ let openFolderId = null;
+ if (this._displayingPopup && this._currentPopup)
+ openFolderId = this._currentPopup._source.id;
+
+ super._redisplay();
+
+ if (openFolderId) {
+ let [folderToReopen] = this.folderIcons.filter(folder => folder.id == openFolderId);
+
+ if (folderToReopen)
+ folderToReopen.open();
+ }
+ }
+
_itemNameChanged(item) {
// If an item's name changed, we can pluck it out of where it's
// supposed to be and reinsert it where it's sorted.
--
2.23.0