Review Request: Manage m_preview->deco pointer around plugin juggling

[Thomas Lübking]

> On July 25, 2012, 2:30 p.m., Martin Gräßlin wrote:
> > I would appreciate if this would go into 4.9.0.
> > 
> > Short summary for the release team:
> > * third party decorations (namely QtCurve and keramik) implemented an internal and unstable API for window decorations
> > * this API is called KDecoration*Unstable* to prevent anyone having the awesome idea to implement it
> > * we ensured that KWin and Systemsettings do not crash when an incompatible deco is found
> > * for Systemsettings there was a small bug which might lead to a crash (see referenced bug)
> > * I would like to not ship with a known crasher
> > * I hope I correctly summarized it :-)

No worry, i'm back in civilization and will have usable access to my system in time ;-)
Also, despite what Ben said, 4.9 is not yet frozen - so we *should* still be permitted to just push?!

About the crasher fixed here  - API safety is not related. Was the general "make it snappier" commit.

I introduced it by attempting to stop (long time present) leaking plugins (and unloading them as well)
Ultimately this (and similar usage) was very likey the source of the GHNS crashes as well, the preview/plugin link is not robust in this regard.


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105721/#review16378
-----------------------------------------------------------


On July 25, 2012, 2:30 p.m., Thomas Lübking wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105721/
> -----------------------------------------------------------
> 
> (Updated July 25, 2012, 2:30 p.m.)
> 
> 
> Review request for kwin, Release Team and Martin Gräßlin.
> 
> 
> Description
> -------
> 
> it is mandatory to manage loadPlugin() and destroyPreviousPlugin() using disablePreview()
> loadPlugin() moves the present factory pointer to "old_fact" which is then deleted by the succeeding destroyPreviousPlugin()
> 
> I also left a warning into the sources for our heirs ;-)
> 
> 
> This addresses bug 304026.
>     http://bugs.kde.org/show_bug.cgi?id=304026
> 
> 
> Diffs
> -----
> 
>   kwin/kcmkwin/kwindecoration/decorationmodel.cpp 3d3bb86 
>   kwin/kcmkwin/kwindecoration/preview.h 20ee869 
>   kwin/kcmkwin/kwindecoration/preview.cpp fe802c7 
> 
> Diff: http://git.reviewboard.kde.org/r/105721/diff/
> 
> 
> Testing
> -------
> 
> Yes, quite some.
> I'm pretty sure this is it and that a sanitation on recreate is not necessary for commented reasons.
> The actual troublemaker was the BorderSizesRole implementation
> 
> 
> /me wonders whether we can now also shortcut if (lib->loaded()) - gonna try that tonight.
> 
> I *may* be not be back in time (but i doubt so), so iff i've not acted on call until 22:00 CEST just push this and the other one or two reviews in a row before the freeze on my behalf. Thanks.
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/release-team/attachments/20120725/6d2ae96a/attachment.html>