Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions crates/hytte-ui/src/bar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ pub struct BarHandle {
impl BarHandle {
/// Close the bar immediately.
pub fn close(self) {
self.window.close();
self.window.destroy();
}

/// Access the underlying layer-shell window — useful for binding
Expand All @@ -205,7 +205,7 @@ impl BarHandle {

impl Drop for BarHandle {
fn drop(&mut self) {
self.window.close();
self.window.destroy();
}
}

Expand All @@ -217,3 +217,44 @@ impl BarHandle {
std::mem::forget(self);
}
}

// #638: `BarHandle`'s teardown paths (`close()`, `Drop`) call `destroy()`
// rather than `close()` specifically because `close()` is a *request* that
// GTK only turns into a real teardown for a *realized* window — see GTK
// 4.22's own GIR docs for `gtk_window_close` vs. `gtk_window_destroy`. A
// `BarHandle` dropped before its bar was ever shown (an error path during
// monitor setup, or a bar built for a monitor that vanished mid-setup) holds
// an unrealized window, which is exactly the case `close()` cannot dispose
// of. This needs a live display (`system-tests`, run under `xvfb-run`); it
// does not need `gtk4-layer-shell` or a compositor, since the
// realized/unrealized distinction it exercises is a plain-GTK toplevel-list
// mechanic, not a layer-shell one — a bare `gtk::Window` stands in fine.
#[cfg(all(test, feature = "system-tests"))]
mod tests {
use super::*;

#[gtk::test]
fn drop_destroys_an_unrealized_bar_window() {
let window = gtk::Window::new();
assert!(
!window.is_realized(),
"a freshly constructed, never-presented window must start unrealized"
);
let weak = window.downgrade();

// `BarHandle`'s `window` field is private, but this test module is a
// child of `bar`, so the struct-literal is reachable — no need for a
// full `Bar::show()` (which would present the window, realizing it).
let handle = BarHandle { window };
drop(handle);

// Let any deferred destroy settle.
while gtk::glib::MainContext::default().iteration(false) {}

assert!(
weak.upgrade().is_none(),
"dropping a BarHandle must destroy its window even when it was \
never realized — a bare close() would silently leave it alive",
);
}
}
10 changes: 5 additions & 5 deletions crates/hytte-ui/src/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ pub fn attach_dismiss_catcher(popover: &gtk::Popover, monitor: &Monitor) {
// live show — e.g. a bar chip's menu is up when `monitors_changed` tears
// the whole bar down on hot-plug. Without an `unmap` hook the catchers'
// `Vec` is merely dropped, but a `gtk::Window` toplevel lives in GTK's
// global toplevel list until `close()`d, so each survives as an invisible
// global toplevel list until `destroy()`d, so each survives as an invisible
// full-output click-eater with no visible cause.
//
// Whichever fires first drains and closes the catchers; the other finds an
Expand All @@ -280,11 +280,11 @@ pub fn attach_dismiss_catcher(popover: &gtk::Popover, monitor: &Monitor) {

/// Idempotently drain and close every catcher window. Called from both the
/// popover's `closed` and `unmap` handlers; the first to fire takes the windows
/// out of the shared cell and `close()`s them (removing each toplevel from
/// out of the shared cell and `destroy()`s them (removing each toplevel from
/// GTK's window list), leaving the other a harmless no-op.
fn close_catchers(catchers: &Rc<RefCell<Vec<gtk::Window>>>) {
for win in catchers.borrow_mut().drain(..) {
win.close();
win.destroy();
}
}

Expand Down Expand Up @@ -371,9 +371,9 @@ fn map_position(p: Position) -> gtk::PositionType {
//
// What *is* exercisable headlessly (needs a display → gated to `system-tests`,
// run under `xvfb-run`) is the teardown mechanism the fix hinges on: that
// `close_catchers` idempotently drains the shared cell and that `close()`ing
// `close_catchers` idempotently drains the shared cell and that `destroy()`ing
// each window actually removes it from GTK's global toplevel list — the very
// thing a bare drop does *not* do, which is why an un-`close()`d catcher leaks.
// thing a bare drop does *not* do, which is why an un-`destroy()`d catcher leaks.
#[cfg(all(test, feature = "system-tests"))]
mod tests {
use super::*;
Expand Down