shell: a hosted module may refuse a close and ask first (makepad#65)

crates/shell/src/lib.rs

@@ -48,6 +48,8 @@ pub mod layout;
 pub mod octosense;
 pub mod module_host;
 #[cfg(test)]
+mod module_close_tests;
+#[cfg(test)]
 mod module_panic_tests;
 pub mod module_view;
 pub mod native_apps;
@@ -104,7 +106,7 @@ use ai_bus::{AiBus, Route};
 use apps::{AppRegistry, Hosting};
 use android_integration::AndroidRuntime;
 use makepad_ai_services::wire::{ServiceCall, ServiceDown, ToolResult};
-use makepad_app_module::{AppModule, ExecOutcome, ModuleUpstream, WindowRequest};
+use makepad_app_module::{AppModule, CloseDecision, ExecOutcome, ModuleCloseAction, ModuleUpstream, WindowRequest};
 use module_host::ModuleHost;
 use pane_links::{PaneCall, PaneLinks};
 use makepad_widgets::ai_slot::AiSlotRequests;
@@ -1576,10 +1578,23 @@ impl App {
     }
 
     fn request_close(&mut self, cx: &mut Cx, client: ClientId) {
+        let stopped = self.state.as_ref().and_then(|s| s.clients.get(&client)).is_some_and(|s| s.stopped);
+        // A live module instance is asked first (makepad#65): one holding
+        // something a close would destroy (the terminal's running jobs)
+        // refuses and asks the person in its own root. Its tile stays, in
+        // front so the question is seen; a yes arrives as the root's
+        // `ModuleCloseAction::Confirmed` (handle_actions), a no as nothing.
+        if !stopped && self.module_host.is_module(client) && self.module_host.ask_close(cx, client) == CloseDecision::Veto {
+            let app = self.state.as_ref().and_then(|s| s.clients.get(&client)).map(|s| s.app.clone()).unwrap_or_default();
+            log!("wm: {app} (client {client}) asks before closing; its tile stays until the person confirms");
+            self.activate_client(cx, client);
+            self.update_bar(cx);
+            self.redraw_all(cx);
+            return;
+        }
         // A module instance has no process to ask politely and nothing to
         // reap later: it ends now, through the same removal as a death.
         // So does one of its extra windows, and a stopped process's tile.
-        let stopped = self.state.as_ref().and_then(|s| s.clients.get(&client)).is_some_and(|s| s.stopped);
         if stopped || self.module_host.is_module(client) || self.module_windows.contains_key(&client) {
             self.remove_client(cx, client);
             self.update_bar(cx);
@@ -1624,6 +1639,38 @@ impl App {
         self.redraw_all(cx);
     }
 
+    /// A module instance's root confirmed the close it refused earlier
+    /// (`ModuleCloseAction::Confirmed` from the root's uid): it closes now,
+    /// through the same removal as any close. A confirmation from a root
+    /// whose close is not pending is ignored.
+    fn module_close_confirmed(&mut self, cx: &mut Cx, uid: WidgetUid) {
+        let Some(client) = self.module_host.close_confirmed(uid) else {
+            return;
+        };
+        log!("wm: client {client} confirmed its close");
+        self.remove_client(cx, client);
+        self.update_bar(cx);
+        self.redraw_all(cx);
+    }
+
+    /// The shell is asked to quit (the menu's Quit, Cmd+Q, the window's
+    /// close button): every hosted instance is asked first. True when
+    /// nobody refused and the quit may go ahead now; otherwise the first
+    /// refusing instance comes to the front with its question, and the
+    /// quit happens when the last of them confirms (`take_quit_ready`). A
+    /// forced end (a termination signal, a kill) never asks.
+    fn ask_before_quit(&mut self, cx: &mut Cx) -> bool {
+        let refused = self.module_host.ask_quit(cx);
+        let Some(&first) = refused.first() else {
+            return true;
+        };
+        log!("wm: quit waits: clients {refused:?} ask before closing");
+        self.activate_client(cx, first);
+        self.update_bar(cx);
+        self.redraw_all(cx);
+        false
+    }
+
     /// A client the pool is holding: no tile, no focus, invisible.
     fn is_warm(&mut self, client: ClientId) -> bool {
         self.state_mut()
@@ -2567,7 +2614,11 @@ impl App {
                     }
                     Some(client) => {
                         self.request_close(cx, client);
-                        ToolResult::ok(id, format!("{app} is closing"), "closing")
+                        if self.module_host.close_pending(client) {
+                            ToolResult::ok(id, format!("{app} asks the person before closing"), "asking")
+                        } else {
+                            ToolResult::ok(id, format!("{app} is closing"), "closing")
+                        }
                     }
                 },
             },
@@ -3144,7 +3195,11 @@ impl App {
             return;
         }
         match target {
-            "system.quit" => cx.quit(),
+            "system.quit" => {
+                if self.ask_before_quit(cx) {
+                    cx.quit();
+                }
+            }
             "style.background" => {
                 self.close_shell_menu(cx);
                 self.next_background(cx);
@@ -5068,6 +5123,9 @@ impl MatchEvent for App {
             let Some(wa) = action.as_widget_action() else {
                 continue;
             };
+            if let ModuleCloseAction::Confirmed = wa.cast::<ModuleCloseAction>() {
+                self.module_close_confirmed(cx, wa.widget_uid);
+            }
             if let glance_panel::ShellGlancePanelAction::Open { app, route } = wa.cast::<glance_panel::ShellGlancePanelAction>() {
                 log!("wm: glance card opens {} (route {:?})", app, route);
                 self.set_glance_open(cx, false);
@@ -5343,9 +5401,28 @@ impl App {
         // or draw, or in a call the shell made — is contained by now; show
         // it closed and free it before the next event (module_host.rs).
         self.contain_module_faults(cx);
+        // A quit that waited on instances asking the person goes ahead once
+        // the last of them confirmed (or failed and has nothing left to ask).
+        if self.module_host.take_quit_ready() {
+            log!("wm: every instance confirmed; quitting");
+            cx.quit();
+        }
     }
 
     fn shell_handle_event_inner(&mut self, cx: &mut Cx, event: &Event) {
+        // Quitting, or closing the shell's window, asks the hosted instances
+        // first (makepad#65). A termination signal is never refused.
+        match event {
+            Event::QuitRequested(request)
+                if request.reason != QuitReason::Signal && !request.handled.get() && !self.ask_before_quit(cx) =>
+            {
+                request.handle();
+            }
+            Event::WindowCloseRequested(request) if request.accept_close.get() && !self.ask_before_quit(cx) => {
+                request.accept_close.set(false);
+            }
+            _ => {}
+        }
         // Recording belongs to the WM, including on Home and in an OS menu.
         // Forwarding this chord also starts a recorder in the focused child.
         if let Event::KeyDown(e) | Event::KeyUp(e) = event {

crates/shell/src/mobile_app.rs

@@ -1157,7 +1157,7 @@ impl App {
                     }
                     _=>{
                         if g.screen==PhoneScreen::Recents && !g.shell && delta.y < -90.0 && delta.y.abs()>delta.x.abs()*1.2 {
-                            if let Some(PhoneHit::Card(client))=g.hit {self.request_close(cx,client);self.state_mut().phone.navigate(PhoneScreen::Recents);}
+                            if let Some(PhoneHit::Card(client))=g.hit {self.request_close(cx,client);if !self.module_host.close_pending(client) {self.state_mut().phone.navigate(PhoneScreen::Recents);}}
                         }else if delta.length()<12.0 {
                             if let Some(hit)=g.hit.filter(|h|Some(h)==hit.as_ref()) {self.phone_action(cx,hit);}
                         }else if !crate::mobile_navigation::ENABLED && g.screen==PhoneScreen::Home && delta.y < -55.0 && delta.y.abs()>delta.x.abs() {self.phone_action(cx,PhoneHit::Drawer);}

crates/shell/src/module_close_tests.rs

@@ -0,0 +1,226 @@
+//! Hosted close confirmation (makepad#65, `AppModule::close_requested`):
+//! a module that refuses a close keeps its instance until its root emits
+//! `ModuleCloseAction::Confirmed`, and a shell quit waits on every refusal.
+//! The shell's side is `App::request_close`, `App::module_close_confirmed`
+//! and `App::ask_before_quit` (lib.rs); these pin the decisions they act on.
+
+use crate::module_host::{CloseGate, ModuleHost};
+use makepad_ai_services::wire::{ServiceCall, ServiceManifest};
+use makepad_app_module::*;
+use makepad_widgets::*;
+
+/// How a probe answers `close_requested`.
+#[derive(Clone, Copy)]
+enum Answer {
+    /// The trait's default (no override reached).
+    Default,
+    Veto,
+    Panic,
+}
+
+struct CloseProbe {
+    id: &'static str,
+    answer: Answer,
+}
+
+impl AppModule for CloseProbe {
+    fn id(&self) -> &'static str {
+        self.id
+    }
+    fn label(&self) -> &'static str {
+        "Close probe"
+    }
+    fn capabilities(&self) -> &'static [&'static str] {
+        &[]
+    }
+    fn open_schema(&self) -> OpenSchema {
+        OpenSchema::new(1)
+    }
+    fn register(&self, _vm: &mut ScriptVm) {}
+    fn create(&self, vm: &mut ScriptVm, _open: ValidatedOpen, _handles: InstanceHandles) -> InstanceParts {
+        let value = script_eval!(vm, { use mod.widgets.* View {} });
+        InstanceParts {
+            root: WidgetRef::script_from_value(vm, value),
+            executor: Box::new(Quiet),
+            shutdown: Box::new(|_| {}),
+        }
+    }
+    fn close_requested(&self, _cx: &mut Cx, _root: &WidgetRef) -> CloseDecision {
+        match self.answer {
+            Answer::Default => CloseDecision::Allow,
+            Answer::Veto => CloseDecision::Veto,
+            Answer::Panic => panic!("probe: panic in close_requested"),
+        }
+    }
+}
+
+struct Quiet;
+impl ServiceExecutor for Quiet {
+    fn manifest(&self) -> ServiceManifest {
+        ServiceManifest::new("close-probe", "Close probe", "test")
+    }
+    fn execute(&mut self, _cx: &mut Cx, call: &ServiceCall) -> ExecOutcome {
+        ExecOutcome::Done(makepad_ai_services::wire::ToolResult::unavailable(&call.call_id, "probe"))
+    }
+}
+
+static AGREEABLE: CloseProbe = CloseProbe { id: "agreeable", answer: Answer::Default };
+static REFUSING: CloseProbe = CloseProbe { id: "refusing", answer: Answer::Veto };
+static PANICKING: CloseProbe = CloseProbe { id: "panicking", answer: Answer::Panic };
+
+fn setup() -> (Cx, ModuleHost) {
+    let mut cx = Cx::new(Box::new(|_, _| {}));
+    cx.with_vm(makepad_widgets::script_mod);
+    (cx, ModuleHost::default())
+}
+
+fn create(cx: &mut Cx, host: &mut ModuleHost, client: u64, module: &'static dyn AppModule) {
+    host.create(cx, client, module, module.open_schema().empty_open().unwrap(), dvec2(400.0, 700.0)).unwrap();
+}
+
+fn uid(host: &ModuleHost, client: u64) -> WidgetUid {
+    host.get(client).unwrap().root.widget_uid()
+}
+
+// ---- the gate's decisions, without a module ----
+
+#[test]
+fn an_allowed_close_tears_down_and_leaves_nothing_pending() {
+    let mut gate = CloseGate::default();
+    assert!(gate.answered(1, CloseDecision::Allow, WidgetUid(10)));
+    assert!(!gate.is_pending(1));
+    assert_eq!(gate.confirmed(WidgetUid(10)), None, "nothing was waiting for a yes");
+}
+
+#[test]
+fn a_vetoed_close_keeps_the_instance_and_waits_for_its_root() {
+    let mut gate = CloseGate::default();
+    assert!(!gate.answered(1, CloseDecision::Veto, WidgetUid(10)), "a veto never tears down");
+    assert!(gate.is_pending(1));
+    // Asked again (the person clicked close twice) and it now allows: the
+    // mark goes and the instance closes.
+    assert!(gate.answered(1, CloseDecision::Allow, WidgetUid(10)));
+    assert!(!gate.is_pending(1));
+}
+
+#[test]
+fn only_the_pending_root_confirms_its_close() {
+    let mut gate = CloseGate::default();
+    gate.answered(1, CloseDecision::Veto, WidgetUid(10));
+    gate.answered(2, CloseDecision::Veto, WidgetUid(20));
+    assert_eq!(gate.confirmed(WidgetUid(99)), None, "another widget's yes is ignored");
+    assert!(gate.is_pending(1) && gate.is_pending(2));
+    assert_eq!(gate.confirmed(WidgetUid(20)), Some(2));
+    assert!(!gate.is_pending(2) && gate.is_pending(1));
+    assert_eq!(gate.confirmed(WidgetUid(20)), None, "a second yes closes nothing more");
+    gate.forget(1);
+    assert_eq!(gate.confirmed(WidgetUid(10)), None, "a torn-down instance's root confirms nothing");
+}
+
+#[test]
+fn a_quit_nobody_refuses_goes_ahead_at_once() {
+    let mut gate = CloseGate::default();
+    assert!(gate.quit_asked([]));
+    assert!(!gate.quit_waiting());
+    assert!(!gate.take_quit_ready(), "it quit already; nothing is waiting");
+}
+
+#[test]
+fn a_quit_waits_for_every_refusing_instance() {
+    let mut gate = CloseGate::default();
+    assert!(!gate.quit_asked([(1, WidgetUid(10)), (2, WidgetUid(20))]));
+    assert!(gate.quit_waiting());
+    assert_eq!(gate.confirmed(WidgetUid(10)), Some(1));
+    assert!(!gate.take_quit_ready(), "one still asks");
+    assert_eq!(gate.confirmed(WidgetUid(20)), Some(2));
+    assert!(gate.take_quit_ready(), "the last yes lets the quit go");
+    assert!(!gate.take_quit_ready(), "once");
+}
+
+#[test]
+fn a_failed_instance_does_not_hold_a_quit_forever() {
+    let mut gate = CloseGate::default();
+    assert!(!gate.quit_asked([(1, WidgetUid(10))]));
+    gate.forget(1);
+    assert!(gate.take_quit_ready());
+}
+
+#[test]
+fn closing_one_app_abandons_a_waiting_quit() {
+    // The person declined the quit's question in the terminal, then later
+    // closes that terminal and says yes: only the terminal closes.
+    let mut gate = CloseGate::default();
+    assert!(!gate.quit_asked([(1, WidgetUid(10))]));
+    gate.close_asked();
+    assert!(!gate.answered(1, CloseDecision::Veto, WidgetUid(10)));
+    assert_eq!(gate.confirmed(WidgetUid(10)), Some(1));
+    assert!(!gate.take_quit_ready(), "the shell stays up");
+}
+
+// ---- the host, with real instances ----
+
+#[test]
+fn a_module_that_allows_is_closed_and_one_that_refuses_is_kept() {
+    let (mut cx, mut host) = setup();
+    create(&mut cx, &mut host, 1, &AGREEABLE);
+    create(&mut cx, &mut host, 2, &REFUSING);
+
+    assert_eq!(host.ask_close(&mut cx, 1), CloseDecision::Allow);
+    assert!(!host.close_pending(1));
+    assert!(host.teardown(&mut cx, 1));
+
+    assert_eq!(host.ask_close(&mut cx, 2), CloseDecision::Veto);
+    assert!(host.close_pending(2), "a refusal is remembered");
+    assert!(host.is_module(2), "and the instance stays");
+
+    assert!(host.teardown(&mut cx, 2));
+    assert!(!host.close_pending(2), "a teardown clears the mark");
+}
+
+#[test]
+fn a_confirmation_closes_only_the_instance_whose_root_sent_it() {
+    let (mut cx, mut host) = setup();
+    create(&mut cx, &mut host, 1, &REFUSING);
+    create(&mut cx, &mut host, 2, &REFUSING);
+    let (first, second) = (uid(&host, 1), uid(&host, 2));
+    assert_ne!(first, second);
+
+    assert_eq!(host.ask_close(&mut cx, 1), CloseDecision::Veto);
+    // Instance 2 was never asked: its yes means nothing.
+    assert_eq!(host.close_confirmed(second), None);
+    assert_eq!(host.close_confirmed(first), Some(1));
+    assert!(!host.close_pending(1));
+    assert!(host.teardown(&mut cx, 1) && host.teardown(&mut cx, 2));
+}
+
+#[test]
+fn a_module_that_panics_while_answering_is_let_go() {
+    let (mut cx, mut host) = setup();
+    create(&mut cx, &mut host, 1, &PANICKING);
+    assert_eq!(host.ask_close(&mut cx, 1), CloseDecision::Allow);
+    assert!(!host.close_pending(1));
+    assert_eq!(host.take_faults(&mut cx).len(), 1, "the panic is contained and reported");
+    assert!(host.teardown(&mut cx, 1));
+}
+
+#[test]
+fn a_shell_quit_asks_every_instance_and_waits_for_the_refusals() {
+    let (mut cx, mut host) = setup();
+    create(&mut cx, &mut host, 1, &AGREEABLE);
+    create(&mut cx, &mut host, 2, &REFUSING);
+    create(&mut cx, &mut host, 3, &REFUSING);
+
+    assert_eq!(host.ask_quit(&mut cx), vec![2, 3]);
+    assert!(!host.take_quit_ready());
+    assert_eq!(host.close_confirmed(uid(&host, 3)), Some(3));
+    assert!(host.teardown(&mut cx, 3));
+    assert!(!host.take_quit_ready(), "instance 2 still asks");
+    assert_eq!(host.close_confirmed(uid(&host, 2)), Some(2));
+    assert!(host.teardown(&mut cx, 2));
+    assert!(host.take_quit_ready());
+
+    // Only instances that allow: the quit goes at once.
+    assert!(host.ask_quit(&mut cx).is_empty());
+    assert!(!host.take_quit_ready());
+    assert!(host.teardown(&mut cx, 1));
+}