Move StatusNotifierItem for VirtualKeyboard into a helper process
AbandonedPublic
Actions

Authored by graesslin on Oct 6 2017, 7:29 PM.

Details

Reviewers

Group Reviewers

KWin
Plasma

Summary

During startup the StatusNotifierItem can cause a freeze as it might do
a blocking DBus call while the other side is performing a Wayland
roundtrip. This freeze is broken when the DBus call times out but then
the SNI doesn't work either.

By moving the SNI into a dedicated helper process KWin itself does not
have any blocking calls to DBus. In addition KWin exposes a DBus API to
enable/disable the virtual keyboard, so that other processes can also
make use of it to provide e.g. a better UI (e.g. Plasmoid).

BUG: 385371

Test Plan

Started a new session, it did not freeze and I had a SNI which
worked as expected

Diff Detail

Repository

R108 KWin

Branch

virt-keyboard-sni

Lint

No Linters Available

Unit

No Unit Test Coverage

graesslin created this revision.Oct 6 2017, 7:29 PM

Restricted Application added a project: KWin. · View Herald TranscriptOct 6 2017, 7:29 PM

Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

Immediately going for a workaround is.. (and I don't want to use this term as you've clearly made a huge patch) lazy.

Especially as the current code path in your trace shows we're doing something very stupid:

we go through KStatusNotifierItem
we find we don't have SNI in a non blocking way (see frame 11 of your trace)
so it falls back to using a QSystemTray
.... which then checks to see if we have an SNI!

Let me fix KNotifications.

In D8163#152777, @davidedmundson wrote:

Immediately going for a workaround is.. (and I don't want to use this term as you've clearly made a huge patch) lazy.

Hey, I discussed this with Marco yesterday afternoon ;-)

If you get it fixed in KNotifications I'm happy to discard this review. I'm putting up the export of the DBus protocol nevertheless as it's in general useful.

Just the DBus protocol is in D8166

deadlock fixed in sni, D8166 should replace this

graesslin abandoned this revision.Oct 31 2017, 9:33 AM

Revision Contents
Changeset List

		Path
M		CMakeLists.txt (1 line)
M		autotests/CMakeLists.txt (8 lines)
A	M	autotests/test_virtualkeyboard_dbus.cpp (142 lines)
M		config-kwin.h.cmake (1 line)
M		helpers/CMakeLists.txt (1 line)
A	M	helpers/virtualkeyboard/CMakeLists.txt (10 lines)
A	M	helpers/virtualkeyboard/main.cpp (40 lines)
A	M	helpers/virtualkeyboard/sni.h (39 lines)
A	M	helpers/virtualkeyboard/sni.cpp (87 lines)
M		virtualkeyboard.h (8 lines)
M		virtualkeyboard.cpp (51 lines)
A	M	virtualkeyboard_dbus.h (62 lines)
A	M	virtualkeyboard_dbus.cpp (37 lines)