Commit 59d66e30 authored by Kai Uwe Broulik's avatar Kai Uwe Broulik 🍇

[Notifications] Put history into a ListView

Notification uses Repeaters for everything. While this is acceptable for jobs and notifications, of which there are few,
the history can turn into a very long list of items. Using a ListView solves memory consumption by creating delegates only as needed.
Since regular notifications and notification history are quite entangled with each other, I had to configure the ListView
from within the Notifications loader. To keep code changes as little as possible, the rest of the UI is just moved into
the ListView header item.

While this is quite an invasive patch for a feature frozen version it's the least invasive approach I could find and is quite
an important memory leak fix for an LTS.

BUG: 389132
FIXED-IN: 5.12.0

CHANGELOG: Fixed memory leak when there are a lot of items in notification history

Differential Revision: https://phabricator.kde.org/D9978

(cherry picked from commit e8f76cc5)
parent 19882776
......@@ -26,7 +26,7 @@ import org.kde.kquickcontrolsaddons 2.0
PlasmaComponents.ListItem {
id: notificationItem
width: popupFlickable.width
width: historyList.width
property ListModel listModel;
......
......@@ -33,9 +33,9 @@ Column {
}
property alias count: notificationsRepeater.count
property alias historyCount: notificationsHistoryRepeater.count
readonly property int historyCount: historyList.count
property bool showHistory
property bool showHistory: plasmoid.configuration.showHistory
signal popupShown(var popup)
......@@ -272,10 +272,27 @@ Column {
visible: historyCount > 0
text: i18n("History")
}
Repeater {
id: notificationsHistoryRepeater
model: notificationsHistoryModel
delegate: NotificationDelegate { listModel: notificationsHistoryModel }
// History stuff
// The history is shown outside in a ListView
Binding {
target: historyList
property: "model"
value: notificationsHistoryModel
when: showHistory
}
Binding {
target: historyList
property: "delegate"
value: notificationsHistoryDelegate
when: showHistory
}
Component {
id: notificationsHistoryDelegate
NotificationDelegate {
listModel: notificationsHistoryModel
}
}
}
......@@ -39,8 +39,6 @@ MouseEventListener {
//Layout.minimumHeight: mainScrollArea.implicitHeight
Layout.minimumWidth: 256 // FIXME: use above
Layout.minimumHeight: 256
Layout.maximumWidth: -1
Layout.maximumHeight: mainScrollArea.implicitHeight
LayoutMirroring.enabled: Qt.application.layoutDirection === Qt.RightToLeft
LayoutMirroring.childrenInherit: true
......@@ -49,8 +47,8 @@ MouseEventListener {
property real globalProgress: 0
property Item notifications: notificationsLoader.item
property Item jobs: jobsLoader.item
property Item notifications: historyList.headerItem ? historyList.headerItem.notifications : null
property Item jobs: historyList.headerItem ? historyList.headerItem.jobs : null
//notifications + jobs
property int activeItemsCount: (notifications ? notifications.count : 0) + (jobs ? jobs.count : 0)
......@@ -100,59 +98,38 @@ MouseEventListener {
id: mainScrollArea
anchors.fill: parent
implicitWidth: theme.mSize(theme.defaultFont).width * 40
implicitHeight: Math.min(theme.mSize(theme.defaultFont).height * 40, Math.max(theme.mSize(theme.defaultFont).height * 6, contentsColumn.height))
state: ""
// HACK The history of notifications can become quite large. In order to avoid a memory leak
// show them in a ListView which creates delegate instances only on demand.
// The ListView's header functionality is abused to provide the jobs and regular notifications
// which are few and might store some state inside the delegate (e.g. expanded state) and
// thus are created all at once by a Repeater.
ListView {
id: historyList
Flickable {
id: popupFlickable
anchors.fill:parent
// The history stuff is quite entangled with regular notifications, so
// model and delegate are set by Bindings {} inside Notifications.qml
contentWidth: width
contentHeight: contentsColumn.height
clip: true
header: Column {
property alias jobs: jobsLoader.item
property alias notifications: notificationsLoader.item
Column {
id: contentsColumn
width: popupFlickable.width
width: historyList.width
Loader {
id: jobsLoader
width: parent.width
source: "Jobs.qml"
active: notificationsApplet.Plasmoid.configuration.showJobs
active: plasmoid.configuration.showJobs
}
Loader {
id: notificationsLoader
width: parent.width
source: "Notifications.qml"
active: notificationsApplet.Plasmoid.configuration.showNotifications
onLoaded: {
notificationsLoader.item.showHistory = Qt.binding(function(){ return notificationsApplet.Plasmoid.configuration.showHistory })
}
active: plasmoid.configuration.showNotifications
}
}
}
states: [
State {
name: "underMouse"
when: notificationsApplet.containsMouse
PropertyChanges {
target: mainScrollArea
implicitHeight: implicitHeight
}
},
State {
name: ""
when: !notificationsApplet.containsMouse
PropertyChanges {
target: mainScrollArea
implicitHeight: Math.min(theme.mSize(theme.defaultFont).height * 40, Math.max(theme.mSize(theme.defaultFont).height * 6, contentsColumn.height))
}
}
]
}
function action_clearNotifications() {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment