Differential D22553

guard against plasmashell being properly dead and not showing the SNI
ClosedPublic
Actions

Authored by sitter on Jul 19 2019, 10:41 AM.

Details

Reviewers
davidedmundson
Group Reviewers
Plasma
Commits
R871:3eeb0ce5bc1e: guard against plasmashell being properly dead and not showing the SNI
Summary

when plasmashell is particularly dead and not even able to restart, the SNI
host may still be around when we check for it in the main.cpp as its
disappearance from dbus is tied to sockets closing and dbus' event loop
reacting to that. so there is a race potential that can cause the SNI
to not actually be visible on account of the SNI host (plasmashell)
being dead even though we expected SNI support to be available.

guard against this scenario by starting an automatic activation timer
when the notification service disappears. should it not reappear within
10 second we consider it dead and show the dialog automatically as the
user won't be seeing the SNI anymore

Test Plan
  • crash something so the sni appears
  • kill plasma
  • wait 10s
  • dialog appears
  • same again
  • restart plasma before 10s have passed
  • sni appears again in plasmashell

Diff Detail

Repository
R871 DrKonqi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Jul 19 2019, 10:41 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 19 2019, 10:41 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Jul 19 2019, 10:41 AM
davidedmundson added a subscriber: davidedmundson.Jul 19 2019, 10:49 AM

Concept is fine, but you probably want to watch whether the SNI Watcher has an available host.

src/statusnotifier.cpp
100

SNIs are different to notifications.

sitter added a comment.Jul 19 2019, 10:53 AM

I suspect we'll also want bool StatusNotifier::notificationServiceRegistered further down changed?

sitter added a comment.Jul 22 2019, 9:12 PM

This actually sucks tremendously.

Kai said he used that service because he primarily cared about the notification being pretty.

BUUUT actually considering the StatusNotifierHost is hugely horrible. We'd have to check and watch StatusNotifierWatcher (kded) which tracks the hosts (plasmashell) and then watch its host registration property. This would blow things up quite a bit and honestly it's not wroth the faff as we could just latch on to the Notifications service and get much the same end result in 99% of cases I imagine?

davidedmundson accepted this revision.Jul 23 2019, 6:46 AM
This revision is now accepted and ready to land.Jul 23 2019, 6:46 AM
Closed by commit R871:3eeb0ce5bc1e: guard against plasmashell being properly dead and not showing the SNI (authored by sitter). · Explain WhyJul 23 2019, 1:51 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/statusnotifier.cpp (18 lines)

Diff 62393

View Options

src/statusnotifier.cpp

Loading...