forked from 1b-agent/opencadstudio-upstream
web: harden drop-open and autosave after adversarial review
Review of the drop-open and autosave commits surfaced one blocker and five majors; all are fixed here, and two claims in the autosave commit message are corrected by this one: tab close does NOT clear a snapshot (matches native .sv$ semantics — saving, restoring, discarding, exit, and the capped boot scan do), and serialization runs on the main thread between awaits, not off the paint path. - The restore prompt can no longer appear with no entries (empty "Recover Drawing" modal on every boot once the autosave dir exists). - Restore no longer consumes the snapshot before the open succeeds: the recovered drawing goes through the guarded drop queue, and the snapshot is deleted only after the tab opens; cancel or parse failure keeps the only copy. - Saving during an autosave round defers its snapshot cleanup until the round finishes, so a stale round can no longer resurrect a snapshot newer than the save. - The boot scan no longer deletes snapshots by mtime comparison (re-opening a drawing refreshes the recent copy's mtime, which silently destroyed wanted snapshots); GC is now a 16-entry cap with oldest-first eviction. - OPFS write failures abort the writable stream (a leaked lock could stall the same-key writer and disable autosave for the session), and DOMException errors now carry their name/message. - Empty (crash-window) snapshots are never offered; an unreadable one warns and stays discardable instead of looping forever. - Drop-open polish: reads start inside the handler and complete in drop order (legacy WebKit safety, deterministic multi-file order), dragenter is canceled too, the install guard only latches on success, non-drawing drops don't steal propagation, a lock-poison panic can't cross into JS, and re-dropping an open drawing no longer risks clobbering an in-flight open (it re-queues). - The model-space sheet floor is per-axis (a thin 8x400 mm profile keeps its length) and centers content on the enlarged page.
This commit is contained in:
1 parent
101d167ebc
commit
3edbc5b7c8
8 files changed
+386
-118
No files matched your search
+98
-11
@@ -8,11 +8,18 @@
|
||||
//! offered for restore at boot when newer than the drawing's last explicit
|
||||
//! save (the OPFS recent copy of the same name).
|
||||
//!
|
||||
//! Losing that comparison never deletes a snapshot: the recent copy is
|
||||
//! refreshed by every OPEN as well as every save, so a snapshot that just
|
||||
//! went stale may still be the only survivor of a crash right after a
|
||||
//! re-open. Accumulation is bounded instead — the boot scan keeps the
|
||||
//! newest [`SNAPSHOT_CAP`] snapshots and evicts only the oldest ones beyond
|
||||
//! that cap.
|
||||
//!
|
||||
//! OPFS entry names carry only the FNV hash of the drawing name (like
|
||||
//! `web_recent`), so a small JSON index maps hashes back to names. The index
|
||||
//! is written before its snapshot: a crash between the two writes leaves a
|
||||
//! name without bytes, which `list` filters out, rather than an orphan
|
||||
//! snapshot nobody can name.
|
||||
//! name without bytes — a missing file or a zero-byte one — which `list`
|
||||
//! filters out, rather than an orphan snapshot nobody can name.
|
||||
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
use wasm_bindgen::JsCast;
|
||||
@@ -38,6 +45,9 @@ pub struct WebAutosaveEntry {
|
||||
/// A snapshot is worth offering when it is strictly newer than the last
|
||||
/// explicit save, or when no saved copy exists at all (the drawing was never
|
||||
/// saved — or its recent copy was evicted — so the snapshot is all there is).
|
||||
/// The comparison decides *offering* only, never survival: the recent copy
|
||||
/// is refreshed by every open, so "older than the recent copy" is not proof
|
||||
/// that a snapshot recovers nothing.
|
||||
#[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))] // unit-tested on native
|
||||
pub(crate) fn newer_than_saved(autosave_ms: f64, saved_ms: Option<f64>) -> bool {
|
||||
match saved_ms {
|
||||
@@ -46,6 +56,33 @@ pub(crate) fn newer_than_saved(autosave_ms: f64, saved_ms: Option<f64>) -> bool
|
||||
}
|
||||
}
|
||||
|
||||
/// Upper bound on stored snapshots; the boot scan evicts the oldest ones
|
||||
/// beyond it. Without a bound, snapshots superseded by a save (or by a
|
||||
/// mere re-open) would pile up forever, since age no longer deletes them.
|
||||
#[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))] // unit-tested on native
|
||||
pub(crate) const SNAPSHOT_CAP: usize = 16;
|
||||
|
||||
/// Split `entries` into `(kept, evicted)` so at most `cap` snapshots remain,
|
||||
/// both sides ordered by `modified_ms` (newest first within each side; the
|
||||
/// evicted names overall run oldest-first, so an interrupted scan deletes
|
||||
/// the least valuable snapshots first). This is the only deletion criterion
|
||||
/// the boot scan applies — being older than the recent copy is not one.
|
||||
#[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))] // unit-tested on native
|
||||
pub(crate) fn cap_evictions(
|
||||
mut entries: Vec<WebAutosaveEntry>,
|
||||
cap: usize,
|
||||
) -> (Vec<WebAutosaveEntry>, Vec<String>) {
|
||||
entries.sort_by(|a, b| b.modified_ms.total_cmp(&a.modified_ms));
|
||||
let overflow = entries.len().saturating_sub(cap);
|
||||
let mut evicted: Vec<String> = entries
|
||||
.split_off(entries.len() - overflow)
|
||||
.into_iter()
|
||||
.map(|entry| entry.name)
|
||||
.collect();
|
||||
evicted.reverse();
|
||||
(entries, evicted)
|
||||
}
|
||||
|
||||
/// The name a restored drawing carries as a tab and download: the drawing's
|
||||
/// stem plus " (recovered)" and its own extension (DWG when the stored name
|
||||
/// has none, matching how autosave serializes untitled tabs). The extension
|
||||
@@ -89,7 +126,10 @@ pub async fn snapshot(name: &str, bytes: &[u8], thumbnail: Option<&[u8]>) -> Res
|
||||
}
|
||||
|
||||
/// All live snapshots. Index rows whose file is gone (crash between writes,
|
||||
/// partial eviction) are skipped rather than reported.
|
||||
/// partial eviction) and zero-byte snapshots (the crash window after the
|
||||
/// index write but before the snapshot write) are skipped rather than
|
||||
/// reported — an empty file recovers nothing and would only route the user
|
||||
/// into the corrupt-file recovery flow.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub async fn list() -> Result<Vec<WebAutosaveEntry>, String> {
|
||||
let directory = autosave_directory(false).await?;
|
||||
@@ -98,6 +138,9 @@ pub async fn list() -> Result<Vec<WebAutosaveEntry>, String> {
|
||||
let Ok(file) = super::web_recent::get_file(&directory, &snapshot_key(&name)).await else {
|
||||
continue;
|
||||
};
|
||||
if file.size() == 0.0 {
|
||||
continue;
|
||||
}
|
||||
entries.push(WebAutosaveEntry {
|
||||
name,
|
||||
size: file.size() as u64,
|
||||
@@ -130,22 +173,26 @@ pub async fn remove(name: &str) -> Result<(), String> {
|
||||
drawing.map(|_| ())
|
||||
}
|
||||
|
||||
/// Boot scan: keep the snapshots worth offering (newer than the drawing's
|
||||
/// last explicit save, newest first) and silently delete the stale ones — a
|
||||
/// snapshot older than the saved copy recovers nothing and would otherwise
|
||||
/// pile up across sessions.
|
||||
/// Boot scan: offer the snapshots worth recovering — strictly newer than the
|
||||
/// drawing's last explicit copy, newest first. Age never deletes: the recent
|
||||
/// copy is refreshed by every OPEN as well as every save, so a snapshot that
|
||||
/// lost the comparison may still be the only survivor of a crash right after
|
||||
/// a re-open. Accumulation is bounded instead — beyond [`SNAPSHOT_CAP`]
|
||||
/// snapshots the oldest are evicted, which is the only deletion here.
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
pub async fn scan() -> Result<Vec<WebAutosaveEntry>, String> {
|
||||
let (kept, evicted) = cap_evictions(list().await?, SNAPSHOT_CAP);
|
||||
for name in evicted {
|
||||
let _ = remove(&name).await;
|
||||
}
|
||||
let mut recoverable = Vec::new();
|
||||
for entry in list().await? {
|
||||
for entry in kept {
|
||||
let saved_ms = super::web_recent::modified_ms(&entry.name).await;
|
||||
if newer_than_saved(entry.modified_ms, saved_ms) {
|
||||
recoverable.push(entry);
|
||||
} else {
|
||||
let _ = remove(&entry.name).await;
|
||||
}
|
||||
}
|
||||
recoverable.sort_by(|a, b| b.modified_ms.total_cmp(&a.modified_ms));
|
||||
// `cap_evictions` already ordered the survivors newest-first.
|
||||
Ok(recoverable)
|
||||
}
|
||||
|
||||
@@ -219,4 +266,44 @@ mod tests {
|
||||
assert_eq!(recovered_name("a.b.dwg"), "a.b (recovered).dwg");
|
||||
assert_eq!(recovered_name(""), " (recovered).dwg");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn under_the_cap_nothing_is_evicted_and_order_is_newest_first() {
|
||||
let (kept, evicted) = cap_evictions(
|
||||
vec![
|
||||
entry("old.dwg", 100.0),
|
||||
entry("new.dwg", 300.0),
|
||||
entry("mid.dwg", 200.0),
|
||||
],
|
||||
SNAPSHOT_CAP,
|
||||
);
|
||||
assert!(evicted.is_empty());
|
||||
assert_eq!(
|
||||
kept.iter().map(|e| e.name.as_str()).collect::<Vec<_>>(),
|
||||
["new.dwg", "mid.dwg", "old.dwg"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn over_the_cap_exactly_the_oldest_overflow_is_evicted() {
|
||||
let entries = (0..SNAPSHOT_CAP + 4)
|
||||
.map(|i| entry(&format!("d{i}.dwg"), i as f64))
|
||||
.collect();
|
||||
let (kept, evicted) = cap_evictions(entries, SNAPSHOT_CAP);
|
||||
assert_eq!(kept.len(), SNAPSHOT_CAP);
|
||||
// Newest kept first, oldest evicted first.
|
||||
assert_eq!(kept.first().map(|e| e.name.as_str()), Some("d19.dwg"));
|
||||
assert_eq!(
|
||||
evicted.iter().map(|name| name.as_str()).collect::<Vec<_>>(),
|
||||
["d0.dwg", "d1.dwg", "d2.dwg", "d3.dwg"]
|
||||
);
|
||||
}
|
||||
|
||||
fn entry(name: &str, modified_ms: f64) -> WebAutosaveEntry {
|
||||
WebAutosaveEntry {
|
||||
name: name.to_string(),
|
||||
size: 16,
|
||||
modified_ms,
|
||||
}
|
||||
}
|
||||
}
|
||||
+60
-29
@@ -5,7 +5,8 @@
|
||||
//! queues them for the 50 ms control poll to open through the web-picker flow.
|
||||
|
||||
use std::collections::VecDeque;
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
use std::sync::Mutex;
|
||||
use wasm_bindgen::JsCast;
|
||||
use wasm_bindgen_futures::JsFuture;
|
||||
|
||||
@@ -14,14 +15,25 @@ use wasm_bindgen_futures::JsFuture;
|
||||
const QUEUE_CAP: usize = 8;
|
||||
|
||||
static DROPPED: Mutex<VecDeque<(String, Vec<u8>)>> = Mutex::new(VecDeque::new());
|
||||
static INSTALLED: OnceLock<()> = OnceLock::new();
|
||||
// `OnceLock::take` needs `&mut self`, which an immutable static cannot
|
||||
// provide — an AtomicBool swap gives the same set-once marker and can still
|
||||
// be reverted when no document exists.
|
||||
static INSTALLED: AtomicBool = AtomicBool::new(false);
|
||||
|
||||
fn warn(line: &str) {
|
||||
web_sys::console::warn_1(&wasm_bindgen::JsValue::from_str(line));
|
||||
}
|
||||
|
||||
fn lock_queue() -> std::sync::MutexGuard<'static, VecDeque<(String, Vec<u8>)>> {
|
||||
// The critical sections are push/pop only, so poisoning is unreachable;
|
||||
// recover anyway rather than panic across the JS boundary.
|
||||
DROPPED
|
||||
.lock()
|
||||
.unwrap_or_else(|poisoned| poisoned.into_inner())
|
||||
}
|
||||
|
||||
fn enqueue(name: String, bytes: Vec<u8>) {
|
||||
let mut queue = DROPPED.lock().unwrap();
|
||||
let mut queue = lock_queue();
|
||||
if queue.len() >= QUEUE_CAP {
|
||||
drop(queue);
|
||||
warn(&format!(
|
||||
@@ -32,6 +44,12 @@ fn enqueue(name: String, bytes: Vec<u8>) {
|
||||
queue.push_back((name, bytes));
|
||||
}
|
||||
|
||||
/// Queue already-read drawing bytes (drag-and-drop, autosave restore) for the
|
||||
/// 50 ms poll to open through the guarded web-picker flow.
|
||||
pub fn enqueue_open(name: String, bytes: Vec<u8>) {
|
||||
enqueue(name, bytes);
|
||||
}
|
||||
|
||||
/// JS-callable entry (same shape as `ocs_control_submit`) for hosts that
|
||||
/// forward drops themselves.
|
||||
#[wasm_bindgen::prelude::wasm_bindgen]
|
||||
@@ -41,7 +59,7 @@ pub fn ocs_web_drop_file(name: String, bytes: js_sys::Uint8Array) {
|
||||
|
||||
/// Pop the oldest dropped file that the poll has not started opening yet.
|
||||
pub fn take_dropped() -> Option<(String, Vec<u8>)> {
|
||||
DROPPED.lock().unwrap().pop_front()
|
||||
lock_queue().pop_front()
|
||||
}
|
||||
|
||||
/// Feed already-read bytes through the same loader tail the web picker uses
|
||||
@@ -71,35 +89,44 @@ pub async fn open_dropped(
|
||||
}
|
||||
}
|
||||
|
||||
/// Register the document-level `dragover`/`drop` listeners once. `dragover`
|
||||
/// MUST preventDefault or the browser navigates to the dropped file instead
|
||||
/// of delivering it to the page.
|
||||
/// Register the document-level `dragenter`/`dragover`/`drop` listeners once.
|
||||
/// `dragover` MUST preventDefault or the browser navigates to the dropped file
|
||||
/// instead of delivering it to the page.
|
||||
pub fn install() {
|
||||
if INSTALLED.set(()).is_err() {
|
||||
if INSTALLED.swap(true, Ordering::SeqCst) {
|
||||
return;
|
||||
}
|
||||
let Some(document) = web_sys::window().and_then(|window| window.document()) else {
|
||||
// Only mark installed once the listeners really are: an early
|
||||
// no-document context must not permanently disable drops.
|
||||
INSTALLED.store(false, Ordering::SeqCst);
|
||||
return;
|
||||
};
|
||||
let on_dragover = wasm_bindgen::closure::Closure::<dyn FnMut(web_sys::Event)>::new(
|
||||
let cancel = wasm_bindgen::closure::Closure::<dyn FnMut(web_sys::Event)>::new(
|
||||
move |event: web_sys::Event| {
|
||||
event.prevent_default();
|
||||
},
|
||||
);
|
||||
let _ =
|
||||
document.add_event_listener_with_callback("dragover", on_dragover.as_ref().unchecked_ref());
|
||||
on_dragover.forget();
|
||||
// Canceling `dragenter` too is belt-and-braces some engines still want
|
||||
// before treating the page as a drop target.
|
||||
let _ = document.add_event_listener_with_callback("dragenter", cancel.as_ref().unchecked_ref());
|
||||
let _ = document.add_event_listener_with_callback("dragover", cancel.as_ref().unchecked_ref());
|
||||
cancel.forget();
|
||||
|
||||
let on_drop = wasm_bindgen::closure::Closure::<dyn FnMut(web_sys::Event)>::new(
|
||||
move |event: web_sys::Event| {
|
||||
event.prevent_default();
|
||||
event.stop_propagation();
|
||||
let Some(drag) = event.dyn_ref::<web_sys::DragEvent>() else {
|
||||
return;
|
||||
};
|
||||
let Some(files) = drag.data_transfer().and_then(|transfer| transfer.files()) else {
|
||||
return;
|
||||
};
|
||||
// Start every read here, inside the handler, and finish them in
|
||||
// one task in drop order: legacy WebKit dislikes drag-originated
|
||||
// reads that begin after the event returns, and parallel reads
|
||||
// would enqueue (and open) in completion order, not drop order.
|
||||
let mut reads: Vec<(String, js_sys::Promise)> = Vec::new();
|
||||
for index in 0..files.length() {
|
||||
let Some(file) = files.get(index) else {
|
||||
continue;
|
||||
@@ -113,15 +140,27 @@ pub fn install() {
|
||||
));
|
||||
continue;
|
||||
}
|
||||
wasm_bindgen_futures::spawn_local(async move {
|
||||
match read_file(&file).await {
|
||||
Ok(bytes) => enqueue(name, bytes),
|
||||
Err(error) => web_sys::console::error_1(&wasm_bindgen::JsValue::from_str(
|
||||
&format!("ocs: failed to read dropped file \"{name}\": {error}"),
|
||||
)),
|
||||
}
|
||||
});
|
||||
reads.push((name, file.array_buffer()));
|
||||
}
|
||||
if reads.is_empty() {
|
||||
return;
|
||||
}
|
||||
// The page is the only intended drop target; hide drawing drops
|
||||
// from host-page handlers only once there is something to open.
|
||||
event.stop_propagation();
|
||||
wasm_bindgen_futures::spawn_local(async move {
|
||||
for (name, read) in reads {
|
||||
match JsFuture::from(read).await {
|
||||
Ok(buffer) => enqueue(name, js_sys::Uint8Array::new(&buffer).to_vec()),
|
||||
Err(error) => {
|
||||
web_sys::console::error_1(&wasm_bindgen::JsValue::from_str(&format!(
|
||||
"ocs: failed to read dropped file \"{name}\": {}",
|
||||
error.as_string().unwrap_or_else(|| format!("{error:?}"))
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
let _ = document.add_event_listener_with_callback("drop", on_drop.as_ref().unchecked_ref());
|
||||
@@ -133,11 +172,3 @@ fn is_drawing_name(name: &str) -> bool {
|
||||
matches!(extension.to_ascii_lowercase().as_str(), "dwg" | "dxf")
|
||||
})
|
||||
}
|
||||
|
||||
/// Same Blob → bytes read as the OPFS recent copies (`web_recent`).
|
||||
async fn read_file(file: &web_sys::File) -> Result<Vec<u8>, String> {
|
||||
let buffer = JsFuture::from(file.array_buffer())
|
||||
.await
|
||||
.map_err(|error| error.as_string().unwrap_or_else(|| format!("{error:?}")))?;
|
||||
Ok(js_sys::Uint8Array::new(&buffer).to_vec())
|
||||
}
|
||||
+35
-7
@@ -90,10 +90,7 @@ pub async fn store_open(
|
||||
return None;
|
||||
};
|
||||
if state.latest_id != pending_id {
|
||||
return Some((
|
||||
state.latest_id,
|
||||
std::sync::Arc::clone(&state.latest_bytes),
|
||||
));
|
||||
return Some((state.latest_id, std::sync::Arc::clone(&state.latest_bytes)));
|
||||
}
|
||||
state.active_writers = state.active_writers.saturating_sub(1);
|
||||
if state.active_writers == 0 {
|
||||
@@ -130,6 +127,24 @@ pub(crate) async fn write_entry(
|
||||
.map_err(js_error)?
|
||||
.dyn_into::<web_sys::FileSystemWritableFileStream>()
|
||||
.map_err(js_error)?;
|
||||
if let Err(error) = write_and_close(&writable, bytes).await {
|
||||
// A failed write must still release the stream: the
|
||||
// FileSystemWritableFileStream holds an exclusive lock on the entry,
|
||||
// and leaving it undrained makes every later write to the same file
|
||||
// fail the same way — one quota error would freeze autosave (its
|
||||
// round would never finish) for the rest of the session. `abort`
|
||||
// discards the pending bytes; ignore its own failure, there is
|
||||
// nothing better to report than `error`.
|
||||
let _ = JsFuture::from(writable.abort()).await;
|
||||
return Err(error);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn write_and_close(
|
||||
writable: &web_sys::FileSystemWritableFileStream,
|
||||
bytes: &[u8],
|
||||
) -> Result<(), String> {
|
||||
let write = writable.write_with_u8_array(bytes).map_err(js_error)?;
|
||||
JsFuture::from(write).await.map_err(js_error)?;
|
||||
JsFuture::from(writable.close()).await.map_err(js_error)?;
|
||||
@@ -262,9 +277,22 @@ fn decode_thumbnail(bytes: Vec<u8>) -> Option<Thumbnail> {
|
||||
|
||||
/// Shared with the autosave snapshot store (`web_autosave`).
|
||||
pub(crate) fn js_error(value: wasm_bindgen::JsValue) -> String {
|
||||
value
|
||||
.as_string()
|
||||
.unwrap_or_else(|| format!("browser storage error: {value:?}"))
|
||||
if let Some(text) = value.as_string() {
|
||||
return text;
|
||||
}
|
||||
// OPFS failures surface as DOMExceptions, which stringify as
|
||||
// "[object DOMException]" — useless in a command-line warning. Their
|
||||
// `name`/`message` properties carry the diagnosis ("QuotaExceededError",
|
||||
// "NotFoundError", …), so render those when present.
|
||||
let property = |key: &str| {
|
||||
js_sys::Reflect::get(&value, &wasm_bindgen::JsValue::from_str(key))
|
||||
.ok()
|
||||
.and_then(|found| found.as_string())
|
||||
};
|
||||
match (property("name"), property("message")) {
|
||||
(Some(name), Some(message)) => format!("{name}: {message}"),
|
||||
_ => format!("browser storage error: {value:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Last-modified time (ms since the Unix epoch) of a drawing's cached copy,
|
||||
|
||||
Reference in new issue
Block a user