Lazy-instanciate and lazy-load KNotification plugins.
ClosedPublic

Authored by hein on Dec 4 2017, 11:53 AM.

Details

Summary

KNotificationManager always instanciates all of its built-in plugins
as well as any externally supplied icons as soon as the first
notification is sent, even if a particular plugin is not actually
needed by the notification.

This means e.g. always instanciating a text-to-speech engine even if
a notification event doesn't have TTS on (as basically none do by
default), or for that matter loading Phonon even when sounds are off.

This patch implements a lazy-loading approach instead. With the
patch applied, e.g. Konversation with default config will only ever
instanciate the Taskbar plugin, as its notification events only
enable this action by default.

The patch also speeds up time from login screen to desktop, by way
of the NetworkManager kded plugin not causing those costly
instanciations with its very early notification events anymore.

As a bonus, the patch also fixes a bug: The old code never properly
supported runtime installation of new plugins. If a plugin were to
be installed at runtime and enabled by the user in the config UI,
it likely never would have been instanciated if a preceding
notification had already caused the one and only instanciation pass
by that time.

Test Plan

Tests pass.

Diff Detail

Repository
R289 KNotifications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
hein created this revision.Dec 4 2017, 11:53 AM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptDec 4 2017, 11:53 AM
hein requested review of this revision.Dec 4 2017, 11:53 AM
davidedmundson accepted this revision.Dec 4 2017, 2:01 PM

One minor comment, then good to go.

src/knotificationmanager.cpp
136 ↗(On Diff #23417)

QLatin1String * 5

156 ↗(On Diff #23417)

No reason we can't add that field now and for compatibility instantiate if that field is not set.

This revision is now accepted and ready to land.Dec 4 2017, 2:01 PM
hein updated this revision to Diff 23426.Dec 4 2017, 2:24 PM

Add and utilize a X-KDE-KNotification-OptionName field in
KNotificationPlugin to try and avoid more instanciations.

davidedmundson accepted this revision.Dec 4 2017, 2:32 PM
davidedmundson added inline comments.
src/knotificationmanager.cpp
160

please add a comment explaining this.

This revision was automatically updated to reflect the committed changes.
hein added a comment.Dec 4 2017, 2:39 PM
src/knotificationmanager.cpp
136 ↗(On Diff #23417)
  • 6 :P
156 ↗(On Diff #23417)

Good point, will do.