Compiz Core

Merge lp://qastaging/~smspillaz/compiz-core/compiz-core.more-sound-reparenting-behaviour into lp://qastaging/compiz-core

compiz-core.more-sound-reparenting-behaviour
Merge into 0.9.7

Proposed by Sam Spilsbury on 2012-04-04

Status:

Merged

Merged at revision:

3085

Proposed branch:

lp://qastaging/~smspillaz/compiz-core/compiz-core.more-sound-reparenting-behaviour

Merge into:

lp://qastaging/compiz-core

Diff against target:

110 lines (+10/-45)

3 files modified

src/event.cpp (+2/-1)
src/privatescreen.h (+0/-2)
src/window.cpp (+8/-42)

To merge this branch:

bzr merge lp://qastaging/~smspillaz/compiz-core/compiz-core.more-sound-reparenting-behaviour

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Daniel van Vugt		2012-04-04	Approve on 2012-04-04
Review via email: mp+100720@code.qastaging.launchpad.net

Description of the change

Stab in the dark.

I think whats going on for bug 940603 is that windows aren't being removed from the window list if they are unreparented by the application itself in very special circumstances (not sure what because I can't reproduce the bug).

This code adds some more sound checks to see where the window is going and destroys it based on that, rather than using the same logic for both reparented and unreparented windows.

Also removed the whole destroyedFrameWindows tracking code which was more or ineffectual and could potentially result in a frame window leak.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-04-04:

In case you're wondering, I just corrected the bug number in the description.

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2012-04-04:

thanks

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-04-04:

I was optimistically hoping this branch would fix bug 865672. But it doesn't.

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2012-04-04:

Seems to work perfectly without any visible regressions.

I cannot confirm it fixes any particular bug at all. And I don't claim to fully understand the code. Based on recent history, that would lead me to abstain. However I strongly support any changes that make the code significantly smaller and easier to maintain. And if there is a chance this fixes bug 940603 then we really want to commit it for the sake of testing at least.

Less importantly, I think it's bad practice to assume None == 0 even though it is true. So this:
if (priv->serverFrame)
should be written as:
if (priv->serverFrame != None)

review: Approve