Review Request: Some system tray fixes

Marco Martin notmart at gmail.com
Sun Mar 7 17:31:07 CET 2010



> On 2010-03-07 12:49:13, Marco Martin wrote:
> > trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/ui/taskarea.cpp, line 418
> > <http://reviewboard.kde.org/r/3165/diff/1/?file=20423#file20423line418>
> >
> >     are you sure this is not needed anymore? do a separate commit for that.
> 
> Andreas Hartmetz wrote:
>     I'm not really sure, so I'll drop that part for now. This hack here is, according to its commit message, supposed to fix some glitch after wake up from suspend. I don't have a computer with working suspend (to disk) currently so I can't test.

i would say to do a separate commit for that and see if breaks something, would be easier to revert just in case


> On 2010-03-07 12:49:13, Marco Martin wrote:
> > trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/ui/taskarea.cpp, line 86
> > <http://reviewboard.kde.org/r/3165/diff/1/?file=20423#file20423line86>
> >
> >     you are still not deleting the bgstore anywhere
> 
> Andreas Hartmetz wrote:
>     Not explicitly. But K_GLOBAL_STATIC will delete its contents at application shutdown, which I think is good enough. Otherwise we'd have to recreate the icon background everytime useCount goes from 0 to 1. This is your decision, I guess. I just emulated the old code without the deletion part here.

yes, but what i'm concerned is that you can close the applet without closing the application, so would keep hanging, at least until you add a second systray, probably no biggie but not super nice
thinkng abot it what i'll probably do even if not super nice code is to create it in the systray applet then pass it, so there will be a copy for each systray, deleted at the death of every systray


- Marco


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


On 2010-03-07 01:16:48, Andreas Hartmetz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3165/
> -----------------------------------------------------------
> 
> (Updated 2010-03-07 01:16:48)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> The system tray behaves very erratically here, depending on exact circumstances (compositing or not, some apparently random factors) so I looked into the code a bit.
> First I removed a very ugly hack that apparently fixed a bug that could be fixed otherwise, this is why I'm posting this to reviewboard. That part is the last hunk.
> After doing that plasma-desktop crashed a lot, so I fixed the obviously dangerous (the way non-POD statics are) to outright incorrect (deleting and never recreating the background thingie) usage of class-statics, too.
> 
> 
> This addresses bug 228655.
>     https://bugs.kde.org/show_bug.cgi?id=228655
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/plasma/generic/applets/systemtray/ui/taskarea.cpp 1100150 
> 
> Diff: http://reviewboard.kde.org/r/3165/diff
> 
> 
> Testing
> -------
> 
> Not much yet, about two hours of use - no crashes so far. No background corruption.
> 
> Update: Two things are still strange:
> - The Konversation icon usually has the same background as other icons, except when it's flashing - then it temporarily gets the background of the notifier (the 'i' in circle thingie). This might be new behavior with these patches.
> - The Nepomuk indexer icon still auto-hides after a while (a few minutes) even though configured to "Always Show". I've even seen it unhide for a fraction of a second and then hide again while I was just typing a message in Konversation. This behavior (except maybe the brief appearance) is not new.
> 
> 
> Thanks,
> 
> Andreas
> 
>



More information about the Plasma-devel mailing list