From 7c51026cfaf72acf985e7dd275744f39bb3d0759 Mon Sep 17 00:00:00 2001 From: Zachary Date: Fri, 25 Feb 2022 20:42:26 +0100 Subject: [PATCH 1/6] Prevent closures from running if code has panicked Closes: #1290 --- egui_web/src/backend.rs | 40 +++- egui_web/src/lib.rs | 398 +++++++++++++++++++------------------ egui_web/src/text_agent.rs | 107 +++++----- 3 files changed, 298 insertions(+), 247 deletions(-) diff --git a/egui_web/src/backend.rs b/egui_web/src/backend.rs index 56acc26614b..f3033cac393 100644 --- a/egui_web/src/backend.rs +++ b/egui_web/src/backend.rs @@ -359,11 +359,37 @@ pub fn start(canvas_id: &str, app: Box) -> Result Result { - let runner_ref = AppRunnerRef(Arc::new(Mutex::new(app_runner))); - install_canvas_events(&runner_ref)?; - install_document_events(&runner_ref)?; - text_agent::install_text_agent(&runner_ref)?; - repaint_every_ms(&runner_ref, 1000)?; // just in case. TODO: make it a parameter - paint_and_schedule(runner_ref.clone())?; - Ok(runner_ref) + let runner_container = AppRunnerContainer { + runner: Arc::new(Mutex::new(app_runner)), + panicked: Arc::new(AtomicBool::new(false)), + }; + + install_canvas_events(&runner_container)?; + install_document_events(&runner_container)?; + text_agent::install_text_agent(&runner_container)?; + repaint_every_ms(&runner_container, 1000)?; // just in case. TODO: make it a parameter + + paint_and_schedule(&runner_container.runner, runner_container.panicked.clone())?; + + // Disable all event handlers on panic + std::panic::set_hook(Box::new({ + let previous_hook = std::panic::take_hook(); + + let panicked = runner_container.panicked; + + move |panic_info| { + tracing::info_span!("egui_panic_handler").in_scope(|| { + tracing::trace!("setting panicked flag"); + + panicked.store(true, SeqCst); + + tracing::info!("egui disabled all event handlers due to panic"); + }); + + // Propagate panic info to the previously registered panic hook + previous_hook(panic_info); + } + })); + + Ok(runner_container.runner) } diff --git a/egui_web/src/lib.rs b/egui_web/src/lib.rs index 04b767fe884..98d029d32b2 100644 --- a/egui_web/src/lib.rs +++ b/egui_web/src/lib.rs @@ -29,14 +29,16 @@ pub mod webgl2; pub use backend::*; -use egui::mutex::Mutex; +use egui::mutex::{Mutex, MutexGuard}; pub use wasm_bindgen; pub use web_sys; use input::*; pub use painter::Painter; +use web_sys::EventTarget; use std::collections::BTreeMap; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use wasm_bindgen::prelude::*; @@ -302,12 +304,56 @@ pub fn percent_decode(s: &str) -> String { // ---------------------------------------------------------------------------- -#[derive(Clone)] -pub struct AppRunnerRef(Arc>); +pub type AppRunnerRef = Arc>; -fn paint_and_schedule(runner_ref: AppRunnerRef) -> Result<(), JsValue> { +pub struct AppRunnerContainer { + runner: AppRunnerRef, + panicked: Arc, +} + +impl AppRunnerContainer { + /// Convenience function to reduce boilerplate and ensure that all event handlers + /// are dealt with in the same way + pub fn add_event_listener( + &self, + target: &EventTarget, + event_name: &'static str, + mut closure: impl FnMut(E, MutexGuard<'_, AppRunner>) + 'static, + ) -> Result<(), JsValue> { + use wasm_bindgen::JsCast; + + // Create a JS closure based on the FnMut provided + let closure = Closure::wrap(Box::new({ + // Clone atomics + let runner_ref = self.runner.clone(); + let panicked = self.panicked.clone(); + + move |event: web_sys::Event| { + // Only call the wrapped closure if the egui code has not panicked + if !panicked.load(Ordering::SeqCst) { + // Cast the event to the expected event type + let event = event.unchecked_into::(); + + let lock = runner_ref.lock(); + + closure(event, lock); + } + } + }) as Box); + + // Add the event listener to the target + target.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; + + // Bypass closure drop so that event handler can call the closure + closure.forget(); + + Ok(()) + } +} + +fn paint_and_schedule(runner_ref: &AppRunnerRef, panicked: Arc) -> Result<(), JsValue> { fn paint_if_needed(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { - let mut runner_lock = runner_ref.0.lock(); + let mut runner_lock = runner_ref.lock(); if runner_lock.needs_repaint.fetch_and_clear() { let (needs_repaint, clipped_meshes) = runner_lock.logic()?; runner_lock.paint(clipped_meshes)?; @@ -320,34 +366,40 @@ fn paint_and_schedule(runner_ref: AppRunnerRef) -> Result<(), JsValue> { Ok(()) } - fn request_animation_frame(runner_ref: AppRunnerRef) -> Result<(), JsValue> { + fn request_animation_frame( + runner_ref: AppRunnerRef, + panicked: Arc, + ) -> Result<(), JsValue> { use wasm_bindgen::JsCast; let window = web_sys::window().unwrap(); - let closure = Closure::once(move || paint_and_schedule(runner_ref)); + let closure = Closure::once(move || paint_and_schedule(&runner_ref, panicked)); window.request_animation_frame(closure.as_ref().unchecked_ref())?; closure.forget(); // We must forget it, or else the callback is canceled on drop Ok(()) } - paint_if_needed(&runner_ref)?; - request_animation_frame(runner_ref) + // Only paint and schedule if there has been no panic + if !panicked.load(Ordering::SeqCst) { + paint_if_needed(runner_ref)?; + request_animation_frame(runner_ref.clone(), panicked)?; + } + + Ok(()) } -fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { - use wasm_bindgen::JsCast; +fn install_document_events(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { let window = web_sys::window().unwrap(); let document = window.document().unwrap(); - { - // keydown - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::KeyboardEvent| { + runner_container.add_event_listener( + &document, + "keydown", + |event: web_sys::KeyboardEvent, mut runner_lock| { if event.is_composing() || event.key_code() == 229 { // https://www.fxsitecompat.dev/en-CA/docs/2018/keydown-and-keyup-events-are-now-fired-during-ime-composition/ return; } - let mut runner_lock = runner_ref.0.lock(); let modifiers = modifiers_from_event(&event); runner_lock.input.raw.modifiers = modifiers; @@ -400,16 +452,13 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { if prevent_default { event.prevent_default(); } - }) as Box); - document.add_event_listener_with_callback("keydown", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - // keyup - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::KeyboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "keyup", + |event: web_sys::KeyboardEvent, mut runner_lock| { let modifiers = modifiers_from_event(&event); runner_lock.input.raw.modifiers = modifiers; if let Some(key) = translate_key(&event.key()) { @@ -420,19 +469,16 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { }); } runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("keyup", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // paste - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::ClipboardEvent| { + runner_container.add_event_listener( + &document, + "paste", + |event: web_sys::ClipboardEvent, mut runner_lock| { if let Some(data) = event.clipboard_data() { if let Ok(text) = data.get_data("text") { - let mut runner_lock = runner_ref.0.lock(); runner_lock .input .raw @@ -443,85 +489,90 @@ fn install_document_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.prevent_default(); } } - }) as Box); - document.add_event_listener_with_callback("paste", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // cut - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |_: web_sys::ClipboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "cut", + |_: web_sys::ClipboardEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::Cut); runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("cut", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; #[cfg(web_sys_unstable_apis)] - { - // copy - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |_: web_sys::ClipboardEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "copy", + |_: web_sys::ClipboardEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::Copy); runner_lock.needs_repaint.set_true(); - }) as Box); - document.add_event_listener_with_callback("copy", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; for event_name in &["load", "pagehide", "pageshow", "resize"] { - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - runner_ref.0.lock().needs_repaint.set_true(); - }) as Box); - window.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); + runner_container.add_event_listener( + &document, + event_name, + |_: web_sys::Event, runner_lock| { + runner_lock.needs_repaint.set_true(); + }, + )?; } - { - // hashchange - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - let runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &document, + "hashchange", + |_: web_sys::Event, runner_lock| { let mut frame_lock = runner_lock.frame.lock(); // `epi::Frame::info(&self)` clones `epi::IntegrationInfo`, but we need to modify the original here if let Some(web_info) = &mut frame_lock.info.web_info { web_info.location.hash = location_hash(); } - }) as Box); - window.add_event_listener_with_callback("hashchange", closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; Ok(()) } /// Repaint at least every `ms` milliseconds. -fn repaint_every_ms(runner_ref: &AppRunnerRef, milliseconds: i32) -> Result<(), JsValue> { +pub fn repaint_every_ms( + runner_container: &AppRunnerContainer, + milliseconds: i32, +) -> Result<(), JsValue> { assert!(milliseconds >= 0); + use wasm_bindgen::JsCast; + let window = web_sys::window().unwrap(); - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move || { - runner_ref.0.lock().needs_repaint.set_true(); + + let closure = Closure::wrap(Box::new({ + let runner = runner_container.runner.clone(); + let panicked = runner_container.panicked.clone(); + + move || { + // Do not lock the runner if the code has panicked + if !panicked.load(Ordering::SeqCst) { + runner.lock().needs_repaint.set_true(); + } + } }) as Box); + window.set_interval_with_callback_and_timeout_and_arguments_0( closure.as_ref().unchecked_ref(), milliseconds, )?; + closure.forget(); Ok(()) } -fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { +fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { use wasm_bindgen::JsCast; - let canvas = canvas_element(runner_ref.0.lock().canvas_id()).unwrap(); + let canvas = canvas_element(runner_container.runner.lock().canvas_id()).unwrap(); { // By default, right-clicks open a context menu. @@ -534,12 +585,11 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { closure.forget(); } - { - let event_name = "mousedown"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { + runner_container.add_event_listener( + &canvas, + "mousedown", + |event: web_sys::MouseEvent, mut runner_lock| { if let Some(button) = button_from_mouse_event(&event) { - let mut runner_lock = runner_ref.0.lock(); let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); let modifiers = runner_lock.input.raw.modifiers; runner_lock @@ -556,16 +606,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { } event.stop_propagation(); // Note: prevent_default breaks VSCode tab focusing, hence why we don't call it here. - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mousemove"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "mousemove", + |event: web_sys::MouseEvent, mut runner_lock| { let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); runner_lock .input @@ -575,17 +622,14 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mouseup"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { + runner_container.add_event_listener( + &canvas, + "mouseup", + |event: web_sys::MouseEvent, mut runner_lock| { if let Some(button) = button_from_mouse_event(&event) { - let mut runner_lock = runner_ref.0.lock(); let pos = pos_from_mouse_event(runner_lock.canvas_id(), &event); let modifiers = runner_lock.input.raw.modifiers; runner_lock @@ -604,30 +648,24 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { } event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "mouseleave"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::MouseEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "mouseleave", + |event: web_sys::MouseEvent, mut runner_lock| { runner_lock.input.raw.events.push(egui::Event::PointerGone); runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "touchstart"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "touchstart", + |event: web_sys::TouchEvent, mut runner_lock| { let mut latest_touch_pos_id = runner_lock.input.latest_touch_pos_id; let pos = pos_from_touch_event(runner_lock.canvas_id(), &event, &mut latest_touch_pos_id); @@ -649,16 +687,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "touchmove"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "touchmove", + |event: web_sys::TouchEvent, mut runner_lock| { let mut latest_touch_pos_id = runner_lock.input.latest_touch_pos_id; let pos = pos_from_touch_event(runner_lock.canvas_id(), &event, &mut latest_touch_pos_id); @@ -674,17 +709,13 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } - - { - let event_name = "touchend"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + }, + )?; + runner_container.add_event_listener( + &canvas, + "touchend", + |event: web_sys::TouchEvent, mut runner_lock| { if let Some(pos) = runner_lock.input.latest_touch_pos { let modifiers = runner_lock.input.raw.modifiers; // First release mouse to click: @@ -709,33 +740,26 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { // Finally, focus or blur text agent to toggle mobile keyboard: text_agent::update_text_agent(&runner_lock); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "touchcancel"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::TouchEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "touchcancel", + |event: web_sys::TouchEvent, mut runner_lock| { push_touches(&mut *runner_lock, egui::TouchPhase::Cancel, &event); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } - - { - let event_name = "wheel"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::WheelEvent| { - let mut runner_lock = runner_ref.0.lock(); + }, + )?; + runner_container.add_event_listener( + &canvas, + "wheel", + |event: web_sys::WheelEvent, mut runner_lock| { let scroll_multiplier = match event.delta_mode() { web_sys::WheelEvent::DOM_DELTA_PAGE => { - canvas_size_in_points(runner_ref.0.lock().canvas_id()).y + canvas_size_in_points(runner_lock.canvas_id()).y } web_sys::WheelEvent::DOM_DELTA_LINE => { #[allow(clippy::let_and_return)] @@ -771,17 +795,14 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "dragover"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { + runner_container.add_event_listener( + &canvas, + "dragover", + |event: web_sys::DragEvent, mut runner_lock| { if let Some(data_transfer) = event.data_transfer() { - let mut runner_lock = runner_ref.0.lock(); runner_lock.input.raw.hovered_files.clear(); for i in 0..data_transfer.items().length() { if let Some(item) = data_transfer.items().get(i) { @@ -795,35 +816,28 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.stop_propagation(); event.prevent_default(); } - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "dragleave"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { - let mut runner_lock = runner_ref.0.lock(); + runner_container.add_event_listener( + &canvas, + "dragleave", + |event: web_sys::DragEvent, mut runner_lock| { runner_lock.input.raw.hovered_files.clear(); runner_lock.needs_repaint.set_true(); event.stop_propagation(); event.prevent_default(); - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + }, + )?; - { - let event_name = "drop"; - let runner_ref = runner_ref.clone(); - let closure = Closure::wrap(Box::new(move |event: web_sys::DragEvent| { + runner_container.add_event_listener(&canvas, "drop", { + let runner_ref = runner_container.runner.clone(); + + move |event: web_sys::DragEvent, mut runner_lock| { if let Some(data_transfer) = event.data_transfer() { - { - let mut runner_lock = runner_ref.0.lock(); - runner_lock.input.raw.hovered_files.clear(); - runner_lock.needs_repaint.set_true(); - } + runner_lock.input.raw.hovered_files.clear(); + runner_lock.needs_repaint.set_true(); + drop(runner_lock); if let Some(files) = data_transfer.files() { for i in 0..files.length() { @@ -847,7 +861,7 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { bytes.len() ); - let mut runner_lock = runner_ref.0.lock(); + let mut runner_lock = runner_ref.lock(); runner_lock.input.raw.dropped_files.push( egui::DroppedFile { name, @@ -870,10 +884,8 @@ fn install_canvas_events(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { event.stop_propagation(); event.prevent_default(); } - }) as Box); - canvas.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; - closure.forget(); - } + } + })?; Ok(()) } diff --git a/egui_web/src/text_agent.rs b/egui_web/src/text_agent.rs index 2fbaa1b0ac2..ceca2935190 100644 --- a/egui_web/src/text_agent.rs +++ b/egui_web/src/text_agent.rs @@ -1,7 +1,8 @@ //! The text agent is an `` element used to trigger //! mobile keyboard and IME input. -use crate::{canvas_element, AppRunner, AppRunnerRef}; +use crate::{canvas_element, AppRunner, AppRunnerContainer}; +use egui::mutex::MutexGuard; use std::cell::Cell; use std::rc::Rc; use wasm_bindgen::prelude::*; @@ -21,7 +22,7 @@ pub fn text_agent() -> web_sys::HtmlInputElement { } /// Text event handler, -pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { +pub fn install_text_agent(runner_container: &AppRunnerContainer) -> Result<(), JsValue> { use wasm_bindgen::JsCast; let window = web_sys::window().unwrap(); let document = window.document().unwrap(); @@ -43,61 +44,73 @@ pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { input.set_size(1); input.set_autofocus(true); input.set_hidden(true); - { - // When IME is off + + // When IME is off + runner_container.add_event_listener(&input, "input", { let input_clone = input.clone(); - let runner_ref = runner_ref.clone(); let is_composing = is_composing.clone(); - let on_input = Closure::wrap(Box::new(move |_event: web_sys::InputEvent| { + + move |_event: web_sys::InputEvent, mut runner_lock| { let text = input_clone.value(); if !text.is_empty() && !is_composing.get() { input_clone.set_value(""); - let mut runner_lock = runner_ref.0.lock(); runner_lock.input.raw.events.push(egui::Event::Text(text)); runner_lock.needs_repaint.set_true(); } - }) as Box); - input.add_event_listener_with_callback("input", on_input.as_ref().unchecked_ref())?; - on_input.forget(); - } + } + })?; + { // When IME is on, handle composition event - let input_clone = input.clone(); - let runner_ref = runner_ref.clone(); - let on_compositionend = Closure::wrap(Box::new(move |event: web_sys::CompositionEvent| { - let mut runner_lock = runner_ref.0.lock(); - let opt_event = match event.type_().as_ref() { - "compositionstart" => { - is_composing.set(true); - input_clone.set_value(""); - Some(egui::Event::CompositionStart) - } - "compositionend" => { - is_composing.set(false); - input_clone.set_value(""); - event.data().map(egui::Event::CompositionEnd) + runner_container.add_event_listener(&input, "compositionstart", { + let input_clone = input.clone(); + let is_composing = is_composing.clone(); + + move |_event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + is_composing.set(true); + input_clone.set_value(""); + + runner_lock + .input + .raw + .events + .push(egui::Event::CompositionStart); + runner_lock.needs_repaint.set_true(); + } + })?; + + runner_container.add_event_listener( + &input, + "compositionupdate", + move |event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + if let Some(event) = event.data().map(egui::Event::CompositionUpdate) { + runner_lock.input.raw.events.push(event); + runner_lock.needs_repaint.set_true(); } - "compositionupdate" => event.data().map(egui::Event::CompositionUpdate), - s => { - tracing::error!("Unknown composition event type: {:?}", s); - None + }, + )?; + + runner_container.add_event_listener(&input, "compositionend", { + let input_clone = input.clone(); + + move |event: web_sys::CompositionEvent, mut runner_lock: MutexGuard<'_, AppRunner>| { + is_composing.set(false); + input_clone.set_value(""); + + if let Some(event) = event.data().map(egui::Event::CompositionEnd) { + runner_lock.input.raw.events.push(event); + runner_lock.needs_repaint.set_true(); } - }; - if let Some(event) = opt_event { - runner_lock.input.raw.events.push(event); - runner_lock.needs_repaint.set_true(); } - }) as Box); - let f = on_compositionend.as_ref().unchecked_ref(); - input.add_event_listener_with_callback("compositionstart", f)?; - input.add_event_listener_with_callback("compositionupdate", f)?; - input.add_event_listener_with_callback("compositionend", f)?; - on_compositionend.forget(); + })?; } - { - // When input lost focus, focus on it again. - // It is useful when user click somewhere outside canvas. - let on_focusout = Closure::wrap(Box::new(move |_event: web_sys::MouseEvent| { + + // When input lost focus, focus on it again. + // It is useful when user click somewhere outside canvas. + runner_container.add_event_listener( + &input, + "focusout", + move |_event: web_sys::MouseEvent, _| { // Delay 10 ms, and focus again. let func = js_sys::Function::new_no_args(&format!( "document.getElementById('{}').focus()", @@ -106,11 +119,11 @@ pub fn install_text_agent(runner_ref: &AppRunnerRef) -> Result<(), JsValue> { window .set_timeout_with_callback_and_timeout_and_arguments_0(&func, 10) .unwrap(); - }) as Box); - input.add_event_listener_with_callback("focusout", on_focusout.as_ref().unchecked_ref())?; - on_focusout.forget(); - } + }, + )?; + body.append_child(&input)?; + Ok(()) } From 37dd927508852a064f94354390d68b4b5e7e68c5 Mon Sep 17 00:00:00 2001 From: Zachary Date: Fri, 25 Feb 2022 21:31:49 +0100 Subject: [PATCH 2/6] Update CHANGELOG --- egui_web/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/egui_web/CHANGELOG.md b/egui_web/CHANGELOG.md index 18752976e24..79754635e09 100644 --- a/egui_web/CHANGELOG.md +++ b/egui_web/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to the `egui_web` integration will be noted in this file. ## Unreleased +* egui code will no longer be called after panic ([#1306](https://github.com/emilk/egui/pull/1306)) ## 0.17.0 - 2022-02-22 From 510c4bbc2766d5d43c464ca536c0114cc31363aa Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 7 Mar 2022 11:01:26 +0100 Subject: [PATCH 3/6] Add docstring for `panicked` --- egui_web/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/egui_web/src/lib.rs b/egui_web/src/lib.rs index 98d029d32b2..cf0e3f344b4 100644 --- a/egui_web/src/lib.rs +++ b/egui_web/src/lib.rs @@ -308,6 +308,8 @@ pub type AppRunnerRef = Arc>; pub struct AppRunnerContainer { runner: AppRunnerRef, + /// Set to `true` if there is a panic. + /// Used to ignore callbacks after a panic. panicked: Arc, } From 741911db8c33cbceeb5d74d54c00509fa8b1d1ad Mon Sep 17 00:00:00 2001 From: Zachary Date: Tue, 8 Mar 2022 21:12:10 +0100 Subject: [PATCH 4/6] Replace RefCell with Cell since it was used like one anyways --- egui/src/widgets/plot/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/egui/src/widgets/plot/mod.rs b/egui/src/widgets/plot/mod.rs index b6f080960a6..8873e92855c 100644 --- a/egui/src/widgets/plot/mod.rs +++ b/egui/src/widgets/plot/mod.rs @@ -1,6 +1,6 @@ //! Simple plotting library. -use std::{cell::RefCell, ops::RangeInclusive, rc::Rc}; +use std::{cell::Cell, ops::RangeInclusive, rc::Rc}; use crate::*; use epaint::ahash::AHashSet; @@ -92,7 +92,7 @@ impl PlotMemory { pub struct LinkedAxisGroup { pub(crate) link_x: bool, pub(crate) link_y: bool, - pub(crate) bounds: Rc>>, + pub(crate) bounds: Rc>>, } impl LinkedAxisGroup { @@ -100,7 +100,7 @@ impl LinkedAxisGroup { Self { link_x, link_y, - bounds: Rc::new(RefCell::new(None)), + bounds: Rc::new(Cell::new(None)), } } @@ -132,11 +132,11 @@ impl LinkedAxisGroup { } fn get(&self) -> Option { - *self.bounds.borrow() + self.bounds.get() } fn set(&self, bounds: PlotBounds) { - *self.bounds.borrow_mut() = Some(bounds); + self.bounds.set(Some(bounds)); } } From ff6f875296f6dac327d55c588931d7ffc2a55f0b Mon Sep 17 00:00:00 2001 From: Zachary Date: Tue, 8 Mar 2022 21:13:23 +0100 Subject: [PATCH 5/6] Fix panic reported by @Titaniumtown See https://github.com/emilk/egui/pull/1306#issuecomment-1060775376 --- egui_web/src/lib.rs | 20 ++++++++++---------- egui_web/src/text_agent.rs | 15 ++++++++++++++- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/egui_web/src/lib.rs b/egui_web/src/lib.rs index cf0e3f344b4..1f271eadbf1 100644 --- a/egui_web/src/lib.rs +++ b/egui_web/src/lib.rs @@ -325,23 +325,21 @@ impl AppRunnerContainer { use wasm_bindgen::JsCast; // Create a JS closure based on the FnMut provided - let closure = Closure::wrap(Box::new({ + let closure = Closure::wrap({ // Clone atomics let runner_ref = self.runner.clone(); let panicked = self.panicked.clone(); - move |event: web_sys::Event| { + Box::new(move |event: web_sys::Event| { // Only call the wrapped closure if the egui code has not panicked if !panicked.load(Ordering::SeqCst) { // Cast the event to the expected event type let event = event.unchecked_into::(); - let lock = runner_ref.lock(); - - closure(event, lock); + closure(event, runner_ref.lock()); } - } - }) as Box); + }) as Box + }); // Add the event listener to the target target.add_event_listener_with_callback(event_name, closure.as_ref().unchecked_ref())?; @@ -646,7 +644,7 @@ fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), Js }); runner_lock.needs_repaint.set_true(); - text_agent::update_text_agent(&runner_lock); + text_agent::update_text_agent(runner_lock); } event.stop_propagation(); event.prevent_default(); @@ -741,7 +739,7 @@ fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), Js } // Finally, focus or blur text agent to toggle mobile keyboard: - text_agent::update_text_agent(&runner_lock); + text_agent::update_text_agent(runner_lock); }, )?; @@ -749,7 +747,7 @@ fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), Js &canvas, "touchcancel", |event: web_sys::TouchEvent, mut runner_lock| { - push_touches(&mut *runner_lock, egui::TouchPhase::Cancel, &event); + push_touches(&mut runner_lock, egui::TouchPhase::Cancel, &event); event.stop_propagation(); event.prevent_default(); }, @@ -839,6 +837,7 @@ fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), Js if let Some(data_transfer) = event.data_transfer() { runner_lock.input.raw.hovered_files.clear(); runner_lock.needs_repaint.set_true(); + // Unlock the runner so it can be locked after a future await point drop(runner_lock); if let Some(files) = data_transfer.files() { @@ -863,6 +862,7 @@ fn install_canvas_events(runner_container: &AppRunnerContainer) -> Result<(), Js bytes.len() ); + // Re-lock the mutex on the other side of the await point let mut runner_lock = runner_ref.lock(); runner_lock.input.raw.dropped_files.push( egui::DroppedFile { diff --git a/egui_web/src/text_agent.rs b/egui_web/src/text_agent.rs index ceca2935190..2c7bc054868 100644 --- a/egui_web/src/text_agent.rs +++ b/egui_web/src/text_agent.rs @@ -128,7 +128,7 @@ pub fn install_text_agent(runner_container: &AppRunnerContainer) -> Result<(), J } /// Focus or blur text agent to toggle mobile keyboard. -pub fn update_text_agent(runner: &AppRunner) -> Option<()> { +pub fn update_text_agent(runner: MutexGuard<'_, AppRunner>) -> Option<()> { use wasm_bindgen::JsCast; use web_sys::HtmlInputElement; let window = web_sys::window()?; @@ -169,7 +169,20 @@ pub fn update_text_agent(runner: &AppRunner) -> Option<()> { } } } else { + // Drop runner lock + drop(runner); + + // Holding the runner lock while calling input.blur() causes a panic. + // This is most probably caused by the browser running the event handler + // synchronously, meaning that the runner does not get dropped when another + // event handler is called. + // + // Why this didn't exist before #1290 is a mystery to me, but it exists now + // and this apparently is the fix for it + // + // ¯\_(ツ)_/¯ - @DusterTheFirst input.blur().ok()?; + input.set_hidden(true); canvas_style.set_property("position", "absolute").ok()?; canvas_style.set_property("top", "0%").ok()?; // move back to normal position From 3e97e51fc3d9753135478f0a8ac0643c95943b4d Mon Sep 17 00:00:00 2001 From: Zachary Date: Tue, 8 Mar 2022 21:25:51 +0100 Subject: [PATCH 6/6] Clarify message --- egui_web/src/text_agent.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/egui_web/src/text_agent.rs b/egui_web/src/text_agent.rs index 2c7bc054868..ef2af8d89b8 100644 --- a/egui_web/src/text_agent.rs +++ b/egui_web/src/text_agent.rs @@ -174,8 +174,8 @@ pub fn update_text_agent(runner: MutexGuard<'_, AppRunner>) -> Option<()> { // Holding the runner lock while calling input.blur() causes a panic. // This is most probably caused by the browser running the event handler - // synchronously, meaning that the runner does not get dropped when another - // event handler is called. + // for the triggered blur event synchronously, meaning that the mutex + // lock does not get dropped by the time another event handler is called. // // Why this didn't exist before #1290 is a mystery to me, but it exists now // and this apparently is the fix for it