Code review comment for lp:~azzar1/compiz/fix-781931

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Ah okay, now that I've had a look into this, this comment here is misleading:

    /* check whether we're allowed to stack under a sibling by finding
     * the above 'sibling' and checking whether or not we're allowed
     * to stack under that - if not, then there is no valid sibling
     * underneath it */

It really should say

    /* check whether we're allowed to stack under a sibling checking if
     * this window, or any other window above it is permitted to stack
     * above this window */

So I think you're definitely on the right track.

My only concern is that changing pendingMaps might have some unwanted side effects. Code that relies on pendingMaps could assume that show () can been called on the window, which has a few other invariants too (eg, not hidden, not shaded etc).

What might be better is to add a new variable to track whether or not the window is currently in map processing (eg, receivedMapRequestAndAwaitingMap). Then you can check that variable in avoidStackingRelativeTo in order to disregard the fact that the window is actually unmapped. Ideally receivedMapRequestAndAwaitingMap should only be set if we were to actually call show () on the window later (eg, !initiallyMinimized && !(priv->state & CompWindowStateHiddenMask))

« Back to merge proposal