js: Use (dis)connectObject()
Start using the new methods to simplify signal cleanup. For now, focus on replacing existing cleanups; in most cases this means signals connected in the constructor and disconnected on destroy, but also other cases with a similarly defined lifetime (say: from show to hide). This doesn't change signal connections that only exist for a short time (say: once), handlers that are connected on-demand (say: the first time a particular method is called), or connections that aren't tracked (read: disconnected) at all. We will eventually replace the latter with connectObject() as well - especially from actor subclasses - but the changeset is already big enough as-is :-) Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1953>
This commit is contained in:

committed by
Marge Bot

parent
f45ccc9143
commit
26235bbe54
@ -506,7 +506,7 @@ var PopupMenuBase = class {
|
||||
|
||||
this._sensitive = true;
|
||||
|
||||
this._sessionUpdatedId = Main.sessionMode.connect('updated', this._sessionUpdated.bind(this));
|
||||
Main.sessionMode.connectObject('updated', () => this._sessionUpdated(), this);
|
||||
}
|
||||
|
||||
_getTopMenu() {
|
||||
@ -609,52 +609,41 @@ var PopupMenuBase = class {
|
||||
}
|
||||
|
||||
_connectItemSignals(menuItem) {
|
||||
menuItem._activeChangeId = menuItem.connect('notify::active', () => {
|
||||
let active = menuItem.active;
|
||||
if (active && this._activeMenuItem != menuItem) {
|
||||
if (this._activeMenuItem)
|
||||
this._activeMenuItem.active = false;
|
||||
this._activeMenuItem = menuItem;
|
||||
this.emit('active-changed', menuItem);
|
||||
} else if (!active && this._activeMenuItem == menuItem) {
|
||||
this._activeMenuItem = null;
|
||||
this.emit('active-changed', null);
|
||||
}
|
||||
});
|
||||
menuItem._sensitiveChangeId = menuItem.connect('notify::sensitive', () => {
|
||||
let sensitive = menuItem.sensitive;
|
||||
if (!sensitive && this._activeMenuItem == menuItem) {
|
||||
if (!this.actor.navigate_focus(menuItem.actor,
|
||||
St.DirectionType.TAB_FORWARD,
|
||||
true))
|
||||
this.actor.grab_key_focus();
|
||||
} else if (sensitive && this._activeMenuItem == null) {
|
||||
if (global.stage.get_key_focus() == this.actor)
|
||||
menuItem.actor.grab_key_focus();
|
||||
}
|
||||
});
|
||||
menuItem._activateId = menuItem.connect_after('activate', () => {
|
||||
this.emit('activate', menuItem);
|
||||
this.itemActivated(BoxPointer.PopupAnimation.FULL);
|
||||
});
|
||||
menuItem.connectObject(
|
||||
'notify::active', () => {
|
||||
const { active } = menuItem;
|
||||
if (active && this._activeMenuItem !== menuItem) {
|
||||
if (this._activeMenuItem)
|
||||
this._activeMenuItem.active = false;
|
||||
this._activeMenuItem = menuItem;
|
||||
this.emit('active-changed', menuItem);
|
||||
} else if (!active && this._activeMenuItem === menuItem) {
|
||||
this._activeMenuItem = null;
|
||||
this.emit('active-changed', null);
|
||||
}
|
||||
},
|
||||
'notify::sensitive', () => {
|
||||
const { sensitive } = menuItem;
|
||||
if (!sensitive && this._activeMenuItem === menuItem) {
|
||||
if (!this.actor.navigate_focus(menuItem.actor,
|
||||
St.DirectionType.TAB_FORWARD, true))
|
||||
this.actor.grab_key_focus();
|
||||
} else if (sensitive && this._activeMenuItem === null) {
|
||||
if (global.stage.get_key_focus() === this.actor)
|
||||
menuItem.actor.grab_key_focus();
|
||||
}
|
||||
},
|
||||
'activate', () => {
|
||||
this.emit('activate', menuItem);
|
||||
this.itemActivated(BoxPointer.PopupAnimation.FULL);
|
||||
}, GObject.ConnectFlags.AFTER,
|
||||
'destroy', () => {
|
||||
if (menuItem === this._activeMenuItem)
|
||||
this._activeMenuItem = null;
|
||||
}, this);
|
||||
|
||||
menuItem._parentSensitiveChangeId = this.connect('notify::sensitive', () => {
|
||||
menuItem.syncSensitive();
|
||||
});
|
||||
|
||||
// the weird name is to avoid a conflict with some random property
|
||||
// the menuItem may have, called destroyId
|
||||
// (FIXME: in the future it may make sense to have container objects
|
||||
// like PopupMenuManager does)
|
||||
menuItem._popupMenuDestroyId = menuItem.connect('destroy', () => {
|
||||
menuItem.disconnect(menuItem._popupMenuDestroyId);
|
||||
menuItem.disconnect(menuItem._activateId);
|
||||
menuItem.disconnect(menuItem._activeChangeId);
|
||||
menuItem.disconnect(menuItem._sensitiveChangeId);
|
||||
this.disconnect(menuItem._parentSensitiveChangeId);
|
||||
if (menuItem == this._activeMenuItem)
|
||||
this._activeMenuItem = null;
|
||||
});
|
||||
this.connectObject('notify::sensitive',
|
||||
() => menuItem.syncSensitive(), menuItem);
|
||||
}
|
||||
|
||||
_updateSeparatorVisibility(menuItem) {
|
||||
@ -726,28 +715,20 @@ var PopupMenuBase = class {
|
||||
}
|
||||
|
||||
if (menuItem instanceof PopupMenuSection) {
|
||||
let activeChangeId = menuItem.connect('active-changed', this._subMenuActiveChanged.bind(this));
|
||||
menuItem.connectObject(
|
||||
'active-changed', this._subMenuActiveChanged.bind(this),
|
||||
'destroy', () => this.length--, this);
|
||||
|
||||
let parentOpenStateChangedId = this.connect('open-state-changed', (self, open) => {
|
||||
if (open)
|
||||
menuItem.open();
|
||||
else
|
||||
menuItem.close();
|
||||
});
|
||||
let parentClosingId = this.connect('menu-closed', () => {
|
||||
menuItem.emit('menu-closed');
|
||||
});
|
||||
let subMenuSensitiveChangedId = this.connect('notify::sensitive', () => {
|
||||
menuItem.emit('notify::sensitive');
|
||||
});
|
||||
|
||||
menuItem.connect('destroy', () => {
|
||||
menuItem.disconnect(activeChangeId);
|
||||
this.disconnect(subMenuSensitiveChangedId);
|
||||
this.disconnect(parentOpenStateChangedId);
|
||||
this.disconnect(parentClosingId);
|
||||
this.length--;
|
||||
});
|
||||
this.connectObject(
|
||||
'open-state-changed', (self, open) => {
|
||||
if (open)
|
||||
menuItem.open();
|
||||
else
|
||||
menuItem.close();
|
||||
},
|
||||
'menu-closed', () => menuItem.emit('menu-closed'),
|
||||
'notify::sensitive', () => menuItem.emit('notify::sensitive'),
|
||||
menuItem);
|
||||
} else if (menuItem instanceof PopupSubMenuMenuItem) {
|
||||
if (beforeItem == null)
|
||||
this.box.add(menuItem.menu.actor);
|
||||
@ -755,15 +736,11 @@ var PopupMenuBase = class {
|
||||
this.box.insert_child_below(menuItem.menu.actor, beforeItem);
|
||||
|
||||
this._connectItemSignals(menuItem);
|
||||
let subMenuActiveChangeId = menuItem.menu.connect('active-changed', this._subMenuActiveChanged.bind(this));
|
||||
let closingId = this.connect('menu-closed', () => {
|
||||
menuItem.menu.connectObject('active-changed',
|
||||
this._subMenuActiveChanged.bind(this), this);
|
||||
this.connectObject('menu-closed', () => {
|
||||
menuItem.menu.close(BoxPointer.PopupAnimation.NONE);
|
||||
});
|
||||
|
||||
menuItem.connect('destroy', () => {
|
||||
menuItem.menu.disconnect(subMenuActiveChangeId);
|
||||
this.disconnect(closingId);
|
||||
});
|
||||
}, menuItem);
|
||||
} else if (menuItem instanceof PopupSeparatorMenuItem) {
|
||||
this._connectItemSignals(menuItem);
|
||||
|
||||
@ -771,13 +748,9 @@ var PopupMenuBase = class {
|
||||
// separator's adjacent siblings change visibility or position.
|
||||
// open-state-changed isn't exactly that, but doing it in more
|
||||
// precise ways would require a lot more bookkeeping.
|
||||
let openStateChangeId = this.connect('open-state-changed', () => {
|
||||
this.connectObject('open-state-changed', () => {
|
||||
this._updateSeparatorVisibility(menuItem);
|
||||
});
|
||||
let destroyId = menuItem.connect('destroy', () => {
|
||||
this.disconnect(openStateChangeId);
|
||||
menuItem.disconnect(destroyId);
|
||||
});
|
||||
}, menuItem);
|
||||
} else if (menuItem instanceof PopupBaseMenuItem) {
|
||||
this._connectItemSignals(menuItem);
|
||||
} else {
|
||||
@ -829,8 +802,7 @@ var PopupMenuBase = class {
|
||||
|
||||
this.emit('destroy');
|
||||
|
||||
Main.sessionMode.disconnect(this._sessionUpdatedId);
|
||||
this._sessionUpdatedId = 0;
|
||||
Main.sessionMode.disconnectObject(this);
|
||||
}
|
||||
};
|
||||
Signals.addSignalMethods(PopupMenuBase.prototype);
|
||||
@ -854,13 +826,12 @@ var PopupMenu = class extends PopupMenuBase {
|
||||
this.actor.reactive = true;
|
||||
|
||||
if (this.sourceActor) {
|
||||
this._keyPressId = this.sourceActor.connect('key-press-event',
|
||||
this._onKeyPress.bind(this));
|
||||
this._notifyMappedId = this.sourceActor.connect('notify::mapped',
|
||||
() => {
|
||||
this.sourceActor.connectObject(
|
||||
'key-press-event', this._onKeyPress.bind(this),
|
||||
'notify::mapped', () => {
|
||||
if (!this.sourceActor.mapped)
|
||||
this.close();
|
||||
});
|
||||
}, this);
|
||||
}
|
||||
|
||||
this._systemModalOpenedId = 0;
|
||||
@ -970,11 +941,7 @@ var PopupMenu = class extends PopupMenuBase {
|
||||
}
|
||||
|
||||
destroy() {
|
||||
if (this._keyPressId)
|
||||
this.sourceActor.disconnect(this._keyPressId);
|
||||
|
||||
if (this._notifyMappedId)
|
||||
this.sourceActor.disconnect(this._notifyMappedId);
|
||||
this.sourceActor?.disconnectObject(this);
|
||||
|
||||
if (this._systemModalOpenedId)
|
||||
Main.layoutManager.disconnect(this._systemModalOpenedId);
|
||||
@ -1333,20 +1300,19 @@ var PopupMenuManager = class {
|
||||
}
|
||||
|
||||
addMenu(menu, position) {
|
||||
if (this._findMenu(menu) > -1)
|
||||
if (this._menus.includes(menu))
|
||||
return;
|
||||
|
||||
let menudata = {
|
||||
menu,
|
||||
openStateChangeId: menu.connect('open-state-changed', this._onMenuOpenState.bind(this)),
|
||||
destroyId: menu.connect('destroy', this._onMenuDestroy.bind(this)),
|
||||
capturedEventId: menu.actor.connect('captured-event', this._onCapturedEvent.bind(this)),
|
||||
};
|
||||
menu.connectObject(
|
||||
'open-state-changed', this._onMenuOpenState.bind(this),
|
||||
'destroy', () => this.removeMenu(menu), this);
|
||||
menu.actor.connectObject('captured-event',
|
||||
this._onCapturedEvent.bind(this), this);
|
||||
|
||||
if (position == undefined)
|
||||
this._menus.push(menudata);
|
||||
this._menus.push(menu);
|
||||
else
|
||||
this._menus.splice(position, 0, menudata);
|
||||
this._menus.splice(position, 0, menu);
|
||||
}
|
||||
|
||||
removeMenu(menu) {
|
||||
@ -1355,13 +1321,12 @@ var PopupMenuManager = class {
|
||||
this._grab = null;
|
||||
}
|
||||
|
||||
let position = this._findMenu(menu);
|
||||
const position = this._menus.indexOf(menu);
|
||||
if (position == -1) // not a menu we manage
|
||||
return;
|
||||
|
||||
let menudata = this._menus[position];
|
||||
menu.disconnect(menudata.openStateChangeId);
|
||||
menu.disconnect(menudata.destroyId);
|
||||
menu.disconnectObject(this);
|
||||
menu.actor.disconnectObject(this);
|
||||
|
||||
this._menus.splice(position, 1);
|
||||
}
|
||||
@ -1426,7 +1391,7 @@ var PopupMenuManager = class {
|
||||
_findMenuForSource(source) {
|
||||
while (source) {
|
||||
let actor = source;
|
||||
const menu = this._menus.map(m => m.menu).find(m => m.sourceActor === actor);
|
||||
const menu = this._menus.find(m => m.sourceActor === actor);
|
||||
if (menu)
|
||||
return menu;
|
||||
source = source.get_parent();
|
||||
@ -1435,19 +1400,6 @@ var PopupMenuManager = class {
|
||||
return null;
|
||||
}
|
||||
|
||||
_onMenuDestroy(menu) {
|
||||
this.removeMenu(menu);
|
||||
}
|
||||
|
||||
_findMenu(item) {
|
||||
for (let i = 0; i < this._menus.length; i++) {
|
||||
let menudata = this._menus[i];
|
||||
if (item == menudata.menu)
|
||||
return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
|
||||
_closeMenu(isUser, menu) {
|
||||
// If this isn't a user action, we called close()
|
||||
// on the BoxPointer ourselves, so we shouldn't
|
||||
|
Reference in New Issue
Block a user