GNOME Bugzilla – Bug 676447
empathy git with appMenu freeze the shell
Last modified: 2012-06-18 08:06:23 UTC
Created attachment 214525 [details] [review] fix currentItems length lower than requested position issue Empathy with appMenu freezes the gnome shell , ie there is an "infinite" (which slow down the shell to a halt) loop. Ie to reproduce on have to apply the first patch about currentItems length being lower than requested position. Then there s a dance between added callback (ie _modeChanged in RemoteMenu of popupMenu.js). To fix the latter I add to the changeSignal callback the action_name argument then check if this signal has not already been handled (ie the this._actions[action_name] is not defined in this case). The whole case only happens when gnome-shell is started then empathy is started. If gnome-shell is killed then restarted with empathy running all is fine. From the log all the items (especially the favorite rooms) are not added fast enough on a first run thus _createMenuItem returns 'action-added' for most of the room menu items and a signal handler is added for action-added on the action_group for each. Then the signal handler is called after "one" item is added , which register a new signal handler on each of the other not yet registered ones. I have 17 favorites rooms here. It is enough to freeze the shell . Only have wait for a tenth of minutes.
Created attachment 214526 [details] [review] avoid freeze of the shell when all menu items are not added in one shot
Review of attachment 214526 [details] [review]: Makes sense
Review of attachment 214525 [details] [review]: I'm not sure about this - in what case can currentItems[k] be undefined (assuming that bookkeeping of ignored is correct, and assuming that signals are emitted in order)?
Review of attachment 214526 [details] [review]: ::: js/ui/popupMenu.js @@ +1966,3 @@ + let signalId = this.actionGroup.connect(changeSignal, Lang.bind(this, function(actionGroup, action_name) { + actionGroup.disconnect(signalId); + if (this._actions[action_name]) return; actionName, not action_name
*** Bug 676472 has been marked as a duplicate of this bug. ***
Created attachment 214548 [details] [review] 2- fix currentItems length lower than requested position issue also do not increment twice (once in the loop next, once on the loop content) k0
Created attachment 214549 [details] [review] 2 - avoid freeze of the shell when all menu items are not added in one shot use actionName instead of action_name
Review of attachment 214548 [details] [review]: This is wrong for sure. Essentially, k0 must be > j0 if there are _ignored items, and the only way for this to happen is to increment it more than j0. (j0 is the index of the first element to insert/remove as GMenu sees it, k0 is the index of the first element as we see it, including separators and section titles)
Created attachment 214571 [details] [review] do not overflow currentItems The first version of this currentItems overflow fix was more a workaround than a fix. Here is the current analysis of this part of the problem and an adhoc patch. JS ERROR: !!! Exception was: TypeError: currentItems[k0] is undefined when model._changedId = model.connect('items-changed', Lang.bind(this, this._modelChanged, target)); is triggered would be that position passed is the one in the model. Then in _createMenuItem if this.actionGroup.has_action(action_id) is null action-added is returned. We then receive he next position (items-changed) from the model which in // skip ignored items at the beginning while (k0 < currentItems.length && currentItems[k0]._ignored) k0++; // find the right menu item matching the model item for (j0 = 0; j0 < position; j0++, k0++) { if (currentItems[k0]._ignored) k0++; } when at position 1 leads to a k0 == 1 when currentItems contains zero items for this target (menu section here). Ie this boils down to elements created via model items-changed , ie all the others have a position equal to zero. Those elements are first in the model then in the gmenu . Thus currentItems (here created via action-addedà lags by one item.
Created attachment 214572 [details] [review] 2 - do not overflow currentItems Take into account the comment about k0 as required to be greater than j0 if ignored exists.
Review of attachment 214572 [details] [review]: This patch is fine, and should fix the original bug (ugh, btw, and kudos for finding it) We could also do by just giving up completely and waiting for the next action-added, which will destroy and recreate the section contents again.
*** Bug 676163 has been marked as a duplicate of this bug. ***
Would be nice to have this fix merged ASAP, the Shell is completely unusable atm with latest Empathy.
Review of attachment 214549 [details] [review]: Just a style fix. Fine to commit.
Alban - do you have a GNOME git account, or do you need somebody else to push these?
I cannot push. I have no git account.
All attachments pushed to master now. Thanks a lot for your patches!
Shouldn't them be pushed to 3.4 has well?
(In reply to comment #18) > Shouldn't them be pushed to 3.4 has well? Yeah that would be good. I'm going to use Shell 3.4 during a while and would like to be able to hack on Empathy master with it. :)
Backported to 3.4 as well.