Read the window-memory store at a moment when it can actually match

Reported twice, as two complaints: windows do not remember their size or
position across a close or a reboot, and windows spawn stacked on one side
with no smart placement. One bug.

`add_window` looks the store up by app_id. A Wayland toplevel role exists
before its client sends set_app_id, so at the moment srdwm placed a window
the app_id was the empty string, every lookup missed, and every window fell
through to the cascade - which is exactly what "they all open on top of
each other" looks like. The store was being written correctly the whole
time and read at the one moment it could not match.

The lookup now runs again the instant a real app_id arrives, which is still
before the client's first buffer, so nothing is drawn in the wrong place
first. It only moves a window still sitting where the cascade put it: a
rule's explicit geometry, a maximize, a dialog's centring and a client's own
committed size are each more specific than "wherever I last left this app",
and a test asserts none of them is overridden.

A second bug sat underneath the first and only appeared once it was fixed:
the position came back and the size did not, which is stranger than nothing
being restored. The backend keeps its own copy of "this size is only a
guess" (provisional_size) and adopt_provisional_size reads that one rather
than the core flag, so the client's next commit overwrote the size that had
just been restored. Cleared with the same call.

Verified end to end in a nested compositor, driving a real edge-drag with
the virtual-pointer tool:

  seeded store 400,300 500x400 -> opened at exactly 400,300 500x400
  dragged the right edge      -> 646 wide, store rewritten to 646 on release
  closed and reopened          -> 400,300 646x400

Before this the same first step opened at 30,30 800x600.

Also: window_memory::save_all's nested guard now allows a write when the
instance was given its own state directory (SRDWM_STATE_PATH or
XDG_STATE_HOME). The blanket refusal added earlier kept the owner's store
safe but made the feature impossible to test without pointing a test
compositor at the real desktop, which is how this went unverified in the
first place.

4 files changed, 135 insertions, 2 deletions

diff --git a/crates/core/src/manager/dragresize.rs b/crates/core/src/manager/dragresize.rs
index cb151bd..bb7264f 100644
--- a/crates/core/src/manager/dragresize.rs
+++ b/crates/core/src/manager/dragresize.rs
@@ -395,6 +395,59 @@ impl WindowManager {
     /// `crates/wayland/src/window_memory.rs` restoring what was persisted
     /// from a *previous* session at startup, before any real drag/resize
     /// has happened this run.
+    /// Applies a remembered geometry to a window whose `app_id` was not
+    /// known when it was placed.
+    ///
+    /// `add_window` looks the store up by `app_id`, but a Wayland toplevel
+    /// role exists before its client has sent `set_app_id` - so at placement
+    /// time the id is usually the empty string, the lookup misses, and every
+    /// window falls through to a fresh cascade. That is why "windows do not
+    /// remember their size and position" and "windows spawn on top of each
+    /// other" were the same bug: the store was written correctly and read at
+    /// the one moment it could not match.
+    ///
+    /// Called again from the backend the moment a real `app_id` arrives,
+    /// which is still before the client's first buffer, so nothing has been
+    /// drawn at the wrong place yet.
+    ///
+    /// Only touches a window that is still sitting where the cascade put it
+    /// (`size_is_provisional`). A rule's explicit `geometry`, a maximize, a
+    /// dialog's centring and a client's own chosen size all clear that flag,
+    /// and each of them is a more specific decision than "wherever I last
+    /// left this app". Returns whether anything moved.
+    pub fn apply_remembered_geometry(&mut self, id: WindowId) -> bool {
+        let Some(w) = self.windows.get(&id) else { return false };
+        if !w.size_is_provisional || w.is_dialog || w.maximized || w.fullscreen || w.app_id.is_empty() {
+            return false;
+        }
+        let Some((x, y, width, height)) = self.remembered_geometry.get(&w.app_id).copied() else { return false };
+        let (min_w, min_h) = w.min_size;
+        let (width, height) = (width.max(min_w), height.max(min_h));
+        // Same two-step as `add_window`: whichever monitor the remembered
+        // point actually lands on - checked against full geometry, so a
+        // spot under a bar still counts as on-screen - else the monitor the
+        // window is already on. Then clamped into that monitor's *usable*
+        // area, which is what keeps a window from reopening beneath a bar.
+        let monitor = self
+            .monitors
+            .iter()
+            .find(|m| m.full_geometry.contains_point(x, y))
+            .or_else(|| self.monitors.iter().find(|m| m.id == w.monitor))
+            .map(|m| (m.id, m.geometry));
+        let Some((monitor_id, area)) = monitor else { return false };
+        let Some(w) = self.windows.get_mut(&id) else { return false };
+        w.monitor = monitor_id;
+        w.geometry.width = width;
+        w.geometry.height = height;
+        w.geometry.x = x.clamp(area.x, (area.right() - width as i32).max(area.x));
+        w.geometry.y = y.clamp(area.y, (area.bottom() - height as i32).max(area.y));
+        // A remembered size is a real preference, not the placeholder the
+        // cascade handed out, so the client no longer gets to replace it
+        // (`adopt_provisional_size`).
+        w.size_is_provisional = false;
+        true
+    }
+
     pub fn set_remembered_geometry(&mut self, app_id: String, geometry: (i32, i32, u32, u32)) {
         self.remembered_geometry.insert(app_id, geometry);
     }
diff --git a/crates/core/src/manager/tests.rs b/crates/core/src/manager/tests.rs
index fdf0517..e20bb74 100644
--- a/crates/core/src/manager/tests.rs
+++ b/crates/core/src/manager/tests.rs
@@ -116,6 +116,54 @@
         assert!(!win.size_is_provisional, "a deliberately remembered size must never be second-guessed by the client's own default");
     }
 
+    /// The real shape of the bug: a Wayland toplevel exists before its
+    /// client sends `set_app_id`, so `add_window` searched the store for the
+    /// empty string and every window fell through to a fresh cascade. That
+    /// is why "windows do not remember their size" and "windows spawn on top
+    /// of each other" were one bug.
+    #[test]
+    fn a_window_that_learns_its_app_id_late_still_gets_its_remembered_geometry() {
+        let mut wm = wm_with_monitor();
+        wm.set_remembered_geometry("late-app".to_string(), (400, 300, 500, 400));
+        let id = wm.alloc_window_id();
+        // Added with no app_id at all, exactly as a real toplevel arrives.
+        wm.add_window(Window::new(id, "late"));
+        assert!(wm.window(id).unwrap().size_is_provisional, "should have been cascaded, nothing to look up yet");
+
+        wm.window_mut(id).unwrap().app_id = "late-app".to_string();
+        assert!(wm.apply_remembered_geometry(id));
+
+        let win = wm.window(id).unwrap();
+        assert_eq!((win.geometry.x, win.geometry.y), (400, 300));
+        assert_eq!((win.geometry.width, win.geometry.height), (500, 400));
+        assert!(!win.size_is_provisional, "a restored size is a real preference, not a guess");
+    }
+
+    /// Every decision more specific than "wherever I last left this app"
+    /// has to survive a late app_id.
+    #[test]
+    fn a_late_app_id_does_not_overwrite_a_more_specific_placement() {
+        for setup in ["dialog", "maximized", "already-sized"] {
+            let mut wm = wm_with_monitor();
+            wm.set_remembered_geometry("late-app".to_string(), (400, 300, 500, 400));
+            let id = wm.alloc_window_id();
+            wm.add_window(Window::new(id, "late"));
+            {
+                let w = wm.window_mut(id).unwrap();
+                w.app_id = "late-app".to_string();
+                match setup {
+                    "dialog" => w.is_dialog = true,
+                    "maximized" => w.maximized = true,
+                    // What a client committing its own size leaves behind.
+                    _ => w.size_is_provisional = false,
+                }
+            }
+            let before = wm.window(id).unwrap().geometry;
+            assert!(!wm.apply_remembered_geometry(id), "{setup} should not be overridden");
+            assert_eq!(wm.window(id).unwrap().geometry, before, "{setup} geometry moved");
+        }
+    }
+
     #[test]
     fn a_rules_explicit_geometry_is_never_provisional() {
         let mut wm = wm_with_monitor();
diff --git a/crates/wayland/src/state/toplevel.rs b/crates/wayland/src/state/toplevel.rs
index 85365c8..20b55f7 100644
--- a/crates/wayland/src/state/toplevel.rs
+++ b/crates/wayland/src/state/toplevel.rs
@@ -73,8 +73,26 @@ pub(crate) fn sync_toplevel_metadata(state: &mut CompState, id: WindowId, surfac
         // window back to the front any time its title happened to
         // update - reported live as an older window jumping in front of
         // a newer, focused one with no user action to explain it.
+        // Before the rules, and before anything is drawn: a real `app_id`
+        // is the first moment the window-memory store can be looked up at
+        // all, since a toplevel role exists before its client sends
+        // `set_app_id` and `add_window` therefore searched the store for the
+        // empty string. See `apply_remembered_geometry`. A rule's own
+        // explicit `geometry` still wins, which is why this runs first and
+        // `reapply_rules_if_pending` runs after.
+        let restored = state.wm.borrow_mut().apply_remembered_geometry(id);
+        if restored {
+            // The backend keeps its own copy of "this size is only a guess"
+            // (`provisional_size`), and `adopt_provisional_size` reads that
+            // one, not the core flag. Leaving this id in it meant the
+            // client's next commit overwrote the size just restored with
+            // whatever the client would have opened at - the position came
+            // back and the size did not, which is a stranger result than
+            // nothing being restored at all.
+            state.provisional_size.remove(&id);
+        }
         let reapplied = state.wm.borrow_mut().reapply_rules_if_pending(id);
-        if reapplied {
+        if reapplied || restored {
             state.redraw_decoration_buffer(id);
             state.sync_geometry(id);
         }
diff --git a/crates/wayland/src/window_memory.rs b/crates/wayland/src/window_memory.rs
index 4376274..f4f0edf 100644
--- a/crates/wayland/src/window_memory.rs
+++ b/crates/wayland/src/window_memory.rs
@@ -106,6 +106,20 @@ pub(crate) fn load() -> HashMap<String, PersistedGeometry> {
     }
 }
 
+/// True when this instance is nested AND has been given no state directory
+/// of its own, so anything it wrote would land in the real session's store.
+///
+/// The guard cannot simply be "nested", even though that is the case it
+/// exists for. A test instance is the only thing that ever needs to
+/// exercise saving, and refusing every nested write makes the feature
+/// untestable without pointing a compositor at the owner's real desktop.
+/// Pointing `SRDWM_STATE_PATH` (or `XDG_STATE_HOME`) somewhere else is a
+/// deliberate act that says exactly where the writes should go, so a nested
+/// instance that has done it is allowed to write there.
+fn writes_would_land_in_the_real_session() -> bool {
+    crate::running_nested() && std::env::var_os("SRDWM_STATE_PATH").is_none() && std::env::var_os("XDG_STATE_HOME").is_none()
+}
+
 /// Overwrites the whole persisted table from `entries` - called after
 /// every drag/resize-end (see `input/pointer.rs`'s call site), which are
 /// rare, real user actions, not a per-frame event, so writing the whole
@@ -123,7 +137,7 @@ pub(crate) fn load() -> HashMap<String, PersistedGeometry> {
 /// back over it is not. Same reasoning as `publish_gtk_stylesheet`'s own
 /// nested guard.
 pub(crate) fn save_all<'a>(entries: impl Iterator<Item = (&'a str, (i32, i32, u32, u32))>) {
-    if crate::running_nested() {
+    if writes_would_land_in_the_real_session() {
         return;
     }
     let apps: HashMap<String, PersistedGeometry> =