diff --git a/crates/web-api/README.md b/crates/web-api/README.md index d14bbfb1..e2d17d13 100644 --- a/crates/web-api/README.md +++ b/crates/web-api/README.md @@ -9,7 +9,9 @@ control channel. replacing an existing `window.`: missing functions are added, an app that already has its own keeps them. Each function returns a Promise that resolves with the method's `result` or rejects with an `Error` carrying `error`. Every call has a timeout (30 s by default, `PageApi::with_timeout`); -when the app does not answer in time, the error says so. +when the app does not answer in time, the error says so and the app's request future is dropped, so an app that releases a request when its future goes away (the desktop `ControlRequest` has `abandon`) never runs a call the page was told had failed. + +`screenshot()` resolves with the reply the app's control method sends, plus `data` when the reply carries the PNG inline: the base64 PNG, whatever field the app names it (`pngBase64`, `png_base64`, `png` or `base64`). A reply with no inline PNG, such as one the app wrote to a `path`, has no `data` and is unchanged. Which functions are installed follows the app's `Methods` table: only the functions whose control method the table provides are installed; `request` is always there. @@ -21,7 +23,7 @@ method the table provides are installed; `request` is always there. | `commands` | `commands()` | `commands` | `engine.commands {}` | | `docInspect` | `docInspect(params)` | `doc_inspect` | `document.inspect` | | `uiInspect` | `uiInspect(params)` | `ui_inspect` | `ui.inspect` | -| `screenshot` | `screenshot(params)` | `screenshot` | `ui.screenshot` (with `data: true`) | +| `screenshot` | `screenshot(params)` | `screenshot` | `ui.screenshot` (with `data: true`); an inline PNG is always also in `data` | `Methods::default()` is the desktop control-server method names; an app whose control channel names a method differently, or lacks one, says so in its table (`Methods::NONE` is the empty table). diff --git a/crates/web-api/src/lib.rs b/crates/web-api/src/lib.rs index 434d5fdb..f02f75b7 100644 --- a/crates/web-api/src/lib.rs +++ b/crates/web-api/src/lib.rs @@ -13,6 +13,7 @@ //! `screenshot(params)`; each function other than `request` is installed only when the app's //! [`methods::Methods`] table provides its control method. //! - [`methods`] is the target-independent table naming the control method behind each function. +//! - [`reply`] shapes replies without a browser: `screenshot()` carries an inline PNG as `data` in every app. //! //! The app hands the crate its request function (its control channel) and a [`methods::Methods`] //! table; the crate never knows the app's internals. @@ -35,6 +36,7 @@ //! ``` pub mod methods; +pub mod reply; #[cfg(target_arch = "wasm32")] pub mod page; diff --git a/crates/web-api/src/page.rs b/crates/web-api/src/page.rs index 538601f8..b59b4829 100644 --- a/crates/web-api/src/page.rs +++ b/crates/web-api/src/page.rs @@ -4,16 +4,19 @@ //! Every function goes through one path, [`PageApi::call`], with the page's timeout; replies //! resolve with the method's `result` or reject with an `Error` carrying `error`. +use std::cell::{Cell, RefCell}; use std::future::Future; use std::pin::Pin; use std::rc::Rc; +use std::task::{Poll, Waker}; use serde_json::{Value, json}; use wasm_bindgen::JsCast; use wasm_bindgen::prelude::{Closure, JsValue}; -use wasm_bindgen_futures::{JsFuture, future_to_promise}; +use wasm_bindgen_futures::future_to_promise; use crate::methods::Methods; +use crate::reply::screenshot_data; /// One control request: method and params in, the protocol's reply object out (`{ok, result}` or /// `{ok: false, error}`). The app builds this from its control channel — send a `ControlRequest`, @@ -126,7 +129,7 @@ impl PageApi { Closure:: js_sys::Promise>::new(move |params: JsValue| { let page = page.clone(); let params = screenshot_params(from_js(¶ms)); - future_to_promise(async move { request_promise_outcome(&page.call(method, params).await) }) + future_to_promise(async move { request_promise_outcome(&screenshot_data(page.call(method, params).await)) }) }) .into_js_value(), )?; @@ -136,30 +139,35 @@ impl PageApi { } /// One control request: the single path through the app's channel that every function takes, - /// with the page's timeout. If the app does not answer within the timeout, the error says so — - /// a hidden tab answers when it is shown again. + /// with the page's timeout. If the app does not answer within the timeout, the error says so + /// (a hidden tab answers when it is shown again) and the app's request future is dropped: an app + /// that releases a request when its future goes away never runs, late, a call the page was told + /// had failed. async fn call(&self, method: &str, params: Value) -> Value { let timeout = Timeout::new(self.timeout_ms); - let request = self.request.clone(); - let method = method.to_string(); - let sent = method.clone(); - let reply = future_to_promise(async move { - let reply = request(sent, params).await; - Ok(to_js(&reply)) - }); - let raced = js_sys::Promise::race(&js_sys::Array::of2(&reply, &timeout.promise)); - let outcome = JsFuture::from(raced).await; + let mut work = (self.request)(method.to_string(), params); + let reply = std::future::poll_fn(|cx| { + if let Poll::Ready(reply) = work.as_mut().poll(cx) { + return Poll::Ready(Some(reply)); + } + if timeout.fired.get() { + return Poll::Ready(None); + } + *timeout.waker.borrow_mut() = Some(cx.waker().clone()); + Poll::Pending + }) + .await; + drop(work); // an unanswered request is released here, before the error goes back drop(timeout); // clear the timer and free its callback once the race is over - match outcome { - Err(_) => json!({ + reply.unwrap_or_else(|| { + json!({ "ok": false, "error": format!( "{} did not answer {} within {} s (a hidden tab answers when it is shown again)", self.app, method, self.timeout_ms / 1000.0 ), - }), - Ok(value) => from_js(&value), - } + }) + }) } } @@ -175,41 +183,43 @@ fn screenshot_params(mut params: Value) -> Value { } } -/// A promise that rejects `ms` milliseconds from now. Dropping it clears the timer (harmless once -/// fired) and frees the timer's callback, so a call leaves nothing behind. The timer comes from -/// `setTimeout` on the global object, reached through `Reflect` — no web-sys, so the crate keeps -/// working in any JS realm. +/// A timer for one call: `fired` turns true `ms` milliseconds from now and wakes the call. Dropping +/// it clears the timer (harmless once fired) and frees the timer's callback, so a call leaves +/// nothing behind. The timer comes from `setTimeout` on the global object, reached through +/// `Reflect` — no web-sys, so the crate keeps working in any JS realm. struct Timeout { - promise: js_sys::Promise, - id: f64, - _fire: Option>, + fired: Rc>, + waker: Rc>>, + id: JsValue, + _fire: Closure, } impl Timeout { fn new(ms: f64) -> Self { - let mut fire = None; - let mut id = f64::INFINITY; - // `Promise::new` runs its executor before returning, so `fire` and `id` are set here. - let promise = js_sys::Promise::new(&mut |_resolve: js_sys::Function, reject: js_sys::Function| { - let callback = Closure::::new(move || { - let _ = reject.call1(&JsValue::NULL, &JsValue::NULL); - }); - id = set_timeout(callback.as_ref().clone(), ms); - fire = Some(callback); + let fired = Rc::new(Cell::new(false)); + let waker: Rc>> = Rc::default(); + let (set, wake) = (fired.clone(), waker.clone()); + let callback = Closure::::new(move || { + set.set(true); + if let Some(waker) = wake.borrow_mut().take() { + waker.wake(); + } }); - Self { promise, id, _fire: fire } + let id = set_timeout(callback.as_ref().clone(), ms); + Self { fired, waker, id, _fire: callback } } } impl Drop for Timeout { fn drop(&mut self) { - clear_timeout(self.id); + clear_timeout(&self.id); } } /// The `setTimeout` global, reached through the global object + `Reflect` (no web-sys). Returns the -/// timer id; a missing `setTimeout` yields an id of infinity, which [`clear_timeout`] ignores. -fn set_timeout(callback: JsValue, ms: f64) -> f64 { +/// timer handle as the environment gave it, to hand back to [`clear_timeout`]: a number in a +/// browser, an object in Node. A missing `setTimeout` yields `undefined`, which `clearTimeout` ignores. +fn set_timeout(callback: JsValue, ms: f64) -> JsValue { let g = global(); get_prop(&g, "setTimeout") .filter(|v| v.is_function()) @@ -217,16 +227,15 @@ fn set_timeout(callback: JsValue, ms: f64) -> f64 { let f: js_sys::Function = f.unchecked_into(); f.call2(&g, &callback, &JsValue::from_f64(ms)).ok() }) - .and_then(|v| v.as_f64()) - .unwrap_or(f64::INFINITY) + .unwrap_or(JsValue::UNDEFINED) } -/// Clear a `setTimeout` id, if the environment has `clearTimeout`. -fn clear_timeout(id: f64) { +/// Clear a timer by the handle [`set_timeout`] returned, if the environment has `clearTimeout`. +fn clear_timeout(id: &JsValue) { let g = global(); if let Some(f) = get_prop(&g, "clearTimeout").filter(|v| v.is_function()) { let f: js_sys::Function = f.unchecked_into(); - let _ = f.call1(&g, &JsValue::from_f64(id)); + let _ = f.call1(&g, id); } } diff --git a/crates/web-api/src/reply.rs b/crates/web-api/src/reply.rs new file mode 100644 index 00000000..0a00a9a7 --- /dev/null +++ b/crates/web-api/src/reply.rs @@ -0,0 +1,68 @@ +//! Reply shaping that needs no browser, so it is natively tested. + +use serde_json::Value; + +/// The names the desktop control protocols give the base64 PNG of a screenshot reply, in the order +/// they are looked for. VectorCraft sends `pngBase64` (when asked with `data: true`), SoundCraft +/// `png_base64`, WordCraft `png`, GridCraft `base64`. +const PNG_FIELDS: [&str; 4] = ["pngBase64", "png_base64", "png", "base64"]; + +/// A successful `screenshot` reply with the PNG also under `data`, whatever the app calls it, so a +/// page reads `const { data } = await window..screenshot()` in every app. The app's own field +/// stays where it is. A reply with no inline PNG (the app wrote it to a `path`), a reply that +/// already has a `data` key of any type, and an error reply are returned as they are. +pub fn screenshot_data(mut reply: Value) -> Value { + if reply.get("ok").and_then(Value::as_bool) == Some(false) { + return reply; + } + let Some(result) = reply.get_mut("result").and_then(Value::as_object_mut) else { + return reply; + }; + if result.contains_key("data") { + return reply; + } + if let Some(png) = PNG_FIELDS.iter().find_map(|field| result.get(*field).filter(|v| v.is_string()).cloned()) { + result.insert("data".to_string(), png); + } + reply +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::screenshot_data; + + #[test] + fn every_apps_field_name_becomes_data_and_stays_where_it_was() { + for field in ["pngBase64", "png_base64", "png", "base64"] { + let reply = json!({"ok": true, "result": {field: "UE5H", "width": 2}}); + let out = screenshot_data(reply); + assert_eq!(out["result"]["data"], "UE5H", "{field}"); + assert_eq!(out["result"][field], "UE5H", "{field} is kept"); + assert_eq!(out["result"]["width"], 2); + } + } + + #[test] + fn a_reply_with_its_own_data_a_path_only_or_an_error_is_left_alone() { + for reply in [ + json!({"ok": true, "result": {"data": "own", "png": "other"}}), + json!({"ok": true, "result": {"data": null, "png": "UE5H"}}), + json!({"ok": true, "result": {"data": {"n": 1}, "png": "UE5H"}}), + json!({"ok": true, "result": {"path": "/tmp/a.png", "width": 2}}), + json!({"ok": false, "error": "boom"}), + json!({"ok": false, "error": "boom", "result": {"png": "UE5H"}}), + json!({"ok": true, "result": null}), + json!({"ok": true, "result": {"png": 7}}), + ] { + assert_eq!(screenshot_data(reply.clone()), reply); + } + } + + #[test] + fn the_first_field_in_the_list_wins() { + let out = screenshot_data(json!({"ok": true, "result": {"png": "b", "pngBase64": "a"}})); + assert_eq!(out["result"]["data"], "a"); + } +} diff --git a/crates/web-api/tests/page.rs b/crates/web-api/tests/page.rs index 092fa296..72fa81e1 100644 --- a/crates/web-api/tests/page.rs +++ b/crates/web-api/tests/page.rs @@ -2,7 +2,7 @@ //! Node tests for the page part: a canned control channel that records what it is asked, and the //! `window.` functions called the way a page calls them. No browser. -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::rc::Rc; use serde_json::{Value, json}; @@ -159,3 +159,56 @@ async fn timeout_gives_an_error_saying_so() { let message = String::from(err.unchecked_into::().message()); assert!(message.contains("vectorcraft did not answer engine.execute within 0.01 s"), "{message}"); } + +thread_local! { + /// Set when the fake control channel's request future is dropped before it answered. + static RELEASED: Cell = const { Cell::new(false) }; +} + +struct ReleaseOnDrop; + +impl Drop for ReleaseOnDrop { + fn drop(&mut self) { + RELEASED.with(|r| r.set(true)); + } +} + +#[wasm_bindgen_test] +async fn a_timed_out_call_drops_the_apps_request_so_the_app_can_release_it() { + reset(); + RELEASED.with(|r| r.set(false)); + let never: Request = Rc::new(|_method: String, _params: Value| { + Box::pin(async { + let _release = ReleaseOnDrop; + let _ = JsFuture::from(js_sys::Promise::new(&mut |_, _| {})).await; + Value::Null + }) + }); + Rc::new(PageApi::new("vectorcraft", Methods::default(), never).with_timeout(10)).install().expect("install"); + assert!(call("commands", &[]).await.is_err(), "the call times out"); + assert!(RELEASED.with(Cell::get), "the request future was dropped when the call timed out"); +} + +#[wasm_bindgen_test] +async fn an_answered_call_does_not_look_timed_out() { + reset(); + RELEASED.with(|r| r.set(false)); + let quick: Request = Rc::new(|_method: String, _params: Value| { + Box::pin(async { + let _release = ReleaseOnDrop; + json!({"ok": true, "result": {"fine": true}}) + }) + }); + Rc::new(PageApi::new("vectorcraft", Methods::default(), quick).with_timeout(10)).install().expect("install"); + let out = call("commands", &[]).await.expect("an answered call resolves"); + assert_eq!(to_value(&out), json!({"fine": true})); +} + +#[wasm_bindgen_test] +async fn screenshot_carries_the_png_as_data_whatever_the_app_calls_it() { + reset(); + let reply = json!({"ok": true, "result": {"base64": "UE5H", "width": 2}}); + Rc::new(PageApi::new("vectorcraft", Methods::default(), canned_request(reply))).install().expect("install"); + let out = call("screenshot", &[]).await.expect("screenshot"); + assert_eq!(to_value(&out), json!({"base64": "UE5H", "width": 2, "data": "UE5H"})); +}