From 38eb1ddb998db19baab8c3f5f906dad78e7adf34 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 1/8] testNotificationsApplet.js: Add Looking Glass checks for the notifications applet. Covers: - rendered order in both sort directions - banner handoff and handback, including a revision while banner up - urgency picking the panel icon - an arrival while the menu is open - transient notifications - the panel and menu chrome - a source holding several notifications - a clear whose destroy() fails partway - benchmark() times the operations that block the main loop --- js/testing/testNotificationsApplet.js | 664 ++++++++++++++++++++++++++ 1 file changed, 664 insertions(+) create mode 100644 js/testing/testNotificationsApplet.js diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js new file mode 100644 index 0000000000..af6fd92b1d --- /dev/null +++ b/js/testing/testNotificationsApplet.js @@ -0,0 +1,664 @@ +// Test helpers for the notifications applet. From Looking Glass: +// +// let t = imports.testing.testNotificationsApplet; +// t.checkOrder(); t.benchmark(200); t.fill(50); t.cleanup(); +// +// Every check clears the tray first, destroying any notifications you had. fill() adds to it. +// benchmark() stalls the session while it runs. + +const GLib = imports.gi.GLib; + +const Main = imports.ui.main; + +const MessageTray = imports.ui.messageTray; + +const AppletManager = imports.ui.appletManager; + +const UUID = "notifications@cinnamon.org"; + +let sources = []; + +function _applet() { + let instances = AppletManager.getRunningInstancesForUuid(UUID); + if (instances.length === 0) + throw new Error(`${UUID} is not on a panel`); + return instances[0]; +} + +function fill(n) { + let applet = _applet(); + + for (let i = 0; i < n; i++) { + let source = new MessageTray.SystemNotificationSource("Test"); + Main.messageTray.add(source); + sources.push(source); + + let notification = new MessageTray.Notification( + source, `Test notification ${i}`, `Body text for notification ${i}.`); + source.pushNotification(notification); + applet._notification_added(Main.messageTray, notification); + } + return applet.notifications.length; +} + +function cleanup() { + let applet = _applet(); + applet.menu.close(); + applet._clear_all(); + // Let in-flight banner transitions finish while their actors are alive. One completing + // after a later check has torn the applet down is reported as a critical. + _pumpUntil(() => false, 150); + sources.forEach(source => { + try { source.destroy(); } catch (e) {} + }); + sources = []; + log(`[testNotificationsApplet] cleaned up, ${applet.notifications.length} left`); +} + +function _newSource(title) { + let source = new MessageTray.SystemNotificationSource(title || "Test"); + Main.messageTray.add(source); + sources.push(source); + return source; +} + +function _notify(applet, source, title, urgency) { + let notification = new MessageTray.Notification(source, title, "body"); + if (urgency !== undefined) + notification.setUrgency(urgency); + source.pushNotification(notification); + applet._notification_added(Main.messageTray, notification); + return notification; +} + +function _rowActors(applet) { + return applet._notificationbin.get_children(); +} + +// The order update_list() renders in: the applet's list, reversed when showNewestFirst is set. +function _displayOrder(applet, notifications) { + let order = applet.notifications.filter(n => notifications.indexOf(n) !== -1); + return applet.showNewestFirst ? order.reverse() : order; +} + +function _orderMatches(applet) { + let wanted = _displayOrder(applet, applet.notifications); + + let children = _rowActors(applet); + if (children.length !== wanted.length) + return `bin has ${children.length} row actors, list has ${wanted.length}`; + + for (let i = 0; i < wanted.length; i++) { + if (children[i] !== wanted[i].actor) + return `actor at index ${i} is not the expected notification`; + } + return null; +} + +function checkOrder() { + let applet = _applet(); + let failures = 0; + + let check = (label) => { + let problem = _orderMatches(applet); + if (problem) { + failures++; + log(`[testNotificationsApplet] FAIL ${label}: ${problem}`); + } else { + log(`[testNotificationsApplet] ok ${label}`); + } + }; + + let original = applet.showNewestFirst; + try { + + for (let newestFirst of [false, true]) { + applet.menu.close(); + applet._clear_all(); + applet.showNewestFirst = newestFirst; + + // Order is only guaranteed once the menu opens: layout runs only while it is active. + fill(5); + + applet._openMenu(); + check(`newestFirst=${newestFirst}, after opening the menu`); + + applet.showNewestFirst = !newestFirst; + applet.update_list(); + check(`newestFirst=${!newestFirst}, after flipping the setting`); + + applet.showNewestFirst = newestFirst; + applet.update_list(); + check(`newestFirst=${newestFirst}, after flipping back`); + + applet.notifications[2].destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + check(`newestFirst=${newestFirst}, after dismissing one`); + + applet.menu.close(); + applet._clear_all(); + if (_rowActors(applet).length !== 0) { + failures++; + log(`[testNotificationsApplet] FAIL newestFirst=${newestFirst}: bin not empty after clear`); + } + } + + } finally { + applet.showNewestFirst = original; + cleanup(); + } + log(`[testNotificationsApplet] checkOrder: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; +} + +function checkBannerHandover() { + let applet = _applet(); + let tray = Main.messageTray; + let failures = 0; + + try { + applet.menu.close(); + applet._clear_all(); + + let listed = []; + for (let i = 0; i < 4; i++) { + fill(1); + listed.push(applet.notifications[applet.notifications.length - 1]); + } + applet._openMenu(); + + let borrowed = listed[1]; + tray._notificationQueue.push(borrowed); + tray._showNotification(); + + if (borrowed.actor.get_parent() === applet._notificationbin) { + failures++; + log("[testNotificationsApplet] FAIL banner: the tray did not take the actor"); + } + + applet.update_list(); + + let rows = _rowActors(applet); + let wanted = _displayOrder(applet, [listed[0], listed[2], listed[3]]).map(n => n.actor); + if (rows.length !== wanted.length) { + failures++; + log(`[testNotificationsApplet] FAIL banner: ${rows.length} rows, expected ${wanted.length}`); + } else { + for (let i = 0; i < wanted.length; i++) { + if (rows[i] !== wanted[i]) { + failures++; + log(`[testNotificationsApplet] FAIL banner: actor at index ${i} is not the expected one`); + break; + } + } + } + if (failures === 0) + log("[testNotificationsApplet] ok rebuild with an actor on loan to a banner"); + + applet._clear_all(); + if (!borrowed._destroyed) { + failures++; + log("[testNotificationsApplet] FAIL banner: clear left the borrowed notification alive"); + } + if (applet.notifications.length !== 0 || _rowActors(applet).length !== 0) { + failures++; + log(`[testNotificationsApplet] FAIL banner: clear left list=${applet.notifications.length} bin=${_rowActors(applet).length}`); + } else { + log("[testNotificationsApplet] ok clear with an actor on loan to a banner"); + } + + } finally { + // Give the actor back and disarm the dwell timer, so nothing is left queued against + // actors the next check destroys. Never hide mid-show: State.SHOWING is 1. + _pumpUntil(() => tray._notificationState !== 1, 300); + // Only if the tray still holds a live one: handing back a destroyed actor is the bug. + if (tray._notification && !tray._notification._destroyed) { + try { tray._hideNotificationCompleted(); } catch (e) { /* best effort */ } + } + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } + cleanup(); + } + + log(`[testNotificationsApplet] checkBannerHandover: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; +} + +function checkHandback() { + let applet = _applet(); + let tray = Main.messageTray; + let ok = true; + function check(label, condition) { + global.log("checkHandback: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + try { + applet.menu.close(); + applet._clear_all(); + fill(4); + applet._openMenu(); + + let listed = applet.notifications.slice(); + let borrowed = listed[1]; + let wasAt = _rowActors(applet).indexOf(borrowed.actor); + + tray._notificationQueue.push(borrowed); + tray._showNotification(); + // Hiding first makes the tray throw: _showNotificationCompleted() reads + // this._notification without a null check. See docs/issues/messagetray-issues.md. + _pumpUntil(() => tray._notificationState === 2, 200); + check("the tray took the actor", borrowed.actor.get_parent() !== applet._notificationbin); + + tray._hideNotificationCompleted(); + + check("the actor came back to the bin", + borrowed.actor.get_parent() === applet._notificationbin); + check("it is marked as living in the bin again", borrowed._inNotificationBin === true); + check("its timestamp is visible again", borrowed._timeLabel.visible === true); + check("the list is unchanged (" + applet.notifications.length + " of " + + listed.length + ")", applet.notifications.length === listed.length); + check("it is back at the same position (" + wasAt + ")", + _rowActors(applet).indexOf(borrowed.actor) === wasAt); + } finally { + // Never hide mid-show: the completion callback reads this._notification, which hiding + // sets to null. State.SHOWING is 1. + _pumpUntil(() => tray._notificationState !== 1, 300); + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires + // inside a later check, against a notification already destroyed. + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } + cleanup(); + } + + global.log("checkHandback: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkRevisedWhileShowing() { + let applet = _applet(); + let tray = Main.messageTray; + let ok = true; + function check(label, condition) { + global.log("checkRevisedWhileShowing: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + try { + applet.menu.close(); + applet._clear_all(); + fill(3); + applet._openMenu(); + + let borrowed = applet.notifications[1]; + tray._notificationQueue.push(borrowed); + tray._showNotification(); + _pumpUntil(() => tray._notificationState === 2, 200); + + borrowed.update("revised title", "revised body"); + check("the revision cleared the in-bin flag", borrowed._inNotificationBin === false); + + tray._hideNotificationCompleted(); + + check("the actor came back", borrowed.actor.get_parent() === applet._notificationbin); + check("the in-bin flag was restored", borrowed._inNotificationBin === true); + check("the timestamp is visible again", borrowed._timeLabel.visible === true); + check("the list did not grow (" + applet.notifications.length + ")", + applet.notifications.length === 3); + } finally { + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires + // inside a later check, against a notification already destroyed. + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } + cleanup(); + } + + global.log("checkRevisedWhileShowing: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkFailedClear() { + let applet = _applet(); + let failures = 0; + + try { + applet.menu.close(); + applet._clear_all(); + fill(5); + applet._openMenu(); + + // Index 2 throws, and _clear_all() destroys back-to-front, so 0 and 1 are never + // attempted. All three should survive, then die on a retry without the fault. + let survivors = applet.notifications.slice(0, 3); + let victim = applet.notifications[2]; + let realDestroy = victim.destroy; + victim.destroy = function () { throw new Error("injected destroy failure"); }; + + try { + applet._clear_all(); + failures++; + log("[testNotificationsApplet] FAIL failed-clear: the injected error did not propagate"); + } catch (e) { + } + + let destroyedEarly = survivors.filter(n => n._destroyed); + if (destroyedEarly.length > 0) { + failures++; + log(`[testNotificationsApplet] FAIL failed-clear: ${destroyedEarly.length} survivor(s) were destroyed anyway despite the injected failure`); + } else { + log("[testNotificationsApplet] ok survivors were not destroyed by the failed clear"); + } + + victim.destroy = realDestroy; + applet._clear_all(); + + let stillUndestroyed = survivors.filter(n => !n._destroyed); + if (stillUndestroyed.length !== 0) { + failures++; + log(`[testNotificationsApplet] FAIL failed-clear: retry did not finish the job -- ${stillUndestroyed.length} survivor(s) were never destroyed`); + } else { + log("[testNotificationsApplet] ok retry destroyed the survivors"); + } + } finally { + cleanup(); + } + + log(`[testNotificationsApplet] checkFailedClear: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; +} + +function checkUrgency() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkUrgency: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + let iconName = () => applet._applet_icon.get_icon_name(); + + try { + applet.menu.close(); + applet._clear_all(); + check("empty: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); + check("empty: not blinking", applet._blinking === false); + + let source = _newSource(); + _notify(applet, source, "low", MessageTray.Urgency.LOW); + check("low: icon is low-notif (" + iconName() + ")", iconName() === "low-notif"); + + _notify(applet, source, "normal", MessageTray.Urgency.NORMAL); + check("normal outranks low (" + iconName() + ")", iconName() === "normal-notif"); + check("normal: not blinking", applet._blinking === false); + + let critical = _notify(applet, source, "critical", MessageTray.Urgency.CRITICAL); + check("critical: blinking", applet._blinking === true); + + // Nothing stops a caller raising urgency after listing, and the icon has to follow. + critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + let late = _notify(applet, source, "late"); + late.setUrgency(MessageTray.Urgency.CRITICAL); + applet.update_list(); + check("urgency raised after arrival: blinking", applet._blinking === true); + late.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + + check("after the criticals go: not blinking", applet._blinking === false); + check("after the critical goes: icon is normal-notif (" + iconName() + ")", + iconName() === "normal-notif"); + + applet._clear_all(); + check("cleared: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); + check("cleared: not blinking", applet._blinking === false); + } finally { + cleanup(); + } + + global.log("checkUrgency: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkArrivalWhileOpen() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkArrivalWhileOpen: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + let original = applet.showNewestFirst; + try { + for (let newestFirst of [false, true]) { + applet.menu.close(); + applet._clear_all(); + applet.showNewestFirst = newestFirst; + fill(3); + applet._openMenu(); + + let source = _newSource(); + let arrived = _notify(applet, source, "arrived while open"); + + let rows = _rowActors(applet); + check("newestFirst=" + newestFirst + ": it is listed (" + + applet.notifications.length + ")", applet.notifications.length === 4); + check("newestFirst=" + newestFirst + ": its actor is in the bin", + rows.indexOf(arrived.actor) !== -1); + check("newestFirst=" + newestFirst + ": it is at the end the setting asks for", + rows.indexOf(arrived.actor) === (newestFirst ? 0 : rows.length - 1)); + // The heading is translated: look for the number, not the wording. + check("newestFirst=" + newestFirst + ": the heading followed (" + + applet.menu_label.label.get_text() + ")", + applet.menu_label.label.get_text().indexOf("4") !== -1); + } + } finally { + applet.showNewestFirst = original; + cleanup(); + } + + global.log("checkArrivalWhileOpen: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkTransient() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkTransient: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + let original = applet.ignoreTransientNotifications; + try { + applet.menu.close(); + applet._clear_all(); + + applet.ignoreTransientNotifications = false; + let kept = new MessageTray.Notification(_newSource(), "transient", "body"); + kept.setTransient(true); + applet._notification_added(Main.messageTray, kept); + check("setting off: a transient is listed (" + applet.notifications.length + ")", + applet.notifications.indexOf(kept) !== -1); + + applet._clear_all(); + applet.ignoreTransientNotifications = true; + let dropped = new MessageTray.Notification(_newSource(), "transient", "body"); + dropped.setTransient(true); + applet._notification_added(Main.messageTray, dropped); + check("setting on: a transient is not listed (" + applet.notifications.length + ")", + applet.notifications.indexOf(dropped) === -1); + check("setting on: a transient was destroyed", dropped._destroyed === true); + + let normal = _notify(applet, _newSource(), "normal"); + check("setting on: a normal notification is still listed", + applet.notifications.indexOf(normal) !== -1); + } finally { + applet.ignoreTransientNotifications = original; + cleanup(); + } + + global.log("checkTransient: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkTrayChrome() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkTrayChrome: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + let panelLabel = () => applet._applet_label.get_text(); + let heading = () => applet.menu_label.label.get_text(); + + let originalEmpty = applet.showEmptyTray; + let originalCount = applet.showNotificationCount; + try { + // Assigning these directly skips the settings binding, so call what it would have called. + applet.showEmptyTray = true; + applet.showNotificationCount = true; + + applet.menu.close(); + applet._clear_all(); + applet._show_hide_tray(); + check("empty: no panel label (" + panelLabel() + ")", panelLabel() === ""); + // The heading is translated: assert it changes, not its wording. + let emptyHeading = heading(); + check("empty: the heading is not blank (" + emptyHeading + ")", emptyHeading.length > 0); + check("empty: the clear item is hidden", applet.clear_action.actor.visible === false); + check("empty: showEmptyTray keeps the applet on the panel", applet.actor.visible === true); + + fill(3); + check("3 listed: panel label is the count (" + panelLabel() + ")", panelLabel() === "3"); + check("3 listed: the heading has the count in it (" + heading() + ")", + heading().indexOf("3") !== -1); + check("3 listed: the heading changed from empty", heading() !== emptyHeading); + check("3 listed: the clear item is shown", applet.clear_action.actor.visible === true); + + applet.showNotificationCount = false; + applet.update_list(); + check("count off: the panel label is empty (" + panelLabel() + ")", panelLabel() === ""); + check("count off: the heading still has it (" + heading() + ")", + heading().indexOf("3") !== -1); + + applet.showNotificationCount = true; + applet.showEmptyTray = false; + applet._clear_all(); + applet._show_hide_tray(); + check("empty with showEmptyTray off: the applet leaves the panel", + applet.actor.visible === false); + + fill(1); + check("an arrival brings it back", applet.actor.visible === true); + } finally { + applet.showEmptyTray = originalEmpty; + applet.showNotificationCount = originalCount; + cleanup(); + // A blanket show() would strand an empty applet on the panel for the next check. + applet._show_hide_tray(); + } + + global.log("checkTrayChrome: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function checkSourceCascade() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkSourceCascade: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + try { + applet.menu.close(); + applet._clear_all(); + + let source = _newSource("Shared"); + for (let i = 0; i < 3; i++) + _notify(applet, source, "shared " + i); + check("one source holds all three (" + source.notifications.length + ")", + source.notifications.length === 3); + check("the applet lists all three (" + applet.notifications.length + ")", + applet.notifications.length === 3); + + source.destroy(); + check("destroying the source empties the applet (" + applet.notifications.length + ")", + applet.notifications.length === 0); + check("and empties the bin (" + _rowActors(applet).length + ")", + _rowActors(applet).length === 0); + + let capped = _newSource("Capped"); + for (let i = 0; i < 25; i++) + _notify(applet, capped, "capped " + i); + check("the source capped itself at 20 (" + capped.notifications.length + ")", + capped.notifications.length === 20); + check("the applet followed it down (" + applet.notifications.length + ")", + applet.notifications.length === 20); + } finally { + cleanup(); + } + + global.log("checkSourceCascade: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +function _pumpUntil(predicate, maxRounds) { + let ctx = GLib.MainContext.default(); + for (let i = 0; i < maxRounds; i++) { + if (predicate()) + return true; + GLib.usleep(2000); + while (ctx.iteration(false)) { /* drain everything ready right now */ } + } + return predicate(); +} + +function benchmark(count) { + let applet = _applet(); + let n = count || 100; + let ms = (start, end) => ((end - start) / 1000).toFixed(1); + let results = {}; + let original = applet.showNewestFirst; + try { + + let timed = (label, setup, action) => { + applet.menu.close(); + applet._clear_all(); + setup(); + let start = GLib.get_monotonic_time(); + action(); + results[label] = ms(start, GLib.get_monotonic_time()); + }; + + timed("arrive", () => applet._openMenu(), () => fill(n)); + + timed("clear", () => { fill(n); applet._openMenu(); }, + () => applet._clear_all()); + + timed("dismiss one", () => { fill(n); applet._openMenu(); }, + () => applet.notifications[Math.floor(n / 2)].destroy( + MessageTray.NotificationDestroyedReason.DISMISSED)); + + // Flip outside the timed region: the setter writes the settings file. + timed("reorder", () => { fill(n); applet._openMenu(); applet.showNewestFirst = !original; }, + () => applet.update_list()); + + timed("open the menu", () => fill(n), () => applet._openMenu()); + + // Reopening, where the rows have been built once already. "open the menu" above cannot + // show this: it only ever measures the first open. + timed("reopen the menu", () => { fill(n); applet._openMenu(); applet.menu.close(); }, + () => applet._openMenu()); + + } finally { + applet.showNewestFirst = original; + cleanup(); + } + for (let label in results) + log(`[testNotificationsApplet] ${n} notifications, ${label}: ${results[label]} ms`); + return results; +} From 718bfb60631c5a59340974e87567f3f158f3f5ff Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 2/8] notifications@cinnamon.org/applet.js: Destroy the menu when the applet is removed. on_applet_removed_from_panel() left the menu in the ui group, so every reload leaked another one, still drawable and still showing a stale count above buttons whose applet was gone. Test: checkMenuNotLeaked() --- .../notifications@cinnamon.org/applet.js | 4 ++ js/testing/testNotificationsApplet.js | 70 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index 361cbca58f..d1c8d8cd4c 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -71,6 +71,10 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this.settings.finalize(); this._crit_icon.destroy(); this._alt_crit_icon.destroy(); + + // Otherwise every reload leaks another menu, still drawable above a dead applet. + this.menuManager.removeMenu(this.menu); + this.menu.destroy(); } _openMenu() { diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index af6fd92b1d..5a8f76e476 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -662,3 +662,73 @@ function benchmark(count) { log(`[testNotificationsApplet] ${n} notifications, ${label}: ${results[label]} ms`); return results; } + +const Extension = imports.ui.extension; + +// AppletPopupMenu parents its actor into Main.uiGroup, so a menu that outlived its applet is a +// stray direct child of it. By actor identity, so the session language does not matter. +function _liveMenus(actors) { + let kids = Main.uiGroup.get_children(); + return actors.filter(actor => actor !== null && kids.indexOf(actor) !== -1); +} + +function _reloadAndWaitForApplet(maxRounds) { + Extension.reloadExtension(UUID, Extension.Type.APPLET); + let back = _pumpUntil(() => AppletManager.getRunningInstancesForUuid(UUID).length > 0, maxRounds); + return back ? _applet() : null; +} + +function checkMenuNotLeaked() { + let ok = true; + function check(label, condition) { + global.log("checkMenuNotLeaked: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + // One reload settles it. More are not more conclusive, and each tears the shell's applets + // down while this call is still pumping the main loop, which has killed the session. + const RELOADS = 1; + // About 10s of pumping, a bound rather than a wait; the reload usually lands in under 50ms. + const MAX_WAIT_ROUNDS = 5000; + + let applet = _applet(); + applet.menu.close(); + applet._clear_all(); + + // A reload must destroy the menu before it, so exactly one of these stays parented. + let seen = [applet.menu.actor]; + check("exactly one menu before reloading (" + _liveMenus(seen).length + ")", + _liveMenus(seen).length === 1); + + let allCameBack = true; + for (let i = 0; i < RELOADS; i++) { + let reloaded = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); + if (!reloaded) { + allCameBack = false; + check("the applet came back after reload " + (i + 1), false); + break; + } + applet = reloaded; + seen.push(applet.menu.actor); + } + check("the applet came back after reloading", allCameBack); + + // Exactly one, not "no more than before": zero is a failure this used to pass. + let live = _liveMenus(seen); + check("exactly one menu left after reloading (" + live.length + " of " + seen.length + + " seen)", live.length === 1); + check("the one left is the reloaded applet's own menu", + applet !== null && live.length === 1 && live[0] === applet.menu.actor); + + // Leave a working applet regardless of the outcome above. + if (!applet) + applet = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); + if (!applet) + global.log("checkMenuNotLeaked: FAIL the applet did not come back; the panel is missing it"); + else + try { cleanup(); } catch (e) { /* best effort */ } + + global.log("checkMenuNotLeaked: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} From 54fe4059288547bcab98af656080e07b1b6e63d8 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 3/8] notifications@cinnamon.org/applet.js: Cancel the critical blink timeout. critical_blink() threw away the timeout id, so nothing could cancel it. It kept ticking after the applet was removed, against icons already destroyed, and a second chain could start while the first was still queued. Test: checkCriticalBlink() --- .../notifications@cinnamon.org/applet.js | 21 ++++-- js/testing/testNotificationsApplet.js | 71 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index d1c8d8cd4c..4e38f54f8a 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -49,6 +49,7 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { // States this._blinking = false; this._blink_toggle = false; + this._blinkTimeoutId = 0; this._display(); } @@ -59,6 +60,7 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } on_applet_removed_from_panel () { + this._stop_blinking(); Main.keybindingManager.removeXletHotKey(this, "notification-open"); Main.keybindingManager.removeXletHotKey(this, "notification-clear"); @@ -209,23 +211,23 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } switch (max_urgency) { case Urgency.LOW: - this._blinking = false; + this._stop_blinking(); this.set_applet_icon_symbolic_name("low-notif"); break; case Urgency.NORMAL: case Urgency.HIGH: - this._blinking = false; + this._stop_blinking(); this.set_applet_icon_symbolic_name("normal-notif"); break; case Urgency.CRITICAL: - if (!this._blinking) { + if (!this._blinking && this._blinkTimeoutId === 0) { this._blinking = true; this.critical_blink(); } break; } } else { // There are no notifications. - this._blinking = false; + this._stop_blinking(); this.set_applet_label(''); this.set_applet_icon_symbolic_name("empty-notif"); this.clear_action.actor.hide(); @@ -325,7 +327,16 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } } + _stop_blinking () { + this._blinking = false; + if (this._blinkTimeoutId > 0) { + Mainloop.source_remove(this._blinkTimeoutId); + this._blinkTimeoutId = 0; + } + } + critical_blink () { + this._blinkTimeoutId = 0; if (!this._blinking) return; if (this._blink_toggle) { @@ -334,7 +345,7 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this._applet_icon_box.child = this._alt_crit_icon; } this._blink_toggle = !this._blink_toggle; - Mainloop.timeout_add_seconds(1, Lang.bind(this, this.critical_blink)); + this._blinkTimeoutId = Mainloop.timeout_add_seconds(1, Lang.bind(this, this.critical_blink)); } } diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index 5a8f76e476..1140e1c366 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -665,6 +665,77 @@ function benchmark(count) { const Extension = imports.ui.extension; +function _sourceIsLive(id) { + if (!id) + return false; + return GLib.MainContext.default().find_source_by_id(id) !== null; +} + +function _blinkNotify(applet, urgency) { + let source = new MessageTray.SystemNotificationSource(); + Main.messageTray.add(source); + sources.push(source); + let notification = new MessageTray.Notification(source, "blink test", "body"); + notification.setUrgency(urgency); + source.pushNotification(notification); + applet._notification_added(Main.messageTray, notification); + return notification; +} + +function checkCriticalBlink() { + let applet = _applet(); + if (!applet) { + global.log("checkCriticalBlink: applet not running"); + return false; + } + + let ok = true; + function check(label, condition) { + global.log("checkCriticalBlink: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + try { + + applet.menu.close(); + applet._clear_all(); + check("idle: no timeout armed", applet._blinkTimeoutId === 0); + + let critical = _blinkNotify(applet, MessageTray.Urgency.CRITICAL); + check("critical: blinking", applet._blinking === true); + check("critical: timeout armed", applet._blinkTimeoutId > 0); + + let armed = applet._blinkTimeoutId; + applet.update_list(); + check("re-entry: same timeout, not forked", applet._blinkTimeoutId === armed); + + check("critical: the source is really queued", _sourceIsLive(armed)); + + critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + _blinkNotify(applet, MessageTray.Urgency.NORMAL); + check("below critical: not blinking", applet._blinking === false); + check("below critical: timeout removed", applet._blinkTimeoutId === 0); + check("below critical: the source is really gone", !_sourceIsLive(armed)); + + _blinkNotify(applet, MessageTray.Urgency.CRITICAL); + let armedAgain = applet._blinkTimeoutId; + applet._clear_all(); + check("cleared: chain was running first", armedAgain > 0); + check("cleared: timeout removed", applet._blinkTimeoutId === 0); + check("cleared: the source is really gone", !_sourceIsLive(armedAgain)); + + // Removal is covered by checkSignalsDisconnected. Doing it here would trip on handlers a + // later commit disconnects, and report that as this check's own failure. + + } finally { + try { cleanup(); } catch (e) { /* best effort */ } + } + + global.log("checkCriticalBlink: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + // AppletPopupMenu parents its actor into Main.uiGroup, so a menu that outlived its applet is a // stray direct child of it. By actor identity, so the session language does not matter. function _liveMenus(actors) { From 7dc878b2c90c1af56f9f1da04319ec146839a4bb Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 4/8] notifications@cinnamon.org/applet.js: Disconnect notification signals when the applet is removed. 'scrolling-changed' and 'destroy' are connected on every notification through raw connect(), so disconnectAllSignals() never reaches them. A notification still listed at removal kept a closure holding the applet alive. Test: checkSignalsDisconnected() --- .../notifications@cinnamon.org/applet.js | 24 +++++++++- js/testing/testNotificationsApplet.js | 48 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index 4e38f54f8a..a980fbd08b 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -59,6 +59,18 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { Main.keybindingManager.addXletHotKey(this, "notification-clear", this.keyClear, Lang.bind(this, this._clear_all)); } + // Idempotent: three call sites can each get here first. + _disconnectNotificationSignals(notification) { + if (notification._appletScrollId) { + notification.disconnect(notification._appletScrollId); + notification._appletScrollId = 0; + } + if (notification._appletDestroyId) { + notification.disconnect(notification._appletDestroyId); + notification._appletDestroyId = 0; + } + } + on_applet_removed_from_panel () { this._stop_blinking(); Main.keybindingManager.removeXletHotKey(this, "notification-open"); @@ -69,6 +81,10 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this._clear_all(); } + // Whatever is still listed keeps its handlers otherwise, and the applet with them. + for (let n of this.notifications) + this._disconnectNotificationSignals(n); + this.signals.disconnectAllSignals(); this.settings.finalize(); this._crit_icon.destroy(); @@ -182,8 +198,12 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { notification.actor._parent_container = this._notificationbin; notification.actor.add_style_class_name('notification-applet-padding'); // Register for destruction. - notification.connect('scrolling-changed', (notif, scrolling) => { this.menu.passEvents = scrolling }); - notification.connect('destroy', () => { + // Ids kept on the notification: one that outlives this applet would otherwise keep a + // closure holding the applet alive. + notification._appletScrollId = notification.connect('scrolling-changed', + (notif, scrolling) => { this.menu.passEvents = scrolling }); + notification._appletDestroyId = notification.connect('destroy', () => { + this._disconnectNotificationSignals(notification); let i = this.notifications.indexOf(notification); if (i != -1) this.notifications.splice(i, 1); diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index 1140e1c366..8d67cb918a 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -736,6 +736,54 @@ function checkCriticalBlink() { return ok; } +function checkSignalsDisconnected() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkSignalsDisconnected: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + try { + applet.menu.close(); + applet._clear_all(); + fill(6); + + // Emitting 'destroy' on a listed notification drops it from the applet's list. The + // tray destroys them of its own accord, so always pick one listed right now. + let before = applet.notifications.length; + check("something is listed to begin with (" + before + ")", before > 0); + applet.notifications[0].emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); + check("the destroy handler is connected on arrival (" + before + " -> " + + applet.notifications.length + ")", applet.notifications.length === before - 1); + + // Reload rather than calling on_applet_removed_from_panel() by hand, so the manager + // tears it down the way a theme change would. Held above zero so the applet's decrement + // does not reach 0 and clear the tray, which is what would leave nothing to leak. + let counter = MessageTray.extensionsHandlingNotifications; + MessageTray.extensionsHandlingNotifications = 2; + _reloadAndWaitForApplet(5000); + MessageTray.extensionsHandlingNotifications = counter; + + let listed = applet.notifications.length; + check("removal left notifications listed, so there is something to leak (" + + listed + ")", listed > 0); + if (listed > 0) { + let target = applet.notifications[0]; + target.emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); + check("a listed notification no longer reaches the removed applet (" + listed + + " -> " + applet.notifications.length + ")", + applet.notifications.length === listed); + } + } finally { + try { cleanup(); } catch (e) { /* best effort */ } + } + + global.log("checkSignalsDisconnected: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + // AppletPopupMenu parents its actor into Main.uiGroup, so a menu that outlived its applet is a // stray direct child of it. By actor identity, so the session language does not matter. function _liveMenus(actors) { From eaf69f11181b719b71f15bd98dace6e0b59173e4 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 5/8] notifications@cinnamon.org/applet.js: Do not touch the actor of a destroyed notification. _notification_added() called actor.unparent() before checking _destroyed, and the tray hands a notification back after hiding its banner even when it was destroyed while shown. The actor is gone by then, so that was a critical on a disposed object. Test: checkDestroyedHandback() --- .../notifications@cinnamon.org/applet.js | 23 ++++--- js/testing/testNotificationsApplet.js | 60 +++++++++++++++++++ 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index a980fbd08b..ec48e833ce 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -175,20 +175,25 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { return; } + // The tray hands a notification back after hiding its banner even if it was + // destroyed meanwhile, and the actor is gone by then. + if (notification._destroyed) { + let destroyed_index = this.notifications.indexOf(notification); + if (destroyed_index != -1) { + this.notifications.splice(destroyed_index, 1); + this.update_list(); + } + return; + } + notification.actor.unparent(); let existing_index = this.notifications.indexOf(notification); if (existing_index != -1) { // This notification is already listed. - if (notification._destroyed) { - this.notifications.splice(existing_index, 1); - } else { - notification._inNotificationBin = true; - global.reparentActor(notification.actor, this._notificationbin); - notification._timeLabel.show(); - } + notification._inNotificationBin = true; + global.reparentActor(notification.actor, this._notificationbin); + notification._timeLabel.show(); this.update_list(); return; - } else if (notification._destroyed) { - return; } // Add notification to list. notification._inNotificationBin = true; diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index 8d67cb918a..05bcff690b 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -663,6 +663,66 @@ function benchmark(count) { return results; } +function checkDestroyedHandback() { + let applet = _applet(); + let tray = Main.messageTray; + let failures = 0; + + let handBack = (label, listed) => { + let source = new MessageTray.SystemNotificationSource("Test"); + Main.messageTray.add(source); + sources.push(source); + let notification = new MessageTray.Notification(source, "handback", "body"); + source.pushNotification(notification); + applet._notification_added(tray, notification); + if (!listed) { + let i = applet.notifications.indexOf(notification); + if (i !== -1) + applet.notifications.splice(i, 1); + } + + let realActor = notification.actor; + notification.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + if (listed && applet.notifications.indexOf(notification) === -1) + applet.notifications.push(notification); + + let touched = false; + Object.defineProperty(notification, "actor", { + configurable: true, + get: () => { touched = true; return realActor; } + }); + try { + applet._notification_added(tray, notification); + } finally { + delete notification.actor; + notification.actor = realActor; + } + + if (touched) { + failures++; + log(`[testNotificationsApplet] FAIL handback ${label}: the applet read the actor of a destroyed notification`); + } else if (applet.notifications.indexOf(notification) !== -1) { + failures++; + log(`[testNotificationsApplet] FAIL handback ${label}: a destroyed notification is still tracked`); + } else { + log(`[testNotificationsApplet] ok handback ${label}`); + } + }; + + try { + applet.menu.close(); + applet._clear_all(); + handBack("when the applet no longer tracks it", false); + applet._clear_all(); + handBack("when the applet still tracks it", true); + } finally { + cleanup(); + } + + log(`[testNotificationsApplet] checkDestroyedHandback: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; +} + const Extension = imports.ui.extension; function _sourceIsLive(id) { From 8b6d25f2cf7262456fb5b47fb744e7b7100fdea7 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 6/8] notifications@cinnamon.org/applet.js: Read the clock format once per update. timeify() constructed a Gio.Settings and read the clock format for every notification, so opening a tray of 200 built 200 of them. Test: checkClockSettings() --- .../notifications@cinnamon.org/applet.js | 23 +++++----- js/testing/testNotificationsApplet.js | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index ec48e833ce..f31dd91adf 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -24,6 +24,7 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { // Settings this.settings = new Settings.AppletSettings(this, metadata.uuid, instanceId); + this._interfaceSettings = new Gio.Settings({schema_id: 'org.cinnamon.desktop.interface'}); this.settings.bind("ignoreTransientNotifications", "ignoreTransientNotifications"); this.settings.bind("showEmptyTray", "showEmptyTray", this._show_hide_tray); this.settings.bind("keyOpen", "keyOpen", this._setKeybinding); @@ -343,12 +344,17 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { _update_timestamp() { let len = this.notifications.length; - if (len > 0) { - for (let i = 0; i < len; i++) { - let notification = this.notifications[i]; - let orig_time = notification._timestamp; - notification._timeLabel.clutter_text.set_markup(timeify(orig_time)); - } + if (len === 0) + return; + + // Read the clock format and the time once, not once per notification. + let use_24h = this._interfaceSettings.get_boolean('clock-use-24h'); + let now = new Date(); + + for (let i = 0; i < len; i++) { + let notification = this.notifications[i]; + notification._timeLabel.clutter_text.set_markup( + timeify(notification._timestamp, use_24h, now)); } } @@ -386,10 +392,7 @@ function stringify(count) { } } -function timeify(orig_time) { - let settings = new Gio.Settings({schema_id: 'org.cinnamon.desktop.interface'}); - let use_24h = settings.get_boolean('clock-use-24h'); - let now = new Date(); +function timeify(orig_time, use_24h, now) { let diff = Math.floor((now.getTime() - orig_time.getTime()) / 1000); // get diff in seconds let str; if (use_24h) { diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index 05bcff690b..dae97e3ced 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -911,3 +911,47 @@ function checkMenuNotLeaked() { global.log("checkMenuNotLeaked: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } + +// Counts Gio.Settings constructions by swapping in a wrapper across the one call: applet.js +// reads imports.gi.Gio.Settings at call time, so it sees it. +function checkClockSettings() { + let applet = _applet(); + const Gio = imports.gi.Gio; + let ok = true; + function check(label, condition) { + global.log("checkClockSettings: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + const ROWS = 12; + let real = Gio.Settings; + let built = 0; + + try { + applet.menu.close(); + applet._clear_all(); + fill(ROWS); + applet._openMenu(); + + Gio.Settings = function (params) { built++; return new real(params); }; + try { + applet._update_timestamp(); + } finally { + Gio.Settings = real; + } + + check("refreshing " + ROWS + " timestamps built no settings object (" + built + ")", + built === 0); + + // The timestamps still say something, so the count above is not zero by accident. + let blank = applet.notifications.filter(n => !n._timeLabel.get_text()).length; + check("every row still has timestamp text (" + blank + " blank)", blank === 0); + } finally { + Gio.Settings = real; + cleanup(); + } + + global.log("checkClockSettings: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} From 53c16b201abed71074db4936595be642a27cbad0 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:06:01 -0400 Subject: [PATCH 7/8] notifications@cinnamon.org/applet.js: Move notification actors instead of reparenting them. update_list() rebuilt the whole list on every change, so clearing N notifications reparented every remaining actor N times. Clearing and bursts of arrivals stop being quadratic in the size of the tray. Also fixes two errors. The applet's list and the bin's children are not the same set while the tray has borrowed an actor to show as a banner: _reorderNotifications() rebuilt the bin from the list, calling add_child() on an actor that already had a parent, and _clear_all() removed that actor from a container that was not its parent. Test: checkBorrowedActor() --- .../notifications@cinnamon.org/applet.js | 97 +- js/testing/testNotificationsApplet.js | 1028 +++++++++-------- 2 files changed, 632 insertions(+), 493 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index f31dd91adf..d0e2e4964e 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -199,10 +199,15 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { // Add notification to list. notification._inNotificationBin = true; this.notifications.push(notification); - // Steal the notification panel. - this._notificationbin.add(notification.actor); - notification.actor._parent_container = this._notificationbin; + // Steal the notification panel. Style before parenting: St skips the restyle + // for an unmapped actor. notification.actor.add_style_class_name('notification-applet-padding'); + // Insert where it belongs rather than appending and reordering after. + if (this.showNewestFirst) + this._notificationbin.insert_child_at_index(notification.actor, 0); + else + this._notificationbin.add(notification.actor); + notification.actor._parent_container = this._notificationbin; // Register for destruction. // Ids kept on the notification: one that outlives this applet would otherwise keep a // closure holding the applet alive. @@ -275,34 +280,81 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } _clear_all() { - let count = this.notifications.length; + // Iterate a snapshot: destroy() splices this.notifications from its own handler. + let list = this.notifications; + let count = list.length; + this.notifications = []; if (count > 0) { - for (let i = count-1; i >=0; i--) { - this._notificationbin.remove_actor(this.notifications[i].actor); - this.notifications[i].destroy(NotificationDestroyedReason.DISMISSED); + // Coalesce the first-child/last-child churn; each one restyles a subtree. + this._notificationbin.freeze_notify(); + try { + for (let i = count - 1; i >= 0; i--) { + // Unmap before destroying: tearing down an unmapped subtree skips + // the style work. Not ours while the tray shows it as a banner. + if (list[i].actor.get_parent() === this._notificationbin) + this._notificationbin.remove_actor(list[i].actor); + list[i].destroy(NotificationDestroyedReason.DISMISSED); + } + } finally { + this._notificationbin.thaw_notify(); + // If a destroy() threw, keep tracking whatever survived so a + // second clear can finish the job. Normally there are none. + let survivors = list.filter(n => !n._destroyed); + if (survivors.length > 0) + this.notifications = survivors.concat(this.notifications); } } - this.notifications = []; this.update_list(); } _reorderNotifications() { - let orderedNotifications = this.notifications.slice(); - - if (this.showNewestFirst) { - orderedNotifications.reverse(); + let bin = this._notificationbin; + let count = this.notifications.length; + let idx = this.showNewestFirst ? count - 1 : 0; + let step = this.showNewestFirst ? -1 : 1; + + // If everything is already in the wanted order, drop what we no longer track + // and skip the moves. That covers arrivals and dismissals, nearly every call. + let child = bin.get_first_child(); + let extras = []; + let ordered = true; + for (let i = 0, j = idx; i < count; i++, j += step) { + let want = this.notifications[j].actor; + while (child !== null && child !== want) { + extras.push(child); + child = child.get_next_sibling(); + } + if (child === null) { + ordered = false; + break; + } + child = child.get_next_sibling(); } - - // Remove all children without destroying them. - let children = this._notificationbin.get_children(); - for (let i = 0; i < children.length; i++) { - this._notificationbin.remove_child(children[i]); + if (ordered) { + while (child !== null) { + extras.push(child); + child = child.get_next_sibling(); + } + for (let i = 0; i < extras.length; i++) + bin.remove_child(extras[i]); + return; } - // Add them back in desired order. - for (let i = 0; i < orderedNotifications.length; i++) { - this._notificationbin.add_child(orderedNotifications[i].actor); + // Moving never unparents, so St skips the subtree restyle a re-add would cost. + // Count what is placed: a notification the tray has taken has no actor here. + let placed = 0; + for (let i = 0; i < count; i++, idx += step) { + let actor = this.notifications[idx].actor; + let parent = actor.get_parent(); + if (parent === bin) + bin.set_child_at_index(actor, placed++); + else if (parent === null) + bin.insert_child_at_index(actor, placed++); } + + // Anything past what we placed is a child we no longer track. + for (let i = bin.get_n_children() - 1; i >= placed; i--) + bin.remove_child(bin.get_child_at_index(i)); } _show_hide_tray() { // Show or hide the notification tray. @@ -353,8 +405,9 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { for (let i = 0; i < len; i++) { let notification = this.notifications[i]; - notification._timeLabel.clutter_text.set_markup( - timeify(notification._timestamp, use_24h, now)); + // set_text skips unchanged strings, which is most of them once the + // relative suffix stops changing after an hour. + notification._timeLabel.set_text(timeify(notification._timestamp, use_24h, now)); } } diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index dae97e3ced..b42d4217f9 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -55,22 +55,7 @@ function cleanup() { log(`[testNotificationsApplet] cleaned up, ${applet.notifications.length} left`); } -function _newSource(title) { - let source = new MessageTray.SystemNotificationSource(title || "Test"); - Main.messageTray.add(source); - sources.push(source); - return source; -} - -function _notify(applet, source, title, urgency) { - let notification = new MessageTray.Notification(source, title, "body"); - if (urgency !== undefined) - notification.setUrgency(urgency); - source.pushNotification(notification); - applet._notification_added(Main.messageTray, notification); - return notification; -} - +// Bin children that are real notification rows, in display order. Every child is one. function _rowActors(applet) { return applet._notificationbin.get_children(); } @@ -223,99 +208,64 @@ function checkBannerHandover() { return failures === 0; } -function checkHandback() { +function checkDestroyedHandback() { let applet = _applet(); let tray = Main.messageTray; - let ok = true; - function check(label, condition) { - global.log("checkHandback: " + label + ": " + (condition ? "ok" : "FAIL")); - if (!condition) - ok = false; - } - - try { - applet.menu.close(); - applet._clear_all(); - fill(4); - applet._openMenu(); - - let listed = applet.notifications.slice(); - let borrowed = listed[1]; - let wasAt = _rowActors(applet).indexOf(borrowed.actor); - - tray._notificationQueue.push(borrowed); - tray._showNotification(); - // Hiding first makes the tray throw: _showNotificationCompleted() reads - // this._notification without a null check. See docs/issues/messagetray-issues.md. - _pumpUntil(() => tray._notificationState === 2, 200); - check("the tray took the actor", borrowed.actor.get_parent() !== applet._notificationbin); + let failures = 0; - tray._hideNotificationCompleted(); + let handBack = (label, listed) => { + let source = new MessageTray.SystemNotificationSource("Test"); + Main.messageTray.add(source); + sources.push(source); + let notification = new MessageTray.Notification(source, "handback", "body"); + source.pushNotification(notification); + applet._notification_added(tray, notification); + if (!listed) { + let i = applet.notifications.indexOf(notification); + if (i !== -1) + applet.notifications.splice(i, 1); + } - check("the actor came back to the bin", - borrowed.actor.get_parent() === applet._notificationbin); - check("it is marked as living in the bin again", borrowed._inNotificationBin === true); - check("its timestamp is visible again", borrowed._timeLabel.visible === true); - check("the list is unchanged (" + applet.notifications.length + " of " + - listed.length + ")", applet.notifications.length === listed.length); - check("it is back at the same position (" + wasAt + ")", - _rowActors(applet).indexOf(borrowed.actor) === wasAt); - } finally { - // Never hide mid-show: the completion callback reads this._notification, which hiding - // sets to null. State.SHOWING is 1. - _pumpUntil(() => tray._notificationState !== 1, 300); - try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } - // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires - // inside a later check, against a notification already destroyed. - try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } - cleanup(); - } + let realActor = notification.actor; + notification.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + if (listed && applet.notifications.indexOf(notification) === -1) + applet.notifications.push(notification); - global.log("checkHandback: " + (ok ? "all checks passed" : "FAILURES above")); - return ok; -} + let touched = false; + Object.defineProperty(notification, "actor", { + configurable: true, + get: () => { touched = true; return realActor; } + }); + try { + applet._notification_added(tray, notification); + } finally { + delete notification.actor; + notification.actor = realActor; + } -function checkRevisedWhileShowing() { - let applet = _applet(); - let tray = Main.messageTray; - let ok = true; - function check(label, condition) { - global.log("checkRevisedWhileShowing: " + label + ": " + (condition ? "ok" : "FAIL")); - if (!condition) - ok = false; - } + if (touched) { + failures++; + log(`[testNotificationsApplet] FAIL handback ${label}: the applet read the actor of a destroyed notification`); + } else if (applet.notifications.indexOf(notification) !== -1) { + failures++; + log(`[testNotificationsApplet] FAIL handback ${label}: a destroyed notification is still tracked`); + } else { + log(`[testNotificationsApplet] ok handback ${label}`); + } + }; try { applet.menu.close(); applet._clear_all(); - fill(3); - applet._openMenu(); - - let borrowed = applet.notifications[1]; - tray._notificationQueue.push(borrowed); - tray._showNotification(); - _pumpUntil(() => tray._notificationState === 2, 200); - - borrowed.update("revised title", "revised body"); - check("the revision cleared the in-bin flag", borrowed._inNotificationBin === false); - - tray._hideNotificationCompleted(); - - check("the actor came back", borrowed.actor.get_parent() === applet._notificationbin); - check("the in-bin flag was restored", borrowed._inNotificationBin === true); - check("the timestamp is visible again", borrowed._timeLabel.visible === true); - check("the list did not grow (" + applet.notifications.length + ")", - applet.notifications.length === 3); + handBack("when the applet no longer tracks it", false); + applet._clear_all(); + handBack("when the applet still tracks it", true); } finally { - try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } - // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires - // inside a later check, against a notification already destroyed. - try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } cleanup(); } - global.log("checkRevisedWhileShowing: " + (ok ? "all checks passed" : "FAILURES above")); - return ok; + log(`[testNotificationsApplet] checkDestroyedHandback: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; } function checkFailedClear() { @@ -368,207 +318,332 @@ function checkFailedClear() { return failures === 0; } -function checkUrgency() { - let applet = _applet(); - let ok = true; - function check(label, condition) { - global.log("checkUrgency: " + label + ": " + (condition ? "ok" : "FAIL")); - if (!condition) - ok = false; +function _pumpUntil(predicate, maxRounds) { + let ctx = GLib.MainContext.default(); + for (let i = 0; i < maxRounds; i++) { + if (predicate()) + return true; + GLib.usleep(2000); + while (ctx.iteration(false)) { /* drain everything ready right now */ } } - let iconName = () => applet._applet_icon.get_icon_name(); + return predicate(); +} +function benchmark(count) { + let applet = _applet(); + let n = count || 100; + let ms = (start, end) => ((end - start) / 1000).toFixed(1); + let results = {}; + let original = applet.showNewestFirst; try { + + let timed = (label, setup, action) => { applet.menu.close(); applet._clear_all(); - check("empty: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); - check("empty: not blinking", applet._blinking === false); + setup(); + let start = GLib.get_monotonic_time(); + action(); + results[label] = ms(start, GLib.get_monotonic_time()); + }; - let source = _newSource(); - _notify(applet, source, "low", MessageTray.Urgency.LOW); - check("low: icon is low-notif (" + iconName() + ")", iconName() === "low-notif"); + timed("arrive", () => applet._openMenu(), () => fill(n)); - _notify(applet, source, "normal", MessageTray.Urgency.NORMAL); - check("normal outranks low (" + iconName() + ")", iconName() === "normal-notif"); - check("normal: not blinking", applet._blinking === false); + timed("clear", () => { fill(n); applet._openMenu(); }, + () => applet._clear_all()); - let critical = _notify(applet, source, "critical", MessageTray.Urgency.CRITICAL); - check("critical: blinking", applet._blinking === true); + timed("dismiss one", () => { fill(n); applet._openMenu(); }, + () => applet.notifications[Math.floor(n / 2)].destroy( + MessageTray.NotificationDestroyedReason.DISMISSED)); - // Nothing stops a caller raising urgency after listing, and the icon has to follow. - critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); - let late = _notify(applet, source, "late"); - late.setUrgency(MessageTray.Urgency.CRITICAL); - applet.update_list(); - check("urgency raised after arrival: blinking", applet._blinking === true); - late.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + // Flip outside the timed region: the setter writes the settings file. + timed("reorder", () => { fill(n); applet._openMenu(); applet.showNewestFirst = !original; }, + () => applet.update_list()); - check("after the criticals go: not blinking", applet._blinking === false); - check("after the critical goes: icon is normal-notif (" + iconName() + ")", - iconName() === "normal-notif"); + timed("open the menu", () => fill(n), () => applet._openMenu()); + + // Reopening, where the rows have been built once already. "open the menu" above cannot + // show this: it only ever measures the first open. + timed("reopen the menu", () => { fill(n); applet._openMenu(); applet.menu.close(); }, + () => applet._openMenu()); - applet._clear_all(); - check("cleared: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); - check("cleared: not blinking", applet._blinking === false); } finally { + applet.showNewestFirst = original; cleanup(); } + for (let label in results) + log(`[testNotificationsApplet] ${n} notifications, ${label}: ${results[label]} ms`); + return results; +} - global.log("checkUrgency: " + (ok ? "all checks passed" : "FAILURES above")); - return ok; +const Extension = imports.ui.extension; + +function _sourceIsLive(id) { + if (!id) + return false; + return GLib.MainContext.default().find_source_by_id(id) !== null; } -function checkArrivalWhileOpen() { +function _blinkNotify(applet, urgency) { + let source = new MessageTray.SystemNotificationSource(); + Main.messageTray.add(source); + sources.push(source); + let notification = new MessageTray.Notification(source, "blink test", "body"); + notification.setUrgency(urgency); + source.pushNotification(notification); + applet._notification_added(Main.messageTray, notification); + return notification; +} + +function checkCriticalBlink() { let applet = _applet(); + if (!applet) { + global.log("checkCriticalBlink: applet not running"); + return false; + } + let ok = true; function check(label, condition) { - global.log("checkArrivalWhileOpen: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkCriticalBlink: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } - let original = applet.showNewestFirst; try { - for (let newestFirst of [false, true]) { - applet.menu.close(); - applet._clear_all(); - applet.showNewestFirst = newestFirst; - fill(3); - applet._openMenu(); - let source = _newSource(); - let arrived = _notify(applet, source, "arrived while open"); + applet.menu.close(); + applet._clear_all(); + check("idle: no timeout armed", applet._blinkTimeoutId === 0); + + let critical = _blinkNotify(applet, MessageTray.Urgency.CRITICAL); + check("critical: blinking", applet._blinking === true); + check("critical: timeout armed", applet._blinkTimeoutId > 0); + + let armed = applet._blinkTimeoutId; + applet.update_list(); + check("re-entry: same timeout, not forked", applet._blinkTimeoutId === armed); + + check("critical: the source is really queued", _sourceIsLive(armed)); + + critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + _blinkNotify(applet, MessageTray.Urgency.NORMAL); + check("below critical: not blinking", applet._blinking === false); + check("below critical: timeout removed", applet._blinkTimeoutId === 0); + check("below critical: the source is really gone", !_sourceIsLive(armed)); + + _blinkNotify(applet, MessageTray.Urgency.CRITICAL); + let armedAgain = applet._blinkTimeoutId; + applet._clear_all(); + check("cleared: chain was running first", armedAgain > 0); + check("cleared: timeout removed", applet._blinkTimeoutId === 0); + check("cleared: the source is really gone", !_sourceIsLive(armedAgain)); + + // Removal is covered by checkSignalsDisconnected. Doing it here would trip on handlers a + // later commit disconnects, and report that as this check's own failure. - let rows = _rowActors(applet); - check("newestFirst=" + newestFirst + ": it is listed (" + - applet.notifications.length + ")", applet.notifications.length === 4); - check("newestFirst=" + newestFirst + ": its actor is in the bin", - rows.indexOf(arrived.actor) !== -1); - check("newestFirst=" + newestFirst + ": it is at the end the setting asks for", - rows.indexOf(arrived.actor) === (newestFirst ? 0 : rows.length - 1)); - // The heading is translated: look for the number, not the wording. - check("newestFirst=" + newestFirst + ": the heading followed (" + - applet.menu_label.label.get_text() + ")", - applet.menu_label.label.get_text().indexOf("4") !== -1); - } } finally { - applet.showNewestFirst = original; - cleanup(); + try { cleanup(); } catch (e) { /* best effort */ } } - global.log("checkArrivalWhileOpen: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkCriticalBlink: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -function checkTransient() { +// AppletPopupMenu parents its actor into Main.uiGroup, so a menu that outlived its applet is a +// stray direct child of it. By actor identity, so the session language does not matter. +function _liveMenus(actors) { + let kids = Main.uiGroup.get_children(); + return actors.filter(actor => actor !== null && kids.indexOf(actor) !== -1); +} + +function _reloadAndWaitForApplet(maxRounds) { + Extension.reloadExtension(UUID, Extension.Type.APPLET); + let back = _pumpUntil(() => AppletManager.getRunningInstancesForUuid(UUID).length > 0, maxRounds); + return back ? _applet() : null; +} + +function checkMenuNotLeaked() { + let ok = true; + function check(label, condition) { + global.log("checkMenuNotLeaked: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + // One reload settles it. More are not more conclusive, and each tears the shell's applets + // down while this call is still pumping the main loop, which has killed the session. + const RELOADS = 1; + // About 10s of pumping, a bound rather than a wait; the reload usually lands in under 50ms. + const MAX_WAIT_ROUNDS = 5000; + + let applet = _applet(); + applet.menu.close(); + applet._clear_all(); + + // A reload must destroy the menu before it, so exactly one of these stays parented. + let seen = [applet.menu.actor]; + check("exactly one menu before reloading (" + _liveMenus(seen).length + ")", + _liveMenus(seen).length === 1); + + let allCameBack = true; + for (let i = 0; i < RELOADS; i++) { + let reloaded = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); + if (!reloaded) { + allCameBack = false; + check("the applet came back after reload " + (i + 1), false); + break; + } + applet = reloaded; + seen.push(applet.menu.actor); + } + check("the applet came back after reloading", allCameBack); + + // Exactly one, not "no more than before": zero is a failure this used to pass. + let live = _liveMenus(seen); + check("exactly one menu left after reloading (" + live.length + " of " + seen.length + + " seen)", live.length === 1); + check("the one left is the reloaded applet's own menu", + applet !== null && live.length === 1 && live[0] === applet.menu.actor); + + // Leave a working applet regardless of the outcome above. + if (!applet) + applet = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); + if (!applet) + global.log("checkMenuNotLeaked: FAIL the applet did not come back; the panel is missing it"); + else + try { cleanup(); } catch (e) { /* best effort */ } + + global.log("checkMenuNotLeaked: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} + +// Connected through raw connect(), not this.signals, so disconnectAllSignals() never touches +// them. What leaks is whatever is still listed when the applet is removed. +function checkSignalsDisconnected() { let applet = _applet(); let ok = true; function check(label, condition) { - global.log("checkTransient: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkSignalsDisconnected: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } - let original = applet.ignoreTransientNotifications; try { applet.menu.close(); applet._clear_all(); + fill(6); - applet.ignoreTransientNotifications = false; - let kept = new MessageTray.Notification(_newSource(), "transient", "body"); - kept.setTransient(true); - applet._notification_added(Main.messageTray, kept); - check("setting off: a transient is listed (" + applet.notifications.length + ")", - applet.notifications.indexOf(kept) !== -1); + // Emitting 'destroy' on a listed notification drops it from the applet's list. The + // tray destroys them of its own accord, so always pick one listed right now. + let before = applet.notifications.length; + check("something is listed to begin with (" + before + ")", before > 0); + applet.notifications[0].emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); + check("the destroy handler is connected on arrival (" + before + " -> " + + applet.notifications.length + ")", applet.notifications.length === before - 1); - applet._clear_all(); - applet.ignoreTransientNotifications = true; - let dropped = new MessageTray.Notification(_newSource(), "transient", "body"); - dropped.setTransient(true); - applet._notification_added(Main.messageTray, dropped); - check("setting on: a transient is not listed (" + applet.notifications.length + ")", - applet.notifications.indexOf(dropped) === -1); - check("setting on: a transient was destroyed", dropped._destroyed === true); + // Reload rather than calling on_applet_removed_from_panel() by hand, so the manager + // tears it down the way a theme change would. Held above zero so the applet's decrement + // does not reach 0 and clear the tray, which is what would leave nothing to leak. + let counter = MessageTray.extensionsHandlingNotifications; + MessageTray.extensionsHandlingNotifications = 2; + _reloadAndWaitForApplet(5000); + MessageTray.extensionsHandlingNotifications = counter; - let normal = _notify(applet, _newSource(), "normal"); - check("setting on: a normal notification is still listed", - applet.notifications.indexOf(normal) !== -1); + let listed = applet.notifications.length; + check("removal left notifications listed, so there is something to leak (" + + listed + ")", listed > 0); + if (listed > 0) { + let target = applet.notifications[0]; + target.emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); + check("a listed notification no longer reaches the removed applet (" + listed + + " -> " + applet.notifications.length + ")", + applet.notifications.length === listed); + } } finally { - applet.ignoreTransientNotifications = original; - cleanup(); + try { cleanup(); } catch (e) { /* best effort */ } } - global.log("checkTransient: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkSignalsDisconnected: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -function checkTrayChrome() { +// Urgency is set before the applet sees it, as notificationDaemon.js does. +function _notify(applet, source, title, urgency) { + let notification = new MessageTray.Notification(source, title, "body"); + if (urgency !== undefined) + notification.setUrgency(urgency); + source.pushNotification(notification); + applet._notification_added(Main.messageTray, notification); + return notification; +} + +function _newSource(title) { + let source = new MessageTray.SystemNotificationSource(title || "Test"); + Main.messageTray.add(source); + sources.push(source); + return source; +} + +// _hideNotificationCompleted() is called directly: the real cycle ends in a Clutter callback +// a synchronous D-Bus call cannot wait for. +function checkHandback() { let applet = _applet(); + let tray = Main.messageTray; let ok = true; function check(label, condition) { - global.log("checkTrayChrome: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkHandback: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } - let panelLabel = () => applet._applet_label.get_text(); - let heading = () => applet.menu_label.label.get_text(); - let originalEmpty = applet.showEmptyTray; - let originalCount = applet.showNotificationCount; try { - // Assigning these directly skips the settings binding, so call what it would have called. - applet.showEmptyTray = true; - applet.showNotificationCount = true; - applet.menu.close(); applet._clear_all(); - applet._show_hide_tray(); - check("empty: no panel label (" + panelLabel() + ")", panelLabel() === ""); - // The heading is translated: assert it changes, not its wording. - let emptyHeading = heading(); - check("empty: the heading is not blank (" + emptyHeading + ")", emptyHeading.length > 0); - check("empty: the clear item is hidden", applet.clear_action.actor.visible === false); - check("empty: showEmptyTray keeps the applet on the panel", applet.actor.visible === true); + fill(4); + applet._openMenu(); - fill(3); - check("3 listed: panel label is the count (" + panelLabel() + ")", panelLabel() === "3"); - check("3 listed: the heading has the count in it (" + heading() + ")", - heading().indexOf("3") !== -1); - check("3 listed: the heading changed from empty", heading() !== emptyHeading); - check("3 listed: the clear item is shown", applet.clear_action.actor.visible === true); + let listed = applet.notifications.slice(); + let borrowed = listed[1]; + let wasAt = _rowActors(applet).indexOf(borrowed.actor); - applet.showNotificationCount = false; - applet.update_list(); - check("count off: the panel label is empty (" + panelLabel() + ")", panelLabel() === ""); - check("count off: the heading still has it (" + heading() + ")", - heading().indexOf("3") !== -1); + tray._notificationQueue.push(borrowed); + tray._showNotification(); + // Hiding first makes the tray throw: _showNotificationCompleted() reads + // this._notification without a null check. See docs/issues/messagetray-issues.md. + _pumpUntil(() => tray._notificationState === 2, 200); + check("the tray took the actor", borrowed.actor.get_parent() !== applet._notificationbin); - applet.showNotificationCount = true; - applet.showEmptyTray = false; - applet._clear_all(); - applet._show_hide_tray(); - check("empty with showEmptyTray off: the applet leaves the panel", - applet.actor.visible === false); + tray._hideNotificationCompleted(); - fill(1); - check("an arrival brings it back", applet.actor.visible === true); + check("the actor came back to the bin", + borrowed.actor.get_parent() === applet._notificationbin); + check("it is marked as living in the bin again", borrowed._inNotificationBin === true); + check("its timestamp is visible again", borrowed._timeLabel.visible === true); + check("the list is unchanged (" + applet.notifications.length + " of " + + listed.length + ")", applet.notifications.length === listed.length); + check("it is back at the same position (" + wasAt + ")", + _rowActors(applet).indexOf(borrowed.actor) === wasAt); } finally { - applet.showEmptyTray = originalEmpty; - applet.showNotificationCount = originalCount; + // Never hide mid-show: the completion callback reads this._notification, which hiding + // sets to null. State.SHOWING is 1. + _pumpUntil(() => tray._notificationState !== 1, 300); + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires + // inside a later check, against a notification already destroyed. + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } cleanup(); - // A blanket show() would strand an empty applet on the panel for the next check. - applet._show_hide_tray(); } - global.log("checkTrayChrome: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkHandback: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -function checkSourceCascade() { +// A revision while the banner is up clears _inNotificationBin and hides the timestamp. +function checkRevisedWhileShowing() { let applet = _applet(); + let tray = Main.messageTray; let ok = true; function check(label, condition) { - global.log("checkSourceCascade: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkRevisedWhileShowing: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } @@ -576,339 +651,276 @@ function checkSourceCascade() { try { applet.menu.close(); applet._clear_all(); + fill(3); + applet._openMenu(); - let source = _newSource("Shared"); - for (let i = 0; i < 3; i++) - _notify(applet, source, "shared " + i); - check("one source holds all three (" + source.notifications.length + ")", - source.notifications.length === 3); - check("the applet lists all three (" + applet.notifications.length + ")", - applet.notifications.length === 3); + let borrowed = applet.notifications[1]; + tray._notificationQueue.push(borrowed); + tray._showNotification(); + _pumpUntil(() => tray._notificationState === 2, 200); - source.destroy(); - check("destroying the source empties the applet (" + applet.notifications.length + ")", - applet.notifications.length === 0); - check("and empties the bin (" + _rowActors(applet).length + ")", - _rowActors(applet).length === 0); + borrowed.update("revised title", "revised body"); + check("the revision cleared the in-bin flag", borrowed._inNotificationBin === false); - let capped = _newSource("Capped"); - for (let i = 0; i < 25; i++) - _notify(applet, capped, "capped " + i); - check("the source capped itself at 20 (" + capped.notifications.length + ")", - capped.notifications.length === 20); - check("the applet followed it down (" + applet.notifications.length + ")", - applet.notifications.length === 20); + tray._hideNotificationCompleted(); + + check("the actor came back", borrowed.actor.get_parent() === applet._notificationbin); + check("the in-bin flag was restored", borrowed._inNotificationBin === true); + check("the timestamp is visible again", borrowed._timeLabel.visible === true); + check("the list did not grow (" + applet.notifications.length + ")", + applet.notifications.length === 3); } finally { + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + // The dwell timeout outlives _hideNotificationCompleted(); left armed it fires + // inside a later check, against a notification already destroyed. + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } cleanup(); } - global.log("checkSourceCascade: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkRevisedWhileShowing: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -function _pumpUntil(predicate, maxRounds) { - let ctx = GLib.MainContext.default(); - for (let i = 0; i < maxRounds; i++) { - if (predicate()) - return true; - GLib.usleep(2000); - while (ctx.iteration(false)) { /* drain everything ready right now */ } +// The highest urgency in the list picks the panel icon, and a critical one starts the blink. +function checkUrgency() { + let applet = _applet(); + let ok = true; + function check(label, condition) { + global.log("checkUrgency: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; } - return predicate(); -} + let iconName = () => applet._applet_icon.get_icon_name(); -function benchmark(count) { - let applet = _applet(); - let n = count || 100; - let ms = (start, end) => ((end - start) / 1000).toFixed(1); - let results = {}; - let original = applet.showNewestFirst; try { - - let timed = (label, setup, action) => { applet.menu.close(); applet._clear_all(); - setup(); - let start = GLib.get_monotonic_time(); - action(); - results[label] = ms(start, GLib.get_monotonic_time()); - }; - - timed("arrive", () => applet._openMenu(), () => fill(n)); + check("empty: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); + check("empty: not blinking", applet._blinking === false); - timed("clear", () => { fill(n); applet._openMenu(); }, - () => applet._clear_all()); + let source = _newSource(); + _notify(applet, source, "low", MessageTray.Urgency.LOW); + check("low: icon is low-notif (" + iconName() + ")", iconName() === "low-notif"); - timed("dismiss one", () => { fill(n); applet._openMenu(); }, - () => applet.notifications[Math.floor(n / 2)].destroy( - MessageTray.NotificationDestroyedReason.DISMISSED)); + _notify(applet, source, "normal", MessageTray.Urgency.NORMAL); + check("normal outranks low (" + iconName() + ")", iconName() === "normal-notif"); + check("normal: not blinking", applet._blinking === false); - // Flip outside the timed region: the setter writes the settings file. - timed("reorder", () => { fill(n); applet._openMenu(); applet.showNewestFirst = !original; }, - () => applet.update_list()); + let critical = _notify(applet, source, "critical", MessageTray.Urgency.CRITICAL); + check("critical: blinking", applet._blinking === true); - timed("open the menu", () => fill(n), () => applet._openMenu()); + // Nothing stops a caller raising urgency after listing, and the icon has to follow. + critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + let late = _notify(applet, source, "late"); + late.setUrgency(MessageTray.Urgency.CRITICAL); + applet.update_list(); + check("urgency raised after arrival: blinking", applet._blinking === true); + late.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); - // Reopening, where the rows have been built once already. "open the menu" above cannot - // show this: it only ever measures the first open. - timed("reopen the menu", () => { fill(n); applet._openMenu(); applet.menu.close(); }, - () => applet._openMenu()); + check("after the criticals go: not blinking", applet._blinking === false); + check("after the critical goes: icon is normal-notif (" + iconName() + ")", + iconName() === "normal-notif"); + applet._clear_all(); + check("cleared: icon is empty-notif (" + iconName() + ")", iconName() === "empty-notif"); + check("cleared: not blinking", applet._blinking === false); } finally { - applet.showNewestFirst = original; cleanup(); } - for (let label in results) - log(`[testNotificationsApplet] ${n} notifications, ${label}: ${results[label]} ms`); - return results; + + global.log("checkUrgency: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; } -function checkDestroyedHandback() { +// A notification arriving while the menu is open has to land in the bin straight away. +function checkArrivalWhileOpen() { let applet = _applet(); - let tray = Main.messageTray; - let failures = 0; - - let handBack = (label, listed) => { - let source = new MessageTray.SystemNotificationSource("Test"); - Main.messageTray.add(source); - sources.push(source); - let notification = new MessageTray.Notification(source, "handback", "body"); - source.pushNotification(notification); - applet._notification_added(tray, notification); - if (!listed) { - let i = applet.notifications.indexOf(notification); - if (i !== -1) - applet.notifications.splice(i, 1); - } + let ok = true; + function check(label, condition) { + global.log("checkArrivalWhileOpen: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } - let realActor = notification.actor; - notification.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); - if (listed && applet.notifications.indexOf(notification) === -1) - applet.notifications.push(notification); + let original = applet.showNewestFirst; + try { + for (let newestFirst of [false, true]) { + applet.menu.close(); + applet._clear_all(); + applet.showNewestFirst = newestFirst; + fill(3); + applet._openMenu(); - let touched = false; - Object.defineProperty(notification, "actor", { - configurable: true, - get: () => { touched = true; return realActor; } - }); - try { - applet._notification_added(tray, notification); - } finally { - delete notification.actor; - notification.actor = realActor; - } + let source = _newSource(); + let arrived = _notify(applet, source, "arrived while open"); - if (touched) { - failures++; - log(`[testNotificationsApplet] FAIL handback ${label}: the applet read the actor of a destroyed notification`); - } else if (applet.notifications.indexOf(notification) !== -1) { - failures++; - log(`[testNotificationsApplet] FAIL handback ${label}: a destroyed notification is still tracked`); - } else { - log(`[testNotificationsApplet] ok handback ${label}`); + let rows = _rowActors(applet); + check("newestFirst=" + newestFirst + ": it is listed (" + + applet.notifications.length + ")", applet.notifications.length === 4); + check("newestFirst=" + newestFirst + ": its actor is in the bin", + rows.indexOf(arrived.actor) !== -1); + check("newestFirst=" + newestFirst + ": it is at the end the setting asks for", + rows.indexOf(arrived.actor) === (newestFirst ? 0 : rows.length - 1)); + // The heading is translated: look for the number, not the wording. + check("newestFirst=" + newestFirst + ": the heading followed (" + + applet.menu_label.label.get_text() + ")", + applet.menu_label.label.get_text().indexOf("4") !== -1); } - }; - - try { - applet.menu.close(); - applet._clear_all(); - handBack("when the applet no longer tracks it", false); - applet._clear_all(); - handBack("when the applet still tracks it", true); } finally { + applet.showNewestFirst = original; cleanup(); - } - - log(`[testNotificationsApplet] checkDestroyedHandback: ${failures === 0 ? "passed" : failures + " failures"}`); - return failures === 0; -} - -const Extension = imports.ui.extension; - -function _sourceIsLive(id) { - if (!id) - return false; - return GLib.MainContext.default().find_source_by_id(id) !== null; -} - -function _blinkNotify(applet, urgency) { - let source = new MessageTray.SystemNotificationSource(); - Main.messageTray.add(source); - sources.push(source); - let notification = new MessageTray.Notification(source, "blink test", "body"); - notification.setUrgency(urgency); - source.pushNotification(notification); - applet._notification_added(Main.messageTray, notification); - return notification; + } + + global.log("checkArrivalWhileOpen: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; } -function checkCriticalBlink() { +// With the setting on, a transient notification is destroyed instead of listed. +function checkTransient() { let applet = _applet(); - if (!applet) { - global.log("checkCriticalBlink: applet not running"); - return false; - } - let ok = true; function check(label, condition) { - global.log("checkCriticalBlink: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkTransient: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } + let original = applet.ignoreTransientNotifications; try { + applet.menu.close(); + applet._clear_all(); - applet.menu.close(); - applet._clear_all(); - check("idle: no timeout armed", applet._blinkTimeoutId === 0); - - let critical = _blinkNotify(applet, MessageTray.Urgency.CRITICAL); - check("critical: blinking", applet._blinking === true); - check("critical: timeout armed", applet._blinkTimeoutId > 0); - - let armed = applet._blinkTimeoutId; - applet.update_list(); - check("re-entry: same timeout, not forked", applet._blinkTimeoutId === armed); - - check("critical: the source is really queued", _sourceIsLive(armed)); - - critical.destroy(MessageTray.NotificationDestroyedReason.DISMISSED); - _blinkNotify(applet, MessageTray.Urgency.NORMAL); - check("below critical: not blinking", applet._blinking === false); - check("below critical: timeout removed", applet._blinkTimeoutId === 0); - check("below critical: the source is really gone", !_sourceIsLive(armed)); - - _blinkNotify(applet, MessageTray.Urgency.CRITICAL); - let armedAgain = applet._blinkTimeoutId; - applet._clear_all(); - check("cleared: chain was running first", armedAgain > 0); - check("cleared: timeout removed", applet._blinkTimeoutId === 0); - check("cleared: the source is really gone", !_sourceIsLive(armedAgain)); + applet.ignoreTransientNotifications = false; + let kept = new MessageTray.Notification(_newSource(), "transient", "body"); + kept.setTransient(true); + applet._notification_added(Main.messageTray, kept); + check("setting off: a transient is listed (" + applet.notifications.length + ")", + applet.notifications.indexOf(kept) !== -1); - // Removal is covered by checkSignalsDisconnected. Doing it here would trip on handlers a - // later commit disconnects, and report that as this check's own failure. + applet._clear_all(); + applet.ignoreTransientNotifications = true; + let dropped = new MessageTray.Notification(_newSource(), "transient", "body"); + dropped.setTransient(true); + applet._notification_added(Main.messageTray, dropped); + check("setting on: a transient is not listed (" + applet.notifications.length + ")", + applet.notifications.indexOf(dropped) === -1); + check("setting on: a transient was destroyed", dropped._destroyed === true); + let normal = _notify(applet, _newSource(), "normal"); + check("setting on: a normal notification is still listed", + applet.notifications.indexOf(normal) !== -1); } finally { - try { cleanup(); } catch (e) { /* best effort */ } + applet.ignoreTransientNotifications = original; + cleanup(); } - global.log("checkCriticalBlink: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkTransient: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -function checkSignalsDisconnected() { +// showEmptyTray decides whether an empty applet stays on the panel. +function checkTrayChrome() { let applet = _applet(); let ok = true; function check(label, condition) { - global.log("checkSignalsDisconnected: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkTrayChrome: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } + let panelLabel = () => applet._applet_label.get_text(); + let heading = () => applet.menu_label.label.get_text(); + let originalEmpty = applet.showEmptyTray; + let originalCount = applet.showNotificationCount; try { + // Assigning these directly skips the settings binding, so call what it would have called. + applet.showEmptyTray = true; + applet.showNotificationCount = true; + applet.menu.close(); applet._clear_all(); - fill(6); + applet._show_hide_tray(); + check("empty: no panel label (" + panelLabel() + ")", panelLabel() === ""); + // The heading is translated: assert it changes, not its wording. + let emptyHeading = heading(); + check("empty: the heading is not blank (" + emptyHeading + ")", emptyHeading.length > 0); + check("empty: the clear item is hidden", applet.clear_action.actor.visible === false); + check("empty: showEmptyTray keeps the applet on the panel", applet.actor.visible === true); - // Emitting 'destroy' on a listed notification drops it from the applet's list. The - // tray destroys them of its own accord, so always pick one listed right now. - let before = applet.notifications.length; - check("something is listed to begin with (" + before + ")", before > 0); - applet.notifications[0].emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); - check("the destroy handler is connected on arrival (" + before + " -> " + - applet.notifications.length + ")", applet.notifications.length === before - 1); + fill(3); + check("3 listed: panel label is the count (" + panelLabel() + ")", panelLabel() === "3"); + check("3 listed: the heading has the count in it (" + heading() + ")", + heading().indexOf("3") !== -1); + check("3 listed: the heading changed from empty", heading() !== emptyHeading); + check("3 listed: the clear item is shown", applet.clear_action.actor.visible === true); - // Reload rather than calling on_applet_removed_from_panel() by hand, so the manager - // tears it down the way a theme change would. Held above zero so the applet's decrement - // does not reach 0 and clear the tray, which is what would leave nothing to leak. - let counter = MessageTray.extensionsHandlingNotifications; - MessageTray.extensionsHandlingNotifications = 2; - _reloadAndWaitForApplet(5000); - MessageTray.extensionsHandlingNotifications = counter; + applet.showNotificationCount = false; + applet.update_list(); + check("count off: the panel label is empty (" + panelLabel() + ")", panelLabel() === ""); + check("count off: the heading still has it (" + heading() + ")", + heading().indexOf("3") !== -1); - let listed = applet.notifications.length; - check("removal left notifications listed, so there is something to leak (" + - listed + ")", listed > 0); - if (listed > 0) { - let target = applet.notifications[0]; - target.emit('destroy', MessageTray.NotificationDestroyedReason.DISMISSED); - check("a listed notification no longer reaches the removed applet (" + listed + - " -> " + applet.notifications.length + ")", - applet.notifications.length === listed); - } + applet.showNotificationCount = true; + applet.showEmptyTray = false; + applet._clear_all(); + applet._show_hide_tray(); + check("empty with showEmptyTray off: the applet leaves the panel", + applet.actor.visible === false); + + fill(1); + check("an arrival brings it back", applet.actor.visible === true); } finally { - try { cleanup(); } catch (e) { /* best effort */ } + applet.showEmptyTray = originalEmpty; + applet.showNotificationCount = originalCount; + cleanup(); + // A blanket show() would strand an empty applet on the panel for the next check. + applet._show_hide_tray(); } - global.log("checkSignalsDisconnected: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkTrayChrome: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } -// AppletPopupMenu parents its actor into Main.uiGroup, so a menu that outlived its applet is a -// stray direct child of it. By actor identity, so the session language does not matter. -function _liveMenus(actors) { - let kids = Main.uiGroup.get_children(); - return actors.filter(actor => actor !== null && kids.indexOf(actor) !== -1); -} - -function _reloadAndWaitForApplet(maxRounds) { - Extension.reloadExtension(UUID, Extension.Type.APPLET); - let back = _pumpUntil(() => AppletManager.getRunningInstancesForUuid(UUID).length > 0, maxRounds); - return back ? _applet() : null; -} - -function checkMenuNotLeaked() { +// One source can hold several notifications, and destroying it takes all of them with it. +function checkSourceCascade() { + let applet = _applet(); let ok = true; function check(label, condition) { - global.log("checkMenuNotLeaked: " + label + ": " + (condition ? "ok" : "FAIL")); + global.log("checkSourceCascade: " + label + ": " + (condition ? "ok" : "FAIL")); if (!condition) ok = false; } - // One reload settles it. More are not more conclusive, and each tears the shell's applets - // down while this call is still pumping the main loop, which has killed the session. - const RELOADS = 1; - // About 10s of pumping, a bound rather than a wait; the reload usually lands in under 50ms. - const MAX_WAIT_ROUNDS = 5000; + try { + applet.menu.close(); + applet._clear_all(); - let applet = _applet(); - applet.menu.close(); - applet._clear_all(); + let source = _newSource("Shared"); + for (let i = 0; i < 3; i++) + _notify(applet, source, "shared " + i); + check("one source holds all three (" + source.notifications.length + ")", + source.notifications.length === 3); + check("the applet lists all three (" + applet.notifications.length + ")", + applet.notifications.length === 3); - // A reload must destroy the menu before it, so exactly one of these stays parented. - let seen = [applet.menu.actor]; - check("exactly one menu before reloading (" + _liveMenus(seen).length + ")", - _liveMenus(seen).length === 1); + source.destroy(); + check("destroying the source empties the applet (" + applet.notifications.length + ")", + applet.notifications.length === 0); + check("and empties the bin (" + _rowActors(applet).length + ")", + _rowActors(applet).length === 0); - let allCameBack = true; - for (let i = 0; i < RELOADS; i++) { - let reloaded = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); - if (!reloaded) { - allCameBack = false; - check("the applet came back after reload " + (i + 1), false); - break; - } - applet = reloaded; - seen.push(applet.menu.actor); + let capped = _newSource("Capped"); + for (let i = 0; i < 25; i++) + _notify(applet, capped, "capped " + i); + check("the source capped itself at 20 (" + capped.notifications.length + ")", + capped.notifications.length === 20); + check("the applet followed it down (" + applet.notifications.length + ")", + applet.notifications.length === 20); + } finally { + cleanup(); } - check("the applet came back after reloading", allCameBack); - - // Exactly one, not "no more than before": zero is a failure this used to pass. - let live = _liveMenus(seen); - check("exactly one menu left after reloading (" + live.length + " of " + seen.length + - " seen)", live.length === 1); - check("the one left is the reloaded applet's own menu", - applet !== null && live.length === 1 && live[0] === applet.menu.actor); - - // Leave a working applet regardless of the outcome above. - if (!applet) - applet = _reloadAndWaitForApplet(MAX_WAIT_ROUNDS); - if (!applet) - global.log("checkMenuNotLeaked: FAIL the applet did not come back; the panel is missing it"); - else - try { cleanup(); } catch (e) { /* best effort */ } - global.log("checkMenuNotLeaked: " + (ok ? "all checks passed" : "FAILURES above")); + global.log("checkSourceCascade: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } @@ -955,3 +967,77 @@ function checkClockSettings() { global.log("checkClockSettings: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } + +// Adding an actor that still has a parent, and removing one that is not a child, are Clutter +// errors rather than exceptions, so count the calls instead of waiting for a throw. +function checkBorrowedActor() { + let applet = _applet(); + let tray = Main.messageTray; + let bin = applet._notificationbin; + let ok = true; + function check(label, condition) { + global.log("checkBorrowedActor: " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + } + + let addToParented = 0; + let removeNonChild = 0; + let realAdd = bin.add_child; + let realInsert = bin.insert_child_at_index; + let realRemoveActor = bin.remove_actor; + let realRemoveChild = bin.remove_child; + + try { + applet.menu.close(); + applet._clear_all(); + fill(4); + applet._openMenu(); + + let borrowed = applet.notifications[1]; + tray._notificationQueue.push(borrowed); + tray._showNotification(); + _pumpUntil(() => tray._notificationState === 2, 200); + check("the tray took the actor", borrowed.actor.get_parent() !== bin); + + bin.add_child = function (actor) { + if (actor.get_parent() !== null) addToParented++; + return realAdd.call(this, actor); + }; + bin.insert_child_at_index = function (actor, i) { + if (actor.get_parent() !== null) addToParented++; + return realInsert.call(this, actor, i); + }; + bin.remove_actor = function (actor) { + if (actor.get_parent() !== this) removeNonChild++; + return realRemoveActor.call(this, actor); + }; + bin.remove_child = function (actor) { + if (actor.get_parent() !== this) removeNonChild++; + return realRemoveChild.call(this, actor); + }; + + applet.update_list(); + check("rebuilding did not add an actor that already had a parent (" + + addToParented + ")", addToParented === 0); + + applet._clear_all(); + check("clearing did not remove an actor from a container that is not its parent (" + + removeNonChild + ")", removeNonChild === 0); + } finally { + bin.add_child = realAdd; + bin.insert_child_at_index = realInsert; + bin.remove_actor = realRemoveActor; + bin.remove_child = realRemoveChild; + _pumpUntil(() => tray._notificationState !== 1, 300); + if (tray._notification && !tray._notification._destroyed) { + try { tray._hideNotificationCompleted(); } catch (e) { /* best effort */ } + } + try { tray._notificationQueue.length = 0; } catch (e) { /* best effort */ } + try { tray._updateNotificationTimeout(0); } catch (e) { /* best effort */ } + cleanup(); + } + + global.log("checkBorrowedActor: " + (ok ? "all checks passed" : "FAILURES above")); + return ok; +} From 55246260b390a4b6ce64c1ee9ec6e11bc35733a2 Mon Sep 17 00:00:00 2001 From: ntwest <7564298+ntwest@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:31:54 -0400 Subject: [PATCH 8/8] notifications@cinnamon.org/applet.js: Parent only the notification rows near the viewport. Opening a tray of 100+ notifications blocked for hundreds of ms and grew with the tray. Only the rows near the viewport are parented now, spacers reserve the height of the rest, and the cap scales with the viewport. Now opening and reopening a large tray get much cheaper. Test: checkRowHeights(), checkRenderedGeometry(), checkGrowOnly(), checkPseudoClasses(), checkOffsetsCurrent() --- .../notifications@cinnamon.org/applet.js | 781 ++++++++++++++---- js/testing/testNotificationsApplet.js | 595 ++++++++++++- 2 files changed, 1205 insertions(+), 171 deletions(-) diff --git a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js index d0e2e4964e..a448954ba5 100644 --- a/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js +++ b/files/usr/share/cinnamon/applets/notifications@cinnamon.org/applet.js @@ -5,6 +5,7 @@ const Gtk = imports.gi.Gtk; const Gio = imports.gi.Gio; const PopupMenu = imports.ui.popupMenu; const St = imports.gi.St; +const GLib = imports.gi.GLib; const Mainloop = imports.mainloop; const Urgency = imports.ui.messageTray.Urgency; const MessageTray = imports.ui.messageTray; @@ -16,13 +17,24 @@ const SignalManager = imports.misc.signalManager; const PANEL_EDIT_MODE_KEY = "panel-edit-mode"; +// Stands in for a row's height until any row has been measured. +const FALLBACK_ROW_HEIGHT = 64; +// Rows kept attached beyond the viewport on each side. +const OVERSCAN_ROWS = 10; +// Floor for the attached-row cap. The cap itself scales with the viewport: a menu on a rotated +// monitor is tall enough to show more rows than any fixed number worth picking. +const MIN_ATTACHED = 60; +// Delay before the idle attach pass starts, so an open-then-close costs nothing. +const IDLE_START_DELAY_MS = 300; +// Per-turn time budget for the idle pass, so cost never depends on per-row cost. +const IDLE_BUDGET_US = 5000; + class CinnamonNotificationsApplet extends Applet.TextIconApplet { constructor(metadata, orientation, panel_height, instanceId) { super(orientation, panel_height, instanceId); this.setAllowedLayout(Applet.AllowedLayout.BOTH); - // Settings this.settings = new Settings.AppletSettings(this, metadata.uuid, instanceId); this._interfaceSettings = new Gio.Settings({schema_id: 'org.cinnamon.desktop.interface'}); this.settings.bind("ignoreTransientNotifications", "ignoreTransientNotifications"); @@ -33,21 +45,17 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this.settings.bind("showNewestFirst", "showNewestFirst", this.update_list); this._setKeybinding(); - // Layout this._orientation = orientation; this.menuManager = new PopupMenu.PopupMenuManager(this); this.menu = new Applet.AppletPopupMenu(this, orientation); this.menuManager.addMenu(this.menu); - // Lists this.notifications = []; // The list of notifications, in order from oldest to newest. - // Events this.signals = new SignalManager.SignalManager(null); this.signals.connect(Main.messageTray, 'notify-applet-update', this._notification_added.bind(this)); this.signals.connect(global.settings, 'changed::' + PANEL_EDIT_MODE_KEY, this._on_panel_edit_mode_changed.bind(this)); - // States this._blinking = false; this._blink_toggle = false; this._blinkTimeoutId = 0; @@ -60,40 +68,33 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { Main.keybindingManager.addXletHotKey(this, "notification-clear", this.keyClear, Lang.bind(this, this._clear_all)); } - // Idempotent: three call sites can each get here first. - _disconnectNotificationSignals(notification) { - if (notification._appletScrollId) { - notification.disconnect(notification._appletScrollId); - notification._appletScrollId = 0; - } - if (notification._appletDestroyId) { - notification.disconnect(notification._appletDestroyId); - notification._appletDestroyId = 0; - } - } - on_applet_removed_from_panel () { this._stop_blinking(); Main.keybindingManager.removeXletHotKey(this, "notification-open"); Main.keybindingManager.removeXletHotKey(this, "notification-clear"); MessageTray.extensionsHandlingNotifications--; - if (MessageTray.extensionsHandlingNotifications === 0) { - this._clear_all(); + try { + if (MessageTray.extensionsHandlingNotifications === 0) { + this._clear_all(); + } + } finally { + // Every teardown step below runs even if _clear_all() threw. Removal has to be + // total: leaving the menu behind here is the leak this applet used to have. + // Left connected, a later destroy() would re-enter update_list() on a destroyed list. + for (let n of this.notifications) + this._disconnectNotificationSignals(n); + // Unconditional, and after _clear_all(), which still needs the list alive. + this._notificationList.destroy(); + + this.signals.disconnectAllSignals(); + this.settings.finalize(); + this._crit_icon.destroy(); + this._alt_crit_icon.destroy(); + + this.menuManager.removeMenu(this.menu); + this.menu.destroy(); } - - // Whatever is still listed keeps its handlers otherwise, and the applet with them. - for (let n of this.notifications) - this._disconnectNotificationSignals(n); - - this.signals.disconnectAllSignals(); - this.settings.finalize(); - this._crit_icon.destroy(); - this._alt_crit_icon.destroy(); - - // Otherwise every reload leaks another menu, still drawable above a dead applet. - this.menuManager.removeMenu(this.menu); - this.menu.destroy(); } _openMenu() { @@ -102,15 +103,12 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } _display() { - // Always start the applet empty, void of any notifications. this.set_applet_icon_symbolic_name("empty-notif"); this.set_applet_tooltip(_("Notifications")); - // Setup the notification container. this._maincontainer = new St.BoxLayout({name: 'traycontainer', vertical: true}); this._notificationbin = new St.BoxLayout({vertical:true}); - // Setup the tray icon. this.menu_label = new PopupMenu.PopupMenuItem(stringify(this.notifications.length)); this.menu_label.actor.reactive = false; this.menu_label.actor.can_focus = false; @@ -133,6 +131,19 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this.scrollview.set_policy(St.PolicyType.NEVER, St.PolicyType.AUTOMATIC); this.scrollview.set_clip_to_allocation(true); + // The callback keeps clock-use-24h out of the list. + let adjustment = this.scrollview.get_vscroll_bar().get_adjustment(); + this._notificationList = new NotificationList(this._notificationbin, adjustment, + (notification) => this._formatTimestamp(notification)); + this.menu.connect('open-state-changed', (menu, open) => { + this._notificationList.setActive(open); + }); + + // Measured heights go stale on a theme, font or display-scale change, and nothing else + // invalidates them. Routed through this.signals so disconnectAllSignals() tears it down. + this._themeContext = St.ThemeContext.get_for_stage(global.stage); + this.signals.connect(this._themeContext, 'changed', () => this._notificationList.invalidateHeights()); + let vscroll = this.scrollview.get_vscroll_bar(); vscroll.connect('scroll-start', Lang.bind(this, function() { this.menu.passEvents = true; @@ -141,7 +152,6 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this.menu.passEvents = false; })); - // Alternative tray icons. this._crit_icon = new St.Icon({icon_name: 'critical-notif', icon_type: St.IconType.SYMBOLIC, reactive: true, track_hover: true, style_class: 'system-status-icon' }); this._alt_crit_icon = new St.Icon({icon_name: 'alt-critical-notif', icon_type: St.IconType.SYMBOLIC, reactive: true, track_hover: true, style_class: 'system-status-icon' }); @@ -151,7 +161,6 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } _arrangeDisplay() { - // Remove menu actors so we can put them back in a different order according to orientation. this.menu.box.remove_all_children(); if (this._orientation == St.Side.BOTTOM) { @@ -169,15 +178,14 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { this.menu.addActor(this.settingsMenuItem.actor); } - _notification_added (mtray, notification) { // Notification event handler. - // Ignore transient notifications? + _notification_added (mtray, notification) { if (this.ignoreTransientNotifications && notification.isTransient) { notification.destroy(); return; } - // The tray hands a notification back after hiding its banner even if it was - // destroyed meanwhile, and the actor is gone by then. + // Check _destroyed before touching the actor: the tray hands a notification back after + // hiding its banner even if it was destroyed while shown, and the actor is gone by then. if (notification._destroyed) { let destroyed_index = this.notifications.indexOf(notification); if (destroyed_index != -1) { @@ -189,58 +197,62 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { notification.actor.unparent(); let existing_index = this.notifications.indexOf(notification); - if (existing_index != -1) { // This notification is already listed. + if (existing_index != -1) { + // A revision: content changed and the app still considers it current, so re-attach + // rather than leave it off-screen. notification._inNotificationBin = true; - global.reparentActor(notification.actor, this._notificationbin); + this._notificationList.ensureAttached(notification); notification._timeLabel.show(); this.update_list(); return; } - // Add notification to list. notification._inNotificationBin = true; this.notifications.push(notification); - // Steal the notification panel. Style before parenting: St skips the restyle - // for an unmapped actor. - notification.actor.add_style_class_name('notification-applet-padding'); - // Insert where it belongs rather than appending and reordering after. - if (this.showNewestFirst) - this._notificationbin.insert_child_at_index(notification.actor, 0); - else - this._notificationbin.add(notification.actor); - notification.actor._parent_container = this._notificationbin; - // Register for destruction. + this._connectNotificationSignals(notification); + notification._timeLabel.show(); + + this.update_list(); + } + + _connectNotificationSignals(notification) { // Ids kept on the notification: one that outlives this applet would otherwise keep a - // closure holding the applet alive. + // closure re-entering update_list(), and keep the applet alive with it. notification._appletScrollId = notification.connect('scrolling-changed', (notif, scrolling) => { this.menu.passEvents = scrolling }); notification._appletDestroyId = notification.connect('destroy', () => { this._disconnectNotificationSignals(notification); let i = this.notifications.indexOf(notification); - if (i != -1) + if (i !== -1) this.notifications.splice(i, 1); this.update_list(); }); - notification._timeLabel.show(); + } - this.update_list(); + // Undoes _connectNotificationSignals(). Clear, destroy and panel removal can each get here first. + _disconnectNotificationSignals(notification) { + if (notification._appletScrollId) { + notification.disconnect(notification._appletScrollId); + notification._appletScrollId = 0; + } + if (notification._appletDestroyId) { + notification.disconnect(notification._appletDestroyId); + notification._appletDestroyId = 0; + } } update_list () { try { let count = this.notifications.length; - if (count > 0) { // There are notifications. + if (count > 0) { this.actor.show(); this.clear_action.actor.show(); this.set_applet_label(count.toString()); - this._reorderNotifications(); - // Find max urgency and derive list icon. - let max_urgency = -1; - for (let i = 0; i < count; i++) { - let cur_urgency = this.notifications[i].urgency; - if (cur_urgency > max_urgency) - max_urgency = cur_urgency; - } - switch (max_urgency) { + this._renderList(); + let maxUrgency = -1; + for (let i = 0; i < count; i++) + if (this.notifications[i].urgency > maxUrgency) + maxUrgency = this.notifications[i].urgency; + switch (maxUrgency) { case Urgency.LOW: this._stop_blinking(); this.set_applet_icon_symbolic_name("low-notif"); @@ -257,19 +269,19 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } break; } - } else { // There are no notifications. + } else { this._stop_blinking(); this.set_applet_label(''); this.set_applet_icon_symbolic_name("empty-notif"); this.clear_action.actor.hide(); + this._notificationList.setItems([]); if (!this.showEmptyTray) { this.actor.hide(); } } - if (!this.showNotificationCount) { // Don't show notification count + if (!this.showNotificationCount) { this.set_applet_label(''); - // this.clear_action.actor.hide(); } this.menu_label.label.set_text(stringify(count)); this._notificationbin.queue_relayout(); @@ -280,84 +292,43 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { } _clear_all() { - // Iterate a snapshot: destroy() splices this.notifications from its own handler. let list = this.notifications; - let count = list.length; this.notifications = []; - if (count > 0) { - // Coalesce the first-child/last-child churn; each one restyles a subtree. - this._notificationbin.freeze_notify(); - try { - for (let i = count - 1; i >= 0; i--) { - // Unmap before destroying: tearing down an unmapped subtree skips - // the style work. Not ours while the tray shows it as a banner. - if (list[i].actor.get_parent() === this._notificationbin) - this._notificationbin.remove_actor(list[i].actor); - list[i].destroy(NotificationDestroyedReason.DISMISSED); - } - } finally { - this._notificationbin.thaw_notify(); - // If a destroy() threw, keep tracking whatever survived so a - // second clear can finish the job. Normally there are none. - let survivors = list.filter(n => !n._destroyed); - if (survivors.length > 0) - this.notifications = survivors.concat(this.notifications); - } - } - this.update_list(); - } + this._notificationList.setItems([]); + for (let n of list) + this._disconnectNotificationSignals(n); - _reorderNotifications() { - let bin = this._notificationbin; - let count = this.notifications.length; - let idx = this.showNewestFirst ? count - 1 : 0; - let step = this.showNewestFirst ? -1 : 1; - - // If everything is already in the wanted order, drop what we no longer track - // and skip the moves. That covers arrivals and dismissals, nearly every call. - let child = bin.get_first_child(); - let extras = []; - let ordered = true; - for (let i = 0, j = idx; i < count; i++, j += step) { - let want = this.notifications[j].actor; - while (child !== null && child !== want) { - extras.push(child); - child = child.get_next_sibling(); - } - if (child === null) { - ordered = false; - break; - } - child = child.get_next_sibling(); - } - if (ordered) { - while (child !== null) { - extras.push(child); - child = child.get_next_sibling(); + let failed = false; + let failure; + try { + for (let i = list.length - 1; i >= 0; i--) + list[i].destroy(NotificationDestroyedReason.DISMISSED); + } catch (e) { + failed = true; + failure = e; + } finally { + // Keep live rows connected and visible if a destroy() fails. + let survivors = list.filter(n => !n._destroyed); + if (survivors.length > 0) { + for (let n of survivors) + this._connectNotificationSignals(n); + this.notifications = survivors.concat(this.notifications); } - for (let i = 0; i < extras.length; i++) - bin.remove_child(extras[i]); - return; + this.update_list(); } - // Moving never unparents, so St skips the subtree restyle a re-add would cost. - // Count what is placed: a notification the tray has taken has no actor here. - let placed = 0; - for (let i = 0; i < count; i++, idx += step) { - let actor = this.notifications[idx].actor; - let parent = actor.get_parent(); - if (parent === bin) - bin.set_child_at_index(actor, placed++); - else if (parent === null) - bin.insert_child_at_index(actor, placed++); - } + if (failed) + throw failure; + } - // Anything past what we placed is a child we no longer track. - for (let i = bin.get_n_children() - 1; i >= placed; i--) - bin.remove_child(bin.get_child_at_index(i)); + _renderList() { + let ordered = this.notifications.slice(); + if (this.showNewestFirst) + ordered.reverse(); + this._notificationList.setItems(ordered); } - _show_hide_tray() { // Show or hide the notification tray. + _show_hide_tray() { if(!global.settings.get_boolean(PANEL_EDIT_MODE_KEY)) { if (this.notifications.length || this.showEmptyTray) { this.actor.show(); @@ -394,19 +365,28 @@ class CinnamonNotificationsApplet extends Applet.TextIconApplet { Util.spawnCommandLine("cinnamon-settings notifications"); } + // Shared by _update_timestamp() (all attached rows, on open) and NotificationList._attach() + // (one row, at the moment it attaches). + _formatTimestamp(notification) { + let use_24h = this._interfaceSettings.get_boolean('clock-use-24h'); + return timeify(notification._timestamp, use_24h, new Date()); + } + _update_timestamp() { let len = this.notifications.length; if (len === 0) return; - // Read the clock format and the time once, not once per notification. let use_24h = this._interfaceSettings.get_boolean('clock-use-24h'); let now = new Date(); + // Only attached rows are on screen; a row about to attach this open is skipped here, and + // _attach() sets its timestamp itself instead. for (let i = 0; i < len; i++) { let notification = this.notifications[i]; - // set_text skips unchanged strings, which is most of them once the - // relative suffix stops changing after an hour. + if (!this._notificationList.isAttached(notification)) + continue; + // set_text no-ops on an unchanged string, true for most rows once the relative suffix stops changing. notification._timeLabel.set_text(timeify(notification._timestamp, use_24h, now)); } } @@ -446,7 +426,7 @@ function stringify(count) { } function timeify(orig_time, use_24h, now) { - let diff = Math.floor((now.getTime() - orig_time.getTime()) / 1000); // get diff in seconds + let diff = Math.floor((now.getTime() - orig_time.getTime()) / 1000); let str; if (use_24h) { str = orig_time.toLocaleFormat('%x, %T'); @@ -468,3 +448,516 @@ function timeify(orig_time, use_24h, now) { } return str; } + +// Height of every row built so far, and the mean of those for the rest. Predicting an unbuilt +// row exactly needs the theme's metrics and a Pango layout per row; the mean costs nothing and +// converges as rows get built, which is enough to size a scrollbar. +var RowHeights = class RowHeights { + constructor() { + this._measured = new Map(); + this._sum = 0; + } + + invalidate() { + this._measured.clear(); + this._sum = 0; + } + + record(notification, px) { + if (px <= 0) + return; + this.forget(notification); + this._measured.set(notification, px); + this._sum += px; + } + + forget(notification) { + let px = this._measured.get(notification); + if (px === undefined) + return; + this._measured.delete(notification); + this._sum -= px; + } + + estimate() { + return this._measured.size > 0 ? this._sum / this._measured.size : FALLBACK_ROW_HEIGHT; + } + + get(notification) { + let px = this._measured.get(notification); + return px !== undefined ? px : this.estimate(); + } + + offsets(notifications) { + let offsets = new Array(notifications.length + 1); + let estimate = this.estimate(); + let y = 0; + for (let i = 0; i < notifications.length; i++) { + offsets[i] = y; + let px = this._measured.get(notifications[i]); + y += px !== undefined ? px : estimate; + } + offsets[notifications.length] = y; + return offsets; + } +}; + +// Keeps only the rows near the viewport parented. Spacers stand in for the rest, so the scrollbar +// is right without building anything. +var NotificationList = class NotificationList { + constructor(bin, adjustment, formatTimestamp) { + this._bin = bin; + this._adjustment = adjustment; + this._heights = new RowHeights(); + this._formatTimestamp = formatTimestamp; + // Makes every public method a no-op after destroy(), for callers that still hold a reference. + this._destroyed = false; + + this._items = []; // notifications, in display order + // Prefix sum: _offsets[i] is the top of item i, and the last entry is the total height, + // so _offsets[i + 1] is where item i ends. + this._offsets = [0]; + this._attached = new Set(); + this._active = false; + + // Attached while unmapped, so not measurable yet; revisited in _flushPendingMeasurements(). + this._pendingMeasure = new Set(); + + // Filler bins stand in for runs of unattached rows. Reused across renders; the ones past + // what a render used are left unparented rather than destroyed. + this._fillers = []; + + this._topSpacer = new St.Bin(); + this._bottomSpacer = new St.Bin(); + this._bin.add_actor(this._topSpacer); + this._bin.add_actor(this._bottomSpacer); + + this._valueId = this._adjustment.connect('notify::value', () => this._onScroll()); + this._pageId = this._adjustment.connect('notify::page-size', () => this._onScroll()); + this._scrollRenderIdleId = 0; + this._idleId = 0; + this._idleDelayId = 0; + } + + attachedCount() { + return this._attached.size; + } + + isAttached(notification) { + return this._attached.has(notification); + } + + totalHeight() { + return this._offsets[this._items.length]; + } + + invalidateHeights() { + if (this._destroyed) + return; + this._heights.invalidate(); + for (let n of this._attached) + this._pendingMeasure.add(n); + this._rebuildOffsets(); + this._render(); + } + + // Replace the whole list; cheap, since only in-range unattached rows get attached. + setItems(orderedNotifications) { + if (this._destroyed) + return; + let next = new Set(orderedNotifications); + // Forget anything that left: RowHeights holds a strong reference, so skipping this would + // leak most notifications on a virtualized list. + for (let n of this._items) { + if (next.has(n)) + continue; + if (this._attached.has(n)) + this._detach(n); + this._heights.forget(n); + } + + this._items = orderedNotifications; + this._rebuildOffsets(); + this._render(); + } + + _rebuildOffsets() { + this._offsets = this._heights.offsets(this._items); + } + + _wantedRange() { + let n = this._items.length; + if (n === 0) + return [0, 0]; + + let top = this._adjustment.value; + // page_size is 0 before the menu has ever laid out; assume a screenful so the first open + // has something to show. + let page = this._adjustment.page_size > 0 ? this._adjustment.page_size : 400; + + // First row whose bottom is past the top of the viewport. _offsets is sorted, so this is + // a binary search rather than a walk from the start of the list. + let lo = 0; + let hi = n; + while (lo < hi) { + let mid = (lo + hi) >> 1; + if (this._offsets[mid + 1] <= top) + lo = mid + 1; + else + hi = mid; + } + let first = lo; + // The last is bounded by how many rows fit on a page, so a walk is right here. + let last = first; + while (last < n && this._offsets[last] < top + page) + last++; + + return [Math.max(0, first - OVERSCAN_ROWS), Math.min(n, last + OVERSCAN_ROWS)]; + } + + // Room for the wanted range, plus the same again for the idle pass to fill outward into. + // Taken from the range rather than from the viewport height, so it cannot come out below + // the range whatever mix of row heights the tray holds. + _attachCap(first, last) { + return Math.max(MIN_ATTACHED, (last - first) + 2 * OVERSCAN_ROWS); + } + + _attach(notification) { + let actor = notification.actor; + if (actor.get_parent() !== null) + return false; + // Style class before parenting: St skips the restyle for an unmapped actor anyway. + actor.add_style_class_name('notification-applet-padding'); + this._bin.insert_child_at_index(actor, 1); + actor._parent_container = this._bin; + this._attached.add(notification); + // Set here as well as in _update_timestamp(): a row attaching during this open's render + // does so after _update_timestamp() has already run. + if (notification._timeLabel) + notification._timeLabel.set_text(this._formatTimestamp(notification)); + + // Measure now if it can be, otherwise leave it queued: a row parented this turn has no + // allocation yet, so _measure() refuses it until the next drain. + if (!actor.mapped || !this._measure(notification)) + this._pendingMeasure.add(notification); + return true; + } + + // get_preferred_height() undercounts an unmapped actor, since St skips the restyle. The parent + // check catches a row the tray borrowed as a banner between attach and here. + // Measures a row and pins it to that height. The pin is what makes the measurement true: + // once the spacers reserve a long list the bin is overcommitted, and StBoxLayout answers + // that by collapsing every flexible child to its minimum height, 10px under natural here. + // Unpinned, the offsets would describe rows 10px taller than the ones on screen. + _measure(notification) { + let actor = notification.actor; + if (actor.get_parent() !== this._bin) + return false; + let width = this._bin.get_width(); + actor.set_height(-1); + let px = actor.get_preferred_height(width > 0 ? width : -1)[1]; + if (!(px > 0)) + return false; + actor.set_height(px); + this._heights.record(notification, px); + return true; + } + + // Runs first in _render(); setActive(true) maps this subtree just before, so a run of + // closed-menu arrivals all get measured on the next open. + _flushPendingMeasurements() { + if (!this._active || this._pendingMeasure.size === 0) + return; + let changed = false; + for (let n of this._pendingMeasure) { + if (!n.actor.mapped || !this._measure(n)) + continue; + this._pendingMeasure.delete(n); + changed = true; + } + if (changed) + this._rebuildOffsets(); + } + + // Detaching never restyles: st_widget_parent_set only fires style_changed for a non-null parent. + _detach(notification) { + let actor = notification.actor; + // Drop the pin _measure() put on it: the tray sizes a borrowed actor itself. + actor.set_height(-1); + if (actor.get_parent() === this._bin) + this._bin.remove_actor(actor); + if (actor._parent_container === this._bin) + actor._parent_container = null; + this._attached.delete(notification); + this._pendingMeasure.delete(notification); + } + + // A non-null, non-bin parent means the tray is showing this row as a banner; its height stays + // reserved so the list does not jump. + _isBorrowed(notification) { + let parent = notification.actor.get_parent(); + return parent !== null && parent !== this._bin; + } + + _render() { + this._flushPendingMeasurements(); + let [first, last] = this._wantedRange(); + + let measuredNew = false; + for (let i = first; i < last; i++) { + let n = this._items[i]; + if (!this._attached.has(n) && !this._isBorrowed(n)) { + if (this._attach(n) && n.actor.mapped) + measuredNew = true; + } + } + // A row measured just now moves the mean, and every offset with it. + if (measuredNew) + this._rebuildOffsets(); + + this._trimToCap(first, last); + + // Nothing is visible while the menu is closed; rows still attach, layout waits for the open. + if (this._active) + this._layoutChildren(); + } + + // Over the cap, detaches the rows farthest from the viewport. Rows in range are never + // evicted: re-attaching costs a full style cascade, detaching costs nothing. + _trimToCap(first, last) { + let cap = this._attachCap(first, last); + if (this._attached.size <= cap) + return; + let mid = (first + last) / 2; + let candidates = []; + for (let i = 0; i < this._items.length; i++) { + let n = this._items[i]; + if (this._attached.has(n) && (i < first || i >= last)) + candidates.push({ n: n, d: Math.abs(i - mid) }); + } + candidates.sort((a, b) => b.d - a.d); + let excess = this._attached.size - cap; + for (let i = 0; i < candidates.length && excess > 0; i++, excess--) + this._detach(candidates[i].n); + } + + // Walks the list in order: top spacer, each attached row or a filler bin for a gap, bottom + // spacer. Leading and trailing gaps fold into the spacers rather than becoming fillers, so + // :first-child and :last-child never flip onto a row. + _layoutChildren() { + let used = 0; + + this._bin.set_child_at_index(this._topSpacer, 0); + let slot = 1; + let pending = 0; + let sawAttached = false; + let leading = 0; + + for (let i = 0; i < this._items.length; i++) { + let n = this._items[i]; + // The tray may have taken this row since attach; route through _detach() so + // _parent_container stays consistent. + if (this._attached.has(n) && n.actor.get_parent() !== this._bin) + this._detach(n); + + if (this._attached.has(n)) { + if (!sawAttached) { + leading = pending; + sawAttached = true; + } else if (pending > 0) { + if (used === this._fillers.length) + this._fillers.push(new St.Bin()); + let f = this._fillers[used++]; + f.set_height(pending); + // set_child_at_index moves an already-parented actor with no restyle; a filler + // used for the first time this render needs a real insert. + if (f.get_parent() === this._bin) + this._bin.set_child_at_index(f, slot); + else + this._bin.insert_child_at_index(f, slot); + slot++; + } + pending = 0; + this._bin.set_child_at_index(n.actor, slot++); + } else { + pending += this._offsets[i + 1] - this._offsets[i]; + } + } + + this._bin.set_child_at_index(this._bottomSpacer, slot); + + // Anything this render did not need stops standing in for a gap. + for (let i = used; i < this._fillers.length; i++) { + if (this._fillers[i].get_parent() === this._bin) + this._bin.remove_actor(this._fillers[i]); + } + if (!sawAttached) { + this._topSpacer.set_height(this.totalHeight()); + this._bottomSpacer.set_height(0); + } else { + this._topSpacer.set_height(Math.max(0, leading)); + this._bottomSpacer.set_height(Math.max(0, pending)); + } + } + + // Rows are not detached on close; their theme nodes stay cached, so the next open costs nothing. + setActive(isMenuOpen) { + if (this._destroyed) + return; + this._active = isMenuOpen; + if (isMenuOpen) { + this._render(); + this._scheduleIdlePass(); + } else { + this._cancelIdlePass(); + } + } + + // These come from st_viewport_allocate(), so rendering synchronously would queue a relayout + // inside the allocation cycle it is still in. Defer to idle, coalescing a burst of scroll + // notifications into one render. + _onScroll() { + if (!this._active) + return; + if (this._scrollRenderIdleId !== 0) + return; + this._scrollRenderIdleId = GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => { + this._scrollRenderIdleId = 0; + if (!this._destroyed && this._active) + this._render(); + return GLib.SOURCE_REMOVE; + }); + } + + // Fills in the rest of the list after a delay, time-sliced: attaching a row runs a CSS cascade + // and can eat the whole budget alone, so most turns still do just one row. + _scheduleIdlePass() { + if (this._idleDelayId !== 0 || this._idleId !== 0) + return; + this._idleDelayId = GLib.timeout_add(GLib.PRIORITY_DEFAULT_IDLE, + IDLE_START_DELAY_MS, () => { + this._idleDelayId = 0; + if (this._idleId !== 0 || !this._active) + return GLib.SOURCE_REMOVE; + this._idleId = GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => { + if (!this._active) { + this._idleId = 0; + return GLib.SOURCE_REMOVE; + } + // Yield while a scroll render is pending; that attach matters more than this one. + if (this._scrollRenderIdleId !== 0) + return GLib.SOURCE_CONTINUE; + let start = GLib.get_monotonic_time(); + let done = false; + do { + if (!this._attachOneMore()) { + done = true; + break; + } + } while (this._scrollRenderIdleId === 0 && + GLib.get_monotonic_time() - start < IDLE_BUDGET_US); + // Once per turn, not once per row. Measuring a row moves the mean every unbuilt + // row is sized from, so the offsets have to be rebuilt before anything reads them. + this._rebuildOffsets(); + this._layoutChildren(); + if (done) { + this._idleId = 0; + return GLib.SOURCE_REMOVE; + } + return GLib.SOURCE_CONTINUE; + }); + return GLib.SOURCE_REMOVE; + }); + } + + _cancelIdlePass() { + if (this._idleDelayId !== 0) { + GLib.source_remove(this._idleDelayId); + this._idleDelayId = 0; + } + if (this._idleId !== 0) { + GLib.source_remove(this._idleId); + this._idleId = 0; + } + } + + // Attaches the unattached row nearest the viewport. The caller rebuilds the offsets and lays + // out once it has finished its turn. + _attachOneMore() { + let [first, last] = this._wantedRange(); + if (this._attached.size >= this._attachCap(first, last)) + return false; + + let mid = Math.floor((first + last) / 2); + + let best = -1; + let bestDistance = Infinity; + for (let i = 0; i < this._items.length; i++) { + let item = this._items[i]; + // Attached means its parent is the bin, borrowed means the tray has it. Anything + // left is unparented, so _attach() cannot refuse it. + if (this._attached.has(item) || this._isBorrowed(item)) + continue; + let distance = Math.abs(i - mid); + if (distance < bestDistance) { + bestDistance = distance; + best = i; + } + } + if (best === -1) + return false; + return this._attach(this._items[best]); + } + + // Forces a revised notification attached and drops its measured height. + // _notification_added() unparents before this runs, which can leave _attached stale-true, so + // reconcile against the real parent first. + ensureAttached(notification) { + if (this._destroyed) + return; + this._heights.forget(notification); + if (notification.actor.get_parent() !== this._bin) + this._detach(notification); + if (!this._attached.has(notification) && !this._isBorrowed(notification)) + this._attach(notification); + this._rebuildOffsets(); + if (this._active) + this._layoutChildren(); + } + + destroy() { + if (this._destroyed) + return; + this._destroyed = true; + this._cancelIdlePass(); + if (this._scrollRenderIdleId !== 0) { + GLib.source_remove(this._scrollRenderIdleId); + this._scrollRenderIdleId = 0; + } + if (this._valueId) { + this._adjustment.disconnect(this._valueId); + this._valueId = 0; + } + if (this._pageId) { + this._adjustment.disconnect(this._pageId); + this._pageId = 0; + } + for (let n of Array.from(this._attached)) + this._detach(n); + for (let f of this._fillers) { + if (f.get_parent() !== null) + this._bin.remove_actor(f); + f.destroy(); + } + this._fillers = []; + if (this._topSpacer.get_parent() !== null) + this._bin.remove_actor(this._topSpacer); + if (this._bottomSpacer.get_parent() !== null) + this._bin.remove_actor(this._bottomSpacer); + this._topSpacer.destroy(); + this._bottomSpacer.destroy(); + this._items = []; + this._attached.clear(); + this._pendingMeasure.clear(); + } +}; diff --git a/js/testing/testNotificationsApplet.js b/js/testing/testNotificationsApplet.js index b42d4217f9..3e45a1806c 100644 --- a/js/testing/testNotificationsApplet.js +++ b/js/testing/testNotificationsApplet.js @@ -7,11 +7,8 @@ // benchmark() stalls the session while it runs. const GLib = imports.gi.GLib; - const Main = imports.ui.main; - const MessageTray = imports.ui.messageTray; - const AppletManager = imports.ui.appletManager; const UUID = "notifications@cinnamon.org"; @@ -25,19 +22,13 @@ function _applet() { return instances[0]; } +// One source per notification: a single source keeps only MAX_NOTIFICATIONS. function fill(n) { let applet = _applet(); - for (let i = 0; i < n; i++) { - let source = new MessageTray.SystemNotificationSource("Test"); - Main.messageTray.add(source); - sources.push(source); - - let notification = new MessageTray.Notification( - source, `Test notification ${i}`, `Body text for notification ${i}.`); - source.pushNotification(notification); - applet._notification_added(Main.messageTray, notification); - } + for (let i = 0; i < n; i++) + _notify(applet, _newSource(), `Test notification ${i}`, undefined, + `Body text for notification ${i}.`); return applet.notifications.length; } @@ -55,9 +46,17 @@ function cleanup() { log(`[testNotificationsApplet] cleaned up, ${applet.notifications.length} left`); } -// Bin children that are real notification rows, in display order. Every child is one. +// By identity, not position: an actor a caller parented itself is not mistaken for a row. function _rowActors(applet) { - return applet._notificationbin.get_children(); + let list = applet._notificationList; + return applet._notificationbin.get_children().filter(k => + k !== list._topSpacer && k !== list._bottomSpacer && + list._fillers.indexOf(k) === -1); +} + +function _activeFillers(applet) { + return applet._notificationList._fillers.filter( + filler => filler.get_parent() === applet._notificationbin); } // The order update_list() renders in: the applet's list, reversed when showNewestFirst is set. @@ -135,6 +134,83 @@ function checkOrder() { return failures === 0; } +// The spacers are permanent first and last bin children, so the pseudo classes land on them. +function checkPseudoClasses() { + let applet = _applet(); + let list = applet._notificationList; + let failures = 0; + + let hasClass = (actor, name) => { + let classes = actor.get_style_pseudo_class(); + return classes ? classes.split(/\s+/).indexOf(name) !== -1 : false; + }; + + let check = (label, expectedRows) => { + let rows = _rowActors(applet); + if (rows.length !== expectedRows) { + failures++; + log(`[testNotificationsApplet] FAIL ${label}: ${rows.length} rows, expected ${expectedRows}`); + return; + } + + let problems = []; + if (!hasClass(list._topSpacer, "first-child")) + problems.push("first-child is not on the top spacer"); + if (!hasClass(list._bottomSpacer, "last-child")) + problems.push("last-child is not on the bottom spacer"); + for (let i = 0; i < rows.length; i++) { + if (hasClass(rows[i], "first-child")) + problems.push(`row ${i} wrongly carries first-child`); + if (hasClass(rows[i], "last-child")) + problems.push(`row ${i} wrongly carries last-child`); + } + + if (problems.length) { + failures++; + log(`[testNotificationsApplet] FAIL ${label}: ${problems.join(", ")}`); + } else { + log(`[testNotificationsApplet] ok ${label} (${rows.length} rows)`); + } + }; + + let original = applet.showNewestFirst; + try { + + applet.menu.close(); + applet._clear_all(); + + // The classes only get applied to mapped actors, so open the menu first. + applet._openMenu(); + fill(6); + check("after arrivals", 6); + + let listed = applet.notifications.slice(); + listed[3].destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + check("after destroying one from the middle", 5); + listed[0].destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + check("after destroying the first", 4); + listed[5].destroy(MessageTray.NotificationDestroyedReason.DISMISSED); + check("after destroying the last", 3); + + applet.showNewestFirst = !applet.showNewestFirst; + applet.update_list(); + check("after flipping the sort order", 3); + applet.showNewestFirst = !applet.showNewestFirst; + applet.update_list(); + check("after flipping back", 3); + + fill(4); + check("after more arrivals", 7); + + } finally { + applet.showNewestFirst = original; + cleanup(); + } + log(`[testNotificationsApplet] checkPseudoClasses: ${failures === 0 ? "passed" : failures + " failures"}`); + return failures === 0; +} + +// A rebuild while the tray holds one of the actors. function checkBannerHandover() { let applet = _applet(); let tray = Main.messageTray; @@ -176,6 +252,11 @@ function checkBannerHandover() { } } } + // The borrowed row sits in the middle, so its gap must become a filler. + if (_activeFillers(applet).length === 0) { + failures++; + log("[testNotificationsApplet] FAIL banner: no filler opened for the borrowed row's gap"); + } if (failures === 0) log("[testNotificationsApplet] ok rebuild with an actor on loan to a banner"); @@ -208,6 +289,8 @@ function checkBannerHandover() { return failures === 0; } +// The tray hands a notification back even if it was destroyed while shown, once its actor is +// disposed. GJS logs a critical rather than throwing, so watch the property, not an exception. function checkDestroyedHandback() { let applet = _applet(); let tray = Main.messageTray; @@ -268,9 +351,13 @@ function checkDestroyedHandback() { return failures === 0; } +// Fault injection. Asserted on each notification's own _destroyed flag, since _clear_all() +// empties the list before the destroy loop runs. function checkFailedClear() { let applet = _applet(); let failures = 0; + let victim = null; + let realDestroy = null; try { applet.menu.close(); @@ -281,8 +368,8 @@ function checkFailedClear() { // Index 2 throws, and _clear_all() destroys back-to-front, so 0 and 1 are never // attempted. All three should survive, then die on a retry without the fault. let survivors = applet.notifications.slice(0, 3); - let victim = applet.notifications[2]; - let realDestroy = victim.destroy; + victim = applet.notifications[2]; + realDestroy = victim.destroy; victim.destroy = function () { throw new Error("injected destroy failure"); }; try { @@ -300,7 +387,34 @@ function checkFailedClear() { log("[testNotificationsApplet] ok survivors were not destroyed by the failed clear"); } + let listedSurvivors = survivors.filter(n => applet.notifications.indexOf(n) !== -1); + if (listedSurvivors.length !== survivors.length) { + failures++; + log(`[testNotificationsApplet] FAIL failed-clear: only ${listedSurvivors.length} survivor(s) stayed listed`); + } else { + log("[testNotificationsApplet] ok survivors stayed listed after the failed clear"); + } + + let connectedSurvivors = survivors.filter( + n => n._appletScrollId !== 0 && n._appletDestroyId !== 0); + if (connectedSurvivors.length !== survivors.length) { + failures++; + log(`[testNotificationsApplet] FAIL failed-clear: only ${connectedSurvivors.length} survivor(s) kept their applet signals`); + } else { + log("[testNotificationsApplet] ok survivors kept their applet signals"); + } + + let renderedSurvivors = _rowActors(applet).filter( + actor => survivors.some(n => n.actor === actor)); + if (renderedSurvivors.length !== survivors.length) { + failures++; + log(`[testNotificationsApplet] FAIL failed-clear: only ${renderedSurvivors.length} survivor(s) were rendered`); + } else { + log("[testNotificationsApplet] ok survivors were rendered after the failed clear"); + } + victim.destroy = realDestroy; + realDestroy = null; applet._clear_all(); let stillUndestroyed = survivors.filter(n => !n._destroyed); @@ -311,6 +425,8 @@ function checkFailedClear() { log("[testNotificationsApplet] ok retry destroyed the survivors"); } } finally { + if (victim !== null && realDestroy !== null) + victim.destroy = realDestroy; cleanup(); } @@ -318,6 +434,7 @@ function checkFailedClear() { return failures === 0; } +// Pumps the main loop until predicate() holds or maxRounds (about 2ms each) is used up. function _pumpUntil(predicate, maxRounds) { let ctx = GLib.MainContext.default(); for (let i = 0; i < maxRounds; i++) { @@ -329,6 +446,373 @@ function _pumpUntil(predicate, maxRounds) { return predicate(); } +// Waits for the deferred render _onScroll() schedules. The id only goes non-zero once the +// scroll view allocates, so waiting for zero alone would return having waited for nothing. +function _waitForDeferredRender(list) { + let scheduled = _pumpUntil(() => list._scrollRenderIdleId !== 0, 150); + let settled = _pumpUntil(() => list._scrollRenderIdleId === 0, 500); + return scheduled && settled; +} + +function _waitForIdlePass(list, wasScheduled) { + if (!wasScheduled) + return false; + return _pumpUntil(() => list._idleDelayId === 0 && list._idleId === 0, 900); +} + +function _newCheck(name) { + let ok = true; + return { + check: (label, condition) => { + global.log(name + ": " + label + ": " + (condition ? "ok" : "FAIL")); + if (!condition) + ok = false; + }, + finish: () => { + global.log(name + ": " + (ok ? "all checks passed" : "FAILURES above")); + return ok; + } + }; +} + +// Not a test of how close the estimate lands, only that measured rows report their own height +// and unbuilt ones get the mean. +function checkRowHeights() { + let applet = _applet(); + let result = _newCheck("checkRowHeights"); + let check = result.check; + try { + applet.menu.close(); + applet._clear_all(); + // Enough rows that plenty stay unbuilt: the overscan margin is attached on open. + fill(200); + applet._openMenu(); + + let list = applet._notificationList; + let heights = list._heights; + // The idle pass would keep attaching rows underneath these assertions. + list._cancelIdlePass(); + + let measured = Array.from(heights._measured.keys()); + check("opening the menu measured some rows but not all (" + + measured.length + " of " + applet.notifications.length + ")", + measured.length > 0 && measured.length < applet.notifications.length); + + // Without a real height a row falls back to the mean and everything below passes + // for the wrong reason. + let attachedUnmeasured = applet.notifications.filter( + n => list.isAttached(n) && !heights._measured.has(n)); + check("every attached row was measured (" + list.attachedCount() + + " attached, " + attachedUnmeasured.length + " of them unmeasured)", + attachedUnmeasured.length === 0); + let measuredUnattached = measured.filter(n => !list.isAttached(n)); + check("nothing unattached is holding a measured height (" + + measuredUnattached.length + ")", measuredUnattached.length === 0); + + // Against the height the bin allocated, which is what the user sees and what the + // spacers and offsets have to agree with. Measuring at natural width instead put this + // out by up to 10px on a row that wraps. + _pumpUntil(() => false, 25); + let allocated = measured.filter(n => n.actor.mapped && n.actor.get_height() > 0); + let allocWorst = 0; + let allocWorstAt = -1; + for (let n of allocated) { + let err = Math.abs(heights.get(n) - n.actor.get_height()); + if (err > allocWorst) { + allocWorst = err; + allocWorstAt = applet.notifications.indexOf(n); + } + } + check("the stored height is the height the bin allocated (" + allocated.length + + " rows, worst " + allocWorst.toFixed(1) + "px at index " + allocWorstAt + ")", + allocated.length > 0 && allocWorst < 1); + + let mean = heights.estimate(); + let sum = 0; + for (let n of measured) + sum += heights.get(n); + check("the estimate is the mean of what was measured (" + mean.toFixed(1) + + " vs " + (sum / measured.length).toFixed(1) + ")", + Math.abs(mean - sum / measured.length) < 0.01); + + let unmeasured = applet.notifications.find(n => !heights._measured.has(n)); + check("an unbuilt row gets the mean", unmeasured !== undefined && + Math.abs(heights.get(unmeasured) - mean) < 0.01); + + let victim = measured[0]; + let was = heights.get(victim); + heights.record(victim, was + 100); + check("re-recording a row replaces its height rather than adding one (" + + heights._measured.size + " entries)", heights._measured.size === measured.length); + check("the mean moved by the change over the count (" + heights.estimate().toFixed(1) + + " vs " + (mean + 100 / measured.length).toFixed(1) + ")", + Math.abs(heights.estimate() - (mean + 100 / measured.length)) < 0.01); + + heights.record(victim, was); + check("putting the height back restores the mean", Math.abs(heights.estimate() - mean) < 0.01); + + heights.forget(victim); + check("forgetting a row drops it from the count (" + heights._measured.size + ")", + heights._measured.size === measured.length - 1); + check("forgetting a row leaves the mean of the rest", + Math.abs(heights.estimate() - (sum - was) / (measured.length - 1)) < 0.01); + + // The offsets themselves, not a total recomputed the way _rebuildOffsets() does it. + list._rebuildOffsets(); + let items = list._items; + check("the first row sits at zero (" + list._offsets[0] + ")", list._offsets[0] === 0); + let gaps = 0; + for (let i = 1; i < items.length; i++) { + let expected = list._offsets[i - 1] + heights.get(items[i - 1]); + if (Math.abs(list._offsets[i] - expected) > 0.01) + gaps++; + } + check("each row starts where the one above it ends (" + gaps + " that do not)", gaps === 0); + let last = items.length - 1; + check("the last row ends at the total height (" + + (list._offsets[last] + heights.get(items[last])).toFixed(1) + " vs " + + list.totalHeight().toFixed(1) + ")", + Math.abs(list._offsets[last] + heights.get(items[last]) - list.totalHeight()) < 0.01); + + heights.invalidate(); + check("with nothing measured the estimate is the fallback (" + heights.estimate() + ")", + heights.estimate() === 64); + + // Otherwise a theme, font or scale change leaves the scrollbar on the fallback height. + list.invalidateHeights(); + let unmeasuredAfterInvalidation = applet.notifications.filter( + n => list.isAttached(n) && !heights._measured.has(n)); + check("height invalidation remeasured every attached row (" + + unmeasuredAfterInvalidation.length + " unmeasured)", + unmeasuredAfterInvalidation.length === 0); + } finally { + cleanup(); + applet._notificationList.invalidateHeights(); + } + return result.finish(); +} + +// Forces a scrollbar jump, which leaves a discontiguous attached set for the fillers to cover. +function checkRenderedGeometry() { + let applet = _applet(); + let list = applet._notificationList; + let result = _newCheck("checkRenderedGeometry"); + let check = result.check; + let idlePassCancelled = false; + let adjustment = null; + let oldValue = 0; + let oldPageSize = 0; + let oldUpper = 0; + try { + applet.menu.close(); + applet._clear_all(); + fill(100); + applet._openMenu(); + + // The idle-fill pass would attach rows on its own. Armed on a delay, so no race. + list._cancelIdlePass(); + idlePassCancelled = true; + + let bin = applet._notificationbin; + let notifications = applet.notifications.slice(); + let adj = applet.scrollview.get_vscroll_bar().get_adjustment(); + adjustment = adj; + oldValue = adj.value; + oldPageSize = adj.page_size; + oldUpper = adj.upper; + // Shared with whatever position an earlier open left: start from a known top. + adj.value = 0; + _waitForDeferredRender(list); + + // Heights from the list, what _layoutChildren() sized against, not get_height(). + function heightSum() { + let sum = 0; + for (let k of bin.get_children()) { + if (k === list._topSpacer || k === list._bottomSpacer || + list._fillers.indexOf(k) !== -1) { + sum += k.get_height(); + } else { + let n = notifications.find(x => x.actor === k); + sum += list._heights.get(n); + } + } + return sum; + } + + function checkGeometry(label) { + let kids = bin.get_children(); + check(label + ": spacers are the first and last children", + kids[0] === list._topSpacer && kids[kids.length - 1] === list._bottomSpacer); + + let actors = _rowActors(applet); + // list._items, not applet.notifications: showNewestFirst reverses the display order. + let wanted = list._items.filter(n => list._attached.has(n)).map(n => n.actor); + let orderOk = actors.length === wanted.length && + actors.every((a, i) => a === wanted[i]); + check(label + ": attached rows and fillers are in display order", orderOk); + + let sum = heightSum(); + let total = list.totalHeight(); + check(label + ": children heights sum to the list total (" + + sum.toFixed(1) + " vs " + total.toFixed(1) + ")", Math.abs(sum - total) < 1); + } + + check("far fewer rows attached than exist (" + list.attachedCount() + " of " + + notifications.length + ")", list.attachedCount() < notifications.length); + check("initial range: no filler needed for one contiguous run", + _activeFillers(applet).length === 0); + checkGeometry("initial range"); + + // Attachment is grow-only, so a jump leaves two attached runs with a gap between them. + let attachedBeforeJump = list.attachedCount(); + adj.page_size = adj.page_size || 400; + adj.upper = list.totalHeight(); + adj.value = list.totalHeight() - adj.page_size; + let grew = _waitForDeferredRender(list) && + list.attachedCount() > attachedBeforeJump; + check("scrollbar jump attached new rows near the new position", grew); + check("discontiguous attached set needs at least one filler", + _activeFillers(applet).length >= 1); + checkGeometry("after scrollbar jump"); + + // Leave the scrollbar where a real reopen would find it, not stranded mid-jump. + adj.value = 0; + _waitForDeferredRender(list); + } finally { + if (adjustment !== null) { + adjustment.upper = oldUpper; + adjustment.page_size = oldPageSize; + adjustment.value = oldValue; + } + if (idlePassCancelled) + list._scheduleIdlePass(); + cleanup(); + } + return result.finish(); +} + +// Re-attaching costs a full style cascade, so scrolling back detaches nothing. Compares the +// exact set, so a detach masked by an unrelated attach cannot pass. +function checkGrowOnly() { + let applet = _applet(); + let list = applet._notificationList; + function sameSet(a, b) { + if (a.size !== b.size) + return false; + for (let x of a) + if (!b.has(x)) + return false; + return true; + } + + function isSupersetOf(a, b) { + for (let x of b) + if (!a.has(x)) + return false; + return true; + } + + let result = _newCheck("checkGrowOnly"); + let check = result.check; + let idlePassCancelled = false; + let adjustment = null; + let oldValue = 0; + let oldPageSize = 0; + let oldUpper = 0; + try { + applet.menu.close(); + applet._clear_all(); + fill(150); + applet._openMenu(); + + // Cancelled for the same reason as in checkRenderedGeometry. + list._cancelIdlePass(); + idlePassCancelled = true; + + let adj = applet.scrollview.get_vscroll_bar().get_adjustment(); + adjustment = adj; + oldValue = adj.value; + oldPageSize = adj.page_size; + oldUpper = adj.upper; + adj.value = 0; + _waitForDeferredRender(list); + let atTop = list.attachedCount(); + let attachedAtTop = new Set(list._attached); + + // About 15 rows: past the 10-row overscan so the range shifts, still under the cap. + // Scaled from the measured average row height, so it holds on any theme. + let avgRowHeight = list.totalHeight() / applet.notifications.length; + let scrollTarget = Math.min(list.totalHeight() - adj.page_size, 15 * avgRowHeight); + adj.value = Math.max(0, scrollTarget); + _waitForDeferredRender(list); + let atScrolled = list.attachedCount(); + let attachedAtScrolled = new Set(list._attached); + + check("scrolling down attached more rows (" + atTop + " -> " + atScrolled + ")", + atScrolled > atTop); + check("scrolling down detached nothing (" + attachedAtTop.size + " still attached)", + isSupersetOf(attachedAtScrolled, attachedAtTop)); + + adj.value = 0; + _waitForDeferredRender(list); + let attachedBackAtTop = new Set(list._attached); + + check("scrolling back attached nothing new and detached nothing (" + + attachedAtScrolled.size + " -> " + attachedBackAtTop.size + ", same rows)", + sameSet(attachedAtScrolled, attachedBackAtTop)); + + // Walking the whole list is the only way to reach _trimToCap(): the scrolling above + // stays under the cap by design. + let cap = list._attachCap(...list._wantedRange()); + let everAttached = new Set(attachedBackAtTop); + let steps = 12; + for (let i = 1; i <= steps; i++) { + adj.value = (i / steps) * Math.max(0, adj.upper - adj.page_size); + _waitForDeferredRender(list); + for (let n of list._attached) + everAttached.add(n); + } + check("walking the list attached more rows than the cap (" + everAttached.size + + " over the walk, cap " + cap + ")", everAttached.size > cap); + check("the cap held anyway (" + list.attachedCount() + " attached now)", + list.attachedCount() <= cap); + + // A cap below the wanted range makes the two fight: the range attaches a row and the cap + // evicts it on the same pass. The cap has to leave the overscan on top of the range, so + // a fixed one fails here as soon as a viewport is tall enough. 3800 is a rotated 4K + // monitor, which is the case that caught the constant this replaced. + const OVERSCAN_BOTH_SIDES = 20; + let realPage = adj.page_size; + let tooTight = []; + try { + for (let page of [realPage, 400, 2000, 3800]) { + adj.page_size = page; + let [first, last] = list._wantedRange(); + let capHere = list._attachCap(first, last); + if (capHere < (last - first) + OVERSCAN_BOTH_SIDES) + tooTight.push(page + "px: cap " + capHere + " < range " + (last - first)); + } + } finally { + adj.page_size = realPage; + } + check("the cap leaves the overscan above the wanted range at every viewport (" + + (tooTight.length > 0 ? tooTight.join("; ") : "400 to 3800px") + ")", + tooTight.length === 0); + } finally { + if (adjustment !== null) { + adjustment.upper = oldUpper; + adjustment.page_size = oldPageSize; + adjustment.value = oldValue; + } + if (idlePassCancelled) + list._scheduleIdlePass(); + cleanup(); + } + return result.finish(); +} + +// These run on the main loop the compositor shares, so the numbers are how long the desktop +// stops repainting and dispatching input. function benchmark(count) { let applet = _applet(); let n = count || 100; @@ -377,6 +861,8 @@ function benchmark(count) { const Extension = imports.ui.extension; +// True while GLib still holds the source. Asserting on _blinkTimeoutId alone cannot tell a +// cancelled timer from one still queued against a torn-down applet. function _sourceIsLive(id) { if (!id) return false; @@ -384,16 +870,10 @@ function _sourceIsLive(id) { } function _blinkNotify(applet, urgency) { - let source = new MessageTray.SystemNotificationSource(); - Main.messageTray.add(source); - sources.push(source); - let notification = new MessageTray.Notification(source, "blink test", "body"); - notification.setUrgency(urgency); - source.pushNotification(notification); - applet._notification_added(Main.messageTray, notification); - return notification; + return _notify(applet, _newSource(), "blink test", urgency); } +// The blink re-arms a one second timeout while a critical notification is listed. function checkCriticalBlink() { let applet = _applet(); if (!applet) { @@ -455,12 +935,16 @@ function _liveMenus(actors) { return actors.filter(actor => actor !== null && kids.indexOf(actor) !== -1); } +// Reloads the way a theme change or a panel move would. reloadExtension() returns long before +// the applet exists, since Extension._init() is async, so pump rather than sleep. function _reloadAndWaitForApplet(maxRounds) { Extension.reloadExtension(UUID, Extension.Type.APPLET); let back = _pumpUntil(() => AppletManager.getRunningInstancesForUuid(UUID).length > 0, maxRounds); return back ? _applet() : null; } +// on_applet_removed_from_panel() used to leave the menu in Main.uiGroup, drawable above a dead +// applet. Reloads the extension rather than calling that by hand, which would stage the bug. function checkMenuNotLeaked() { let ok = true; function check(label, condition) { @@ -567,8 +1051,9 @@ function checkSignalsDisconnected() { } // Urgency is set before the applet sees it, as notificationDaemon.js does. -function _notify(applet, source, title, urgency) { - let notification = new MessageTray.Notification(source, title, "body"); +function _notify(applet, source, title, urgency, body) { + let notification = new MessageTray.Notification( + source, title, body === undefined ? "body" : body); if (urgency !== undefined) notification.setUrgency(urgency); source.pushNotification(notification); @@ -1041,3 +1526,59 @@ function checkBorrowedActor() { global.log("checkBorrowedActor: " + (ok ? "all checks passed" : "FAILURES above")); return ok; } + +// Differing heights on purpose: with a uniform list the mean never moves, and a stale prefix +// sum looks identical to a current one. +function _fillVaried(n) { + let applet = _applet(); + for (let i = 0; i < n; i++) + _notify(applet, _newSource(), `Varied ${i}`, undefined, + "word ".repeat(1 + (i % 9) * 6)); + return applet.notifications.length; +} + +// Measuring one row moves the mean, and so every offset. The idle fill measures as it attaches, +// so it has to rebuild the offsets before anything reads them. +function checkOffsetsCurrent() { + let applet = _applet(); + let result = _newCheck("checkOffsetsCurrent"); + let check = result.check; + try { + applet.menu.close(); + applet._clear_all(); + _fillVaried(200); + applet._openMenu(); + + let list = applet._notificationList; + let heights = list._heights; + let sum = () => list._items.reduce((t, n) => t + heights.get(n), 0); + let idleWasScheduled = list._idleDelayId !== 0 || list._idleId !== 0; + + _pumpUntil(() => false, 60); + check("rows really do differ in height (" + heights._measured.size + " measured, " + + new Set(Array.from(heights._measured.values())).size + " distinct)", + new Set(Array.from(heights._measured.values())).size > 1); + check("the total matches the heights after opening (" + list.totalHeight().toFixed(0) + + " vs " + sum().toFixed(0) + ")", Math.abs(list.totalHeight() - sum()) < 1); + + // Most rows are unbuilt, so the mean sizes them and moving it must reach the offsets. + check("most rows are still sized from the mean (" + heights._measured.size + " of " + + list._items.length + " measured)", heights._measured.size < list._items.length); + + check("the idle fill completed", _waitForIdlePass(list, idleWasScheduled)); + check("the total still matches the heights (" + list.totalHeight().toFixed(0) + + " vs " + sum().toFixed(0) + ")", Math.abs(list.totalHeight() - sum()) < 1); + + let gaps = 0; + for (let i = 1; i < list._items.length; i++) { + let expected = list._offsets[i - 1] + heights.get(list._items[i - 1]); + if (Math.abs(list._offsets[i] - expected) > 0.01) + gaps++; + } + check("each row still starts where the one above it ends (" + gaps + " that do not)", + gaps === 0); + } finally { + cleanup(); + } + return result.finish(); +}