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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
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

QLatin1String * 5

156

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
  • 6 :P
156

Good point, will do.