GNOME Bugzilla – Bug 777784
Possible inconsistent calls to completed_size_change() when fullscreening
Last modified: 2017-01-26 16:55:18 UTC
Opening this bug as a continuation of bug #777691. gnome-shell is responsible for the multiple completed_size_change() calls being performed that trigger the inconsistent warnings/errors there. What seems to happen here is that we do receive one MetaPlugin::size-change event, which starts the _fullscreenAnimation function in windowManager.js, but later we occasionally receive more than one MetaPlugin::size-changed signals, which each time ends up in a completed_size_change(). AFAICS the emission of size-change and size-changed is somewhat decoupled in mutter, so I assume per design. But if that is so, it looks like the ::size-changed handler in windowManager.js should avoid reentrancy. And I do think the doubly emission happens because the window has first a NULL texture, but is later given one (Presumably after the client first drawn the window). I guess the animation could be skipped then, as there's not much of an initial state. I'm attaching patches doing these two things, either or both seem to fix the issue here.
Created attachment 344304 [details] [review] windowManager: Unset fullscreenInfo when dispatching ::size-changed We have no guarantees on the number of ::size-changed signals that we may receive, so the _sizeChangedWindow handler may run multiple times, which leads to multiple calls to meta_plugin_size_change_completed(). Unsetting the fullscreenInfo data ensures the handler code is only run once for the current fullscreening animation.
Created attachment 344305 [details] [review] windowManager: Avoid fullscreen animation if the window has no texture There is hardly anything to animate, so just avoid the animation in this case.
Review of attachment 344305 [details] [review]: looks fine, better be safe
Review of attachment 344304 [details] [review]: I had it like this at some point, but this means we destroy the clone immediately so the animation won't look good. I think what we need to prevent reentrancy here is just check if (this._resizing.indexOf(actor) != -1)
Created attachment 344331 [details] [review] windowManager: Avoid reentrancy on the ::size-changed handler We have no guarantees on the number of ::size-changed signals that we may receive, so the _sizeChangedWindow handler may run multiple times, which leads to multiple calls to meta_plugin_size_change_completed(). So double check the actor is not already being animated in the _sizeChangedWindow handler to avoid reentrancy.
(In reply to Rui Matos from comment #4) > Review of attachment 344304 [details] [review] [review]: > > I had it like this at some point, but this means we destroy the clone > immediately so the animation won't look good. > > I think what we need to prevent reentrancy here is just check if > (this._resizing.indexOf(actor) != -1) I liked this more :), this new patch goes as suggested, and also does work.
Review of attachment 344331 [details] [review]: ++
Thanks for the reviews! Attachment 344305 [details] pushed as e0d7d28 - windowManager: Avoid fullscreen animation if the window has no texture Attachment 344331 [details] pushed as a82c564 - windowManager: Avoid reentrancy on the ::size-changed handler