Review Request 121885: Properly check for systray being available

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121885/#review73456
-----------------------------------------------------------


I don't know the whole design around this, but this patch looks ... hackish (and possibly slow) to me.

It looks very implementation-dependent...

Is the absence of a well-known dbus name because there could be more than one systray?

Looking at this again, the first patch seemed better to me, is there any reason for the change?
The deadlock?

"StatusNotifierWatcher is another kded module, it can't respond because we're blocked awaiting a reply from ourselves."  makes no sense to me. In-process DBus calls definitely work. QtDBus turns it into a direct method call. In fact this makes the v1 of the patch really fast :)

- David Faure


On Jan. 7, 2015, 8:10 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121885/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2015, 8:10 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 339707
>     https://bugs.kde.org/show_bug.cgi?id=339707
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> -------
> 
> The "org.kde.StatusNotifierWatcher" is just a watcher/helper, not the actual systray object, that's "org.kde.StatusNotifierHost-$PID". Because Plasma appends the PID, we cannot check directly for it being present on the bus, so we check the org.kde.StatusNotifierWatcher.IsStatusNotifierHostRegistered property that's meant to be used for this.
> 
> Plus QSystemTrayIcon::isSystemTrayAvailable() is actually returning always true, because the Watcher is in kded and is /always/ present.
> 
> This also fixes many apps with KSNI crashing on plasma exit, bug 339707 (though I believe this is not the direct cause for that bug)
> 
> 
> Diffs
> -----
> 
>   src/platformtheme/kdeplatformsystemtrayicon.cpp b5e207c 
> 
> Diff: https://git.reviewboard.kde.org/r/121885/diff/
> 
> 
> Testing
> -------
> 
> Things do not crash anymore and the ::isSystemTrayAvailable() now returns correct value.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150108/f3dd865e/attachment.html>