From 22d2d04d1eb56e7bd4938b1a43d136e760d09b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 10 Oct 2026 11:59:47 -0400 Subject: [PATCH] ElyMark: renders asked of a failed page end at once (M-08) The tally remembers why the page went down; until a load succeeds, render() ends each new request with that reason instead of calling a dead page. end_renders is a free function with a gpui test. Co-Authored-By: Claude Opus 5.5 --- apps/ely-mark/Cargo.toml | 1 + apps/ely-mark/src/restore.rs | 2 -- apps/ely-mark/src/tally.rs | 53 ++++++++++++++++++++++++++------- apps/ely-mark/src/view.rs | 51 +++++++++++++++++-------------- apps/ely-mark/src/view/tests.rs | 18 +++++++++++ tasks/BUGS.md | 2 +- tasks/reviews/M-08-r1.md | 5 ++++ tasks/reviews/M-08-r2.md | 1 + 8 files changed, 98 insertions(+), 35 deletions(-) create mode 100644 apps/ely-mark/src/view/tests.rs create mode 100644 tasks/reviews/M-08-r1.md create mode 100644 tasks/reviews/M-08-r2.md diff --git a/apps/ely-mark/Cargo.toml b/apps/ely-mark/Cargo.toml index 161ce50..91bbd9c 100644 --- a/apps/ely-mark/Cargo.toml +++ b/apps/ely-mark/Cargo.toml @@ -27,4 +27,5 @@ serde_json.workspace = true workspace = true [dev-dependencies] +gpui = { workspace = true, features = ["test-support"] } tempfile.workspace = true diff --git a/apps/ely-mark/src/restore.rs b/apps/ely-mark/src/restore.rs index c0c0f94..161e54e 100644 --- a/apps/ely-mark/src/restore.rs +++ b/apps/ely-mark/src/restore.rs @@ -201,8 +201,6 @@ fn clear(dir: &Path) -> io::Result<()> { } } -/// What tab `id` restores: by file, or untitled by number. - impl Desk { /// A tab just shown takes its kept mode, place, text. pub fn restore_shown(&mut self, id: u64, cx: &mut Context) { diff --git a/apps/ely-mark/src/tally.rs b/apps/ely-mark/src/tally.rs index 6ba24c1..e1e9ec9 100644 --- a/apps/ely-mark/src/tally.rs +++ b/apps/ely-mark/src/tally.rs @@ -11,11 +11,13 @@ pub struct Tally { reported: u64, renders: HashMap, asked: u64, + /// Why the page is down, until it loads again. + down: Option, } impl Tally { pub fn new() -> Self { - Tally { load: 0, changed: 0, reported: 0, renders: HashMap::new(), asked: 0 } + Tally { load: 0, changed: 0, reported: 0, renders: HashMap::new(), asked: 0, down: None } } pub fn load(&self) -> u64 { @@ -38,8 +40,19 @@ impl Tally { self.lapse() } - /// The renders owed, as the page fails or restarts. - pub fn lapse(&mut self) -> Vec { + /// A page loaded: it answers again. + pub fn loaded(&mut self) -> Vec { + self.down = None; + self.restart() + } + + /// The page failed: owed and later renders end with `why`. + pub fn fail(&mut self, why: String) -> Vec { + self.down = Some(why); + self.lapse() + } + + fn lapse(&mut self) -> Vec { self.renders.drain().map(|(_, render)| render).collect() } @@ -62,10 +75,13 @@ impl Tally { } /// Owes `render`; its number goes to the page. - pub fn ask(&mut self, render: R) -> u64 { + pub fn ask(&mut self, render: R) -> Result { + if let Some(why) = &self.down { + return Err((render, why.clone())); + } self.asked += 1; self.renders.insert(self.asked, render); - self.asked + Ok(self.asked) } pub fn answered(&mut self, ask: u64) -> Option { @@ -99,12 +115,29 @@ mod tests { } #[test] - fn a_restart_ends_every_owed_render() { + fn a_load_or_failure_ends_every_owed_render() { let mut tally = Tally::new(); - let ask = tally.ask("export"); - assert_eq!(tally.restart(), vec!["export"]); - // Its late answer finds nobody waiting. - assert_eq!(tally.answered(ask), None); + let (html, text) = (tally.ask("html").expect("up"), tally.ask("text").expect("up")); + let mut owed = tally.loaded(); + owed.sort(); + assert_eq!(owed, ["html", "text"]); + // Late answers find nobody waiting. + assert_eq!((tally.answered(html), tally.answered(text)), (None, None)); + let pdf = tally.ask("pdf").expect("up"); + assert_eq!(tally.fail("denied".into()), ["pdf"]); + assert_eq!(tally.answered(pdf), None); + } + + #[test] + fn a_failed_page_refuses_renders_until_it_loads() { + let mut tally = Tally::new(); + tally.fail("denied".into()); + assert_eq!(tally.ask("html"), Err(("html", "denied".to_string()))); + // A refresh is no load: the page is still down. + assert!(tally.restart().is_empty()); + assert_eq!(tally.ask("text"), Err(("text", "denied".to_string()))); + assert!(tally.loaded().is_empty()); + assert!(tally.ask("pdf").is_ok()); } #[test] diff --git a/apps/ely-mark/src/view.rs b/apps/ely-mark/src/view.rs index c8afa12..9188e09 100644 --- a/apps/ely-mark/src/view.rs +++ b/apps/ely-mark/src/view.rs @@ -37,6 +37,20 @@ pub const LARGE: usize = 2 * 1024 * 1024; /// A render's answer: the page's reply, or why not. type Rendered = Box, &mut gpui::App)>; +/// Renders the page will never answer end with `why`. +fn end_renders(pending: Vec, why: &str, cx: &mut gpui::App) { + if pending.is_empty() { + return; + } + log::warn!("mark: {} renders end: {why}", pending.len()); + let why = why.to_string(); + cx.defer(move |cx| { + for done in pending { + done(Err(why.clone()), cx); + } + }); +} + /// The page failed; the shell shows why. pub struct Failed(pub String); @@ -186,7 +200,7 @@ impl MarkView { self.saved = mark(&text); // Reports and renders from before belong to the old text. let lapsed = self.tally.restart(); - self.end_renders(lapsed, "the text was replaced from disk", cx); + end_renders(lapsed, "the text was replaced from disk", cx); crate::page::refresh(self.web.read(cx), &text, self.tally.load()); cx.emit(Reported); } @@ -209,9 +223,13 @@ impl MarkView { done: impl FnOnce(Result, &mut gpui::App) + 'static, cx: &mut Context, ) { - let ask = self.tally.ask(Box::new(done)); - let ask = serde_json::json!({ "kind": kind, "ask": ask }); - crate::page::call(self.web.read(cx), "render", &ask); + match self.tally.ask(Box::new(done)) { + Ok(ask) => { + let ask = serde_json::json!({ "kind": kind, "ask": ask }); + crate::page::call(self.web.read(cx), "render", &ask); + } + Err((done, why)) => end_renders(vec![done], &why, cx), + } } /// `text` replaces the editor's, as one undoable edit. @@ -257,8 +275,8 @@ impl MarkView { match event { // A reload shows the last text that arrived. Event::Loaded => { - let lapsed = self.tally.restart(); - self.end_renders(lapsed, "the page reloaded", cx); + let lapsed = self.tally.loaded(); + end_renders(lapsed, "the page reloaded", cx); self.shown = false; let text = self.doc.lock().expect("the text lock").current.clone(); let at = (self.mode, self.tally.load(), self.place); @@ -268,8 +286,8 @@ impl MarkView { Event::Query(said, reply) => self.query(said, reply, cx), Event::Leave(url) => self.follow(url, cx), Event::Failed(why) => { - let lapsed = self.tally.lapse(); - self.end_renders(lapsed, why, cx); + let lapsed = self.tally.fail(why.clone()); + end_renders(lapsed, why, cx); cx.emit(Failed(why.clone())); } Event::Printed(path, _) => { @@ -278,20 +296,6 @@ impl MarkView { } } - /// Renders the old page will never answer end with `why`. - fn end_renders(&mut self, pending: Vec, why: &str, cx: &mut Context) { - if pending.is_empty() { - return; - } - log::warn!("mark: {} renders lost with the page", pending.len()); - let why = why.to_string(); - cx.defer(move |cx| { - for done in pending { - done(Err(why.clone()), cx); - } - }); - } - /// A clicked link: web and mail open outside. fn follow(&mut self, url: &str, cx: &mut Context) { if ely_office::outward(url) { @@ -486,3 +490,6 @@ impl Render for MarkView { div().size_full().on_drop(drop).child(self.web.clone()) } } + +#[cfg(test)] +mod tests; diff --git a/apps/ely-mark/src/view/tests.rs b/apps/ely-mark/src/view/tests.rs new file mode 100644 index 0000000..cccfa9b --- /dev/null +++ b/apps/ely-mark/src/view/tests.rs @@ -0,0 +1,18 @@ +use std::{cell::RefCell, rc::Rc}; + +use serde_json::Value; + +#[gpui::test] +fn each_owed_render_ends_once_with_the_reason(cx: &mut gpui::TestAppContext) { + let got = Rc::new(RefCell::new(Vec::<(u8, Result)>::new())); + let pending = (0..2) + .map(|n| { + let got = got.clone(); + Box::new(move |rendered, _: &mut gpui::App| got.borrow_mut().push((n, rendered))) as super::Rendered + }) + .collect(); + cx.update(|cx| super::end_renders(pending, "denied", cx)); + cx.run_until_parked(); + let denied = || Err("denied".to_string()); + assert_eq!(*got.borrow(), [(0, denied()), (1, denied())]); +} diff --git a/tasks/BUGS.md b/tasks/BUGS.md index 651811f..aba3518 100644 --- a/tasks/BUGS.md +++ b/tasks/BUGS.md @@ -68,7 +68,7 @@ - [x] M-05 点页面里的链接什么也不发生 — 页面拦下链接单击，地址取链接标记原文，预览单击、可视 Cmd/Ctrl 单击交应用（审查 4 轮） - [x] M-06 `on_app_quit` 无条件删恢复点与崩溃锁，AppKit 直接终止时丢未保存内容 — 未保存时退出只放下锁、不清点，窗口释放时不再删（审查 1 轮） - [x] M-07 崩溃恢复的未命名副本按“第一个显示的未命名标签”配对，内容可能落错标签 — 未命名副本按新建时记下的标签号配对，经 Keeping::take（审查 2 轮） -- [ ] M-08 页面重载后悬着的渲染回调永不完成，导出既不成功也不报错 +- [x] M-08 页面重载后悬着的渲染回调永不完成，导出既不成功也不报错 — 页面失败后挂起与新到的渲染都以错误结束，载入成功才恢复（审查 2 轮） - [ ] M-09 导出的保存面板出错被当作取消 - [ ] M-10 关闭查找框只清当前标签的高亮 - [ ] M-11 页面把搜索词写进应用日志 diff --git a/tasks/reviews/M-08-r1.md b/tasks/reviews/M-08-r1.md new file mode 100644 index 0000000..6abbad1 --- /dev/null +++ b/tasks/reviews/M-08-r1.md @@ -0,0 +1,5 @@ +FAIL + +apps/ely-mark/src/view.rs:212 — `render` queues requests after the page has failed; `heard` at line 270 only ends requests already pending. Native WSL CEF reproduction: deny read access to the disposable page's index.html and reload. Both existing renders correctly receive ERR_ACCESS_DENIED. Request another render while the fault is displayed: Chromium logs `ely is not defined`, and its callback never runs until a later successful reload ends it with `the page reloaded`. HTML, text and PDF exports therefore still hang without an output or export error when requested after failure. Retain the failure state and asynchronously reject new renders until a successful load clears it. Evidence: `/home/raidenmei/m08-native-probe.log`; rendering succeeds again after recovery. + +apps/ely-mark/src/tally.rs:102 — The render regression checks one value returned by `restart`, but never verifies callback execution through `Loaded`/`Failed`, `end_renders`, or the export error handlers. In a disposable source copy, replacing the entire `end_renders` body with a no-op leaves all 22 Linux tests passing while silently dropping every interrupted export. The native CEF probe detects that mutation: after failure, zero callbacks complete instead of two. Add regression tests that exercise both lifecycle events with multiple owed renders, assert each callback receives its error exactly once and reaches the export failure path, reject late replies, and cover requests made after failure. Evidence: `/home/raidenmei/m08-mutation-tests.log`, `/home/raidenmei/m08-native-mutation.log`. Baseline cargo tests: Linux 22/22; Windows 20/21, with only the documented W-02 trailing-space Git-path failure (20/20 when excluded). Probe copies, their target directories and both platforms' incremental caches were removed before this verdict; only small evidence logs remain. diff --git a/tasks/reviews/M-08-r2.md b/tasks/reviews/M-08-r2.md new file mode 100644 index 0000000..7ef22e9 --- /dev/null +++ b/tasks/reviews/M-08-r2.md @@ -0,0 +1 @@ +PASS