Moves all the stuff that is needed to start DBus on macOS (which for some reason it needs several steps, involving the use of `launchctl`) into a single `startDBusDaemon()` function. Before, it was spread into `kdeconnectconfig.cpp`, `indicatorhelper_mac.cpp` and `dbushelper.cpp`.
It also removes checking for an existing DBus daemon and always starts our own, since in most cases we couldn't connect to it anyway. This, together with removing the sleep in the retries when polling for the DBus daemon from 3s to 100ms, makes the startup much faster, so I removed the loading splash screen.
- Using QLatin1String when concatinating strings is faster, because they
are more lightweight. For the resulting string, we need to allocate
new memory anyway
- Use QLatin1String overloads where they are provided by Qt APIs
- Just use const char* for log messages, the quoting of QStrings is not
needed
- Make sure to reuse string results when possible
Unset launchctl env before running the new one, to avoid connecting through the old launchctl env.
Show a warn message for D-Bus connection failure on macOS.
Better patch to replace !218.
- Auto and quick detection of previous D-Bus instance;
- Remove private D-Bus compile definition, only use it on macOS without an existing D-Bus instance;
- Safe reboot after crashes because the indicator is not relating on the kdeconnectd to run a D-Bus session;
- Safe exit after clicking on `Quit` in the systray.
More details in commit logs:
Only enable private D-Bus on macOS because the other platforms do not
need them.
The app should be able to easily detect the session bus from the env
DBUS_LAUNCHD_SESSION_BUS_SOCKET from launchd through launchctl.
Because https://gitlab.freedesktop.org/dbus/dbus/-/blob/master/dbus/dbus-sysdeps-unix.c#L4392
shows that it is the only probing method on macOS with launchd.
The D-Bus session bus can be easily found from launchd/launchctl
with DBUS_LAUNCHD_SESSION_BUS_SOCKET env. It can be an external one
(installed from HomeBrew) or an internal one (launched by a previous
instance followed by a crash).
The indicator helper on macOS can now automatically detect whether we can use a potentially
(with launchd/launchctl env set, or KDE Connect macOS
private_bus_address set) existed and usable session bus.
If previous bus is usable, just try to launch the kdeconnectd with us.
Otherwise, launch a private D-Bus daemon, export the launchd/launchctl
env, and run a kdeconnectd instance.
Everything works better and quicker now :)
## Summary
On MacOS the system tray icon is off-color because it uses the gray-colored `kdeconnectindicatordark` icon which does not match the rest of MacOS.
![image](/uploads/430933399d9570dc1c59807e4715e87b/image.png)
BUG: 430226
I've used two patches to fix this:
1. Always use the status `KStatusNotifierItem::Passive` on MacOS. `KStatusNotifierItem` will only mark the icon as a mask (which is needed to allow it to dynamically switch between light and dark theme) [if the status is passive](cff7c337ab/src/kstatusnotifieritem.cpp (L1079-1081)).
2. The above should theoretically be enough to fix the issue and I swear at one point it was all that was needed. However, to fix this issue in my dev environment I also needed to pass in a `QIcon` with `setIsMask(true)` instead of setting the icon by its name. And I also use the `kdeconnectindicator` instead of `kdeconnectindicatordark` icon.
## Test Plan
The icon now renders in the correct color, regardless of whether devices are connected:
![image](/uploads/5010a07cbb5f23a286ece641c6b3879c/image.png) ![image](/uploads/2ae5d3d8aa633ebafb260febe313057c/image.png)
## Future work
Once I've verified this PR is working in the right direction, I want to look into making the icon gray (and hopefully a much easier-to-see gray) when no devices are connected!
For example, WireGuard, when not connected, looks like this:
![image](/uploads/43c2ef6bc7261431e878c9c1c05174f9/image.png) ![image](/uploads/f7587190648606df77ad3e3dde84098f/image.png)
P.S. I've been testing off the v21.12.2 tag since the master branch doesn't compile for me, so I haven't tested this change on the latest dev commit. But there should be no conflicts.