From c1d26234f1c96ed7817c3061248b78c955f16543 Mon Sep 17 00:00:00 2001
From: jprochazk <1665677+jprochazk@users.noreply.github.com>
Date: Wed, 13 Mar 2024 17:03:43 +0100
Subject: [PATCH 1/4] call `cancel_animation_frame` in `destroy`

---
 crates/eframe/src/web/events.rs     | 12 ++------
 crates/eframe/src/web/web_runner.rs | 44 ++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs
index 71fe04e0c142..2cf5941f4df8 100644
--- a/crates/eframe/src/web/events.rs
+++ b/crates/eframe/src/web/events.rs
@@ -5,12 +5,12 @@ use super::*;
 /// Calls `request_animation_frame` to schedule repaint.
 ///
 /// It will only paint if needed, but will always call `request_animation_frame` immediately.
-fn paint_and_schedule(runner_ref: &WebRunner) -> Result<(), JsValue> {
+pub(crate) fn paint_and_schedule(runner_ref: &WebRunner) -> Result<(), JsValue> {
     // Only paint and schedule if there has been no panic
     if let Some(mut runner_lock) = runner_ref.try_lock() {
         paint_if_needed(&mut runner_lock);
         drop(runner_lock);
-        request_animation_frame(runner_ref.clone())?;
+        runner_ref.request_animation_frame()?;
     }
     Ok(())
 }
@@ -45,14 +45,6 @@ fn paint_if_needed(runner: &mut AppRunner) {
     runner.auto_save_if_needed();
 }
 
-pub(crate) fn request_animation_frame(runner_ref: WebRunner) -> Result<(), JsValue> {
-    let window = web_sys::window().unwrap();
-    let closure = Closure::once(move || paint_and_schedule(&runner_ref));
-    window.request_animation_frame(closure.as_ref().unchecked_ref())?;
-    closure.forget(); // We must forget it, or else the callback is canceled on drop
-    Ok(())
-}
-
 // ------------------------------------------------------------------------
 
 pub(crate) fn install_document_events(runner_ref: &WebRunner) -> Result<(), JsValue> {
diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs
index 602c566e9d73..802935848dca 100644
--- a/crates/eframe/src/web/web_runner.rs
+++ b/crates/eframe/src/web/web_runner.rs
@@ -1,4 +1,7 @@
-use std::{cell::RefCell, rc::Rc};
+use std::{
+    cell::{Cell, RefCell},
+    rc::Rc,
+};
 
 use wasm_bindgen::prelude::*;
 
@@ -24,6 +27,9 @@ pub struct WebRunner {
     /// They have to be in a separate `Rc` so that we don't need to pass them to
     /// the panic handler, since they aren't `Send`.
     events_to_unsubscribe: Rc<RefCell<Vec<EventToUnsubscribe>>>,
+
+    /// Used in `destroy` to cancel a pending frame.
+    raf_id: Cell<Option<i32>>,
 }
 
 impl WebRunner {
@@ -41,6 +47,7 @@ impl WebRunner {
             panic_handler,
             runner: Rc::new(RefCell::new(None)),
             events_to_unsubscribe: Rc::new(RefCell::new(Default::default())),
+            raf_id: Cell::new(None),
         }
     }
 
@@ -71,7 +78,7 @@ impl WebRunner {
                 events::install_color_scheme_change_event(self)?;
             }
 
-            events::request_animation_frame(self.clone())?;
+            self.request_animation_frame()?;
         }
 
         Ok(())
@@ -108,6 +115,11 @@ impl WebRunner {
     pub fn destroy(&self) {
         self.unsubscribe_from_all_events();
 
+        if let Some(id) = self.raf_id.get() {
+            let window = web_sys::window().unwrap();
+            window.cancel_animation_frame(id).ok();
+        }
+
         if let Some(runner) = self.runner.replace(None) {
             runner.destroy();
         }
@@ -179,23 +191,35 @@ impl WebRunner {
 
         Ok(())
     }
+
+    pub(crate) fn request_animation_frame(&self) -> Result<(), wasm_bindgen::JsValue> {
+        let window = web_sys::window().unwrap();
+        let closure = Closure::once({
+            let runner_ref = self.clone();
+            move || events::paint_and_schedule(&runner_ref)
+        });
+        let id = window.request_animation_frame(closure.as_ref().unchecked_ref())?;
+        self.raf_id.set(Some(id));
+        closure.forget(); // We must forget it, or else the callback is canceled on drop
+        Ok(())
+    }
 }
 
 // ----------------------------------------------------------------------------
 
-struct TargetEvent {
-    target: web_sys::EventTarget,
-    event_name: String,
-    closure: Closure<dyn FnMut(web_sys::Event)>,
+pub(super) struct TargetEvent {
+    pub target: web_sys::EventTarget,
+    pub event_name: String,
+    pub closure: Closure<dyn FnMut(web_sys::Event)>,
 }
 
 #[allow(unused)]
-struct IntervalHandle {
-    handle: i32,
-    closure: Closure<dyn FnMut()>,
+pub(super) struct IntervalHandle {
+    pub handle: i32,
+    pub closure: Closure<dyn FnMut()>,
 }
 
-enum EventToUnsubscribe {
+pub(super) enum EventToUnsubscribe {
     TargetEvent(TargetEvent),
 
     #[allow(unused)]

From 71db2c048bc6510a4ee710628cf8bab9acbf5a24 Mon Sep 17 00:00:00 2001
From: jprochazk <1665677+jprochazk@users.noreply.github.com>
Date: Wed, 13 Mar 2024 17:17:49 +0100
Subject: [PATCH 2/4] use stored reference instead of finding canvas by id all
 the time

---
 crates/eframe/src/web/app_runner.rs       | 10 +++++-----
 crates/eframe/src/web/events.rs           | 14 +++++++-------
 crates/eframe/src/web/input.rs            | 14 ++++++++------
 crates/eframe/src/web/mod.rs              | 21 ++++++++++-----------
 crates/eframe/src/web/text_agent.rs       | 10 ++++++----
 crates/eframe/src/web/web_painter.rs      |  4 ++--
 crates/eframe/src/web/web_painter_glow.rs | 13 ++++---------
 crates/eframe/src/web/web_painter_wgpu.rs |  8 +++-----
 8 files changed, 45 insertions(+), 49 deletions(-)

diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs
index d63469110c7c..97a3ac98ce36 100644
--- a/crates/eframe/src/web/app_runner.rs
+++ b/crates/eframe/src/web/app_runner.rs
@@ -165,8 +165,8 @@ impl AppRunner {
         self.last_save_time = now_sec();
     }
 
-    pub fn canvas_id(&self) -> &str {
-        self.painter.canvas_id()
+    pub fn canvas(&self) -> web_sys::HtmlCanvasElement {
+        self.painter.canvas()
     }
 
     pub fn destroy(mut self) {
@@ -182,8 +182,8 @@ impl AppRunner {
     ///
     /// The result can be painted later with a call to [`Self::run_and_paint`] or [`Self::paint`].
     pub fn logic(&mut self) {
-        super::resize_canvas_to_screen_size(self.canvas_id(), self.web_options.max_size_points);
-        let canvas_size = super::canvas_size_in_points(self.canvas_id());
+        super::resize_canvas_to_screen_size(&self.canvas(), self.web_options.max_size_points);
+        let canvas_size = super::canvas_size_in_points(&self.canvas());
         let raw_input = self.input.new_frame(canvas_size);
 
         let full_output = self.egui_ctx.run(raw_input, |egui_ctx| {
@@ -268,7 +268,7 @@ impl AppRunner {
         self.mutable_text_under_cursor = mutable_text_under_cursor;
 
         if self.ime != ime {
-            super::text_agent::move_text_cursor(ime, self.canvas_id());
+            super::text_agent::move_text_cursor(ime, self.canvas());
             self.ime = ime;
         }
     }
diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs
index 2cf5941f4df8..e0b41370412c 100644
--- a/crates/eframe/src/web/events.rs
+++ b/crates/eframe/src/web/events.rs
@@ -267,7 +267,7 @@ pub(crate) fn install_color_scheme_change_event(runner_ref: &WebRunner) -> Resul
 }
 
 pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValue> {
-    let canvas = canvas_element(runner_ref.try_lock().unwrap().canvas_id()).unwrap();
+    let canvas = runner_ref.try_lock().unwrap().canvas();
 
     {
         let prevent_default_events = [
@@ -296,7 +296,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
             let modifiers = modifiers_from_mouse_event(&event);
             runner.input.raw.modifiers = modifiers;
             if let Some(button) = button_from_mouse_event(&event) {
-                let pos = pos_from_mouse_event(runner.canvas_id(), &event);
+                let pos = pos_from_mouse_event(&runner.canvas(), &event);
                 let modifiers = runner.input.raw.modifiers;
                 runner.input.raw.events.push(egui::Event::PointerButton {
                     pos,
@@ -323,7 +323,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         |event: web_sys::MouseEvent, runner| {
             let modifiers = modifiers_from_mouse_event(&event);
             runner.input.raw.modifiers = modifiers;
-            let pos = pos_from_mouse_event(runner.canvas_id(), &event);
+            let pos = pos_from_mouse_event(&runner.canvas(), &event);
             runner.input.raw.events.push(egui::Event::PointerMoved(pos));
             runner.needs_repaint.repaint_asap();
             event.stop_propagation();
@@ -335,7 +335,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         let modifiers = modifiers_from_mouse_event(&event);
         runner.input.raw.modifiers = modifiers;
         if let Some(button) = button_from_mouse_event(&event) {
-            let pos = pos_from_mouse_event(runner.canvas_id(), &event);
+            let pos = pos_from_mouse_event(&runner.canvas(), &event);
             let modifiers = runner.input.raw.modifiers;
             runner.input.raw.events.push(egui::Event::PointerButton {
                 pos,
@@ -373,7 +373,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         "touchstart",
         |event: web_sys::TouchEvent, runner| {
             let mut latest_touch_pos_id = runner.input.latest_touch_pos_id;
-            let pos = pos_from_touch_event(runner.canvas_id(), &event, &mut latest_touch_pos_id);
+            let pos = pos_from_touch_event(&runner.canvas(), &event, &mut latest_touch_pos_id);
             runner.input.latest_touch_pos_id = latest_touch_pos_id;
             runner.input.latest_touch_pos = Some(pos);
             let modifiers = runner.input.raw.modifiers;
@@ -396,7 +396,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         "touchmove",
         |event: web_sys::TouchEvent, runner| {
             let mut latest_touch_pos_id = runner.input.latest_touch_pos_id;
-            let pos = pos_from_touch_event(runner.canvas_id(), &event, &mut latest_touch_pos_id);
+            let pos = pos_from_touch_event(&runner.canvas(), &event, &mut latest_touch_pos_id);
             runner.input.latest_touch_pos_id = latest_touch_pos_id;
             runner.input.latest_touch_pos = Some(pos);
             runner.input.raw.events.push(egui::Event::PointerMoved(pos));
@@ -459,7 +459,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         });
 
         let scroll_multiplier = match unit {
-            egui::MouseWheelUnit::Page => canvas_size_in_points(runner.canvas_id()).y,
+            egui::MouseWheelUnit::Page => canvas_size_in_points(&runner.canvas()).y,
             egui::MouseWheelUnit::Line => {
                 #[allow(clippy::let_and_return)]
                 let points_per_scroll_line = 8.0; // Note that this is intentionally different from what we use in winit.
diff --git a/crates/eframe/src/web/input.rs b/crates/eframe/src/web/input.rs
index 223693f42633..65b9d1782f0d 100644
--- a/crates/eframe/src/web/input.rs
+++ b/crates/eframe/src/web/input.rs
@@ -1,7 +1,9 @@
-use super::{canvas_element, canvas_origin, AppRunner};
+use super::{canvas_origin, AppRunner};
 
-pub fn pos_from_mouse_event(canvas_id: &str, event: &web_sys::MouseEvent) -> egui::Pos2 {
-    let canvas = canvas_element(canvas_id).unwrap();
+pub fn pos_from_mouse_event(
+    canvas: &web_sys::HtmlCanvasElement,
+    event: &web_sys::MouseEvent,
+) -> egui::Pos2 {
     let rect = canvas.get_bounding_client_rect();
     egui::Pos2 {
         x: event.client_x() as f32 - rect.left() as f32,
@@ -27,7 +29,7 @@ pub fn button_from_mouse_event(event: &web_sys::MouseEvent) -> Option<egui::Poin
 /// `touch_id_for_pos` is the [`TouchId`](egui::TouchId) of the [`Touch`](web_sys::Touch) we previously used to determine the
 /// pointer position.
 pub fn pos_from_touch_event(
-    canvas_id: &str,
+    canvas: &web_sys::HtmlCanvasElement,
     event: &web_sys::TouchEvent,
     touch_id_for_pos: &mut Option<egui::TouchId>,
 ) -> egui::Pos2 {
@@ -47,7 +49,7 @@ pub fn pos_from_touch_event(
         .or_else(|| event.touches().get(0))
         .map_or(Default::default(), |touch| {
             *touch_id_for_pos = Some(egui::TouchId::from(touch.identifier()));
-            pos_from_touch(canvas_origin(canvas_id), &touch)
+            pos_from_touch(canvas_origin(canvas), &touch)
         })
 }
 
@@ -59,7 +61,7 @@ fn pos_from_touch(canvas_origin: egui::Pos2, touch: &web_sys::Touch) -> egui::Po
 }
 
 pub fn push_touches(runner: &mut AppRunner, phase: egui::TouchPhase, event: &web_sys::TouchEvent) {
-    let canvas_origin = canvas_origin(runner.canvas_id());
+    let canvas_origin = canvas_origin(&runner.canvas());
     for touch_idx in 0..event.changed_touches().length() {
         if let Some(touch) = event.changed_touches().item(touch_idx) {
             runner.input.raw.events.push(egui::Event::Touch {
diff --git a/crates/eframe/src/web/mod.rs b/crates/eframe/src/web/mod.rs
index cb5d6937c4c9..d88e94229fd0 100644
--- a/crates/eframe/src/web/mod.rs
+++ b/crates/eframe/src/web/mod.rs
@@ -100,26 +100,23 @@ fn theme_from_dark_mode(dark_mode: bool) -> Theme {
     }
 }
 
-fn canvas_element(canvas_id: &str) -> Option<web_sys::HtmlCanvasElement> {
+fn get_canvas_element_by_id(canvas_id: &str) -> Option<web_sys::HtmlCanvasElement> {
     let document = web_sys::window()?.document()?;
     let canvas = document.get_element_by_id(canvas_id)?;
     canvas.dyn_into::<web_sys::HtmlCanvasElement>().ok()
 }
 
-fn canvas_element_or_die(canvas_id: &str) -> web_sys::HtmlCanvasElement {
-    canvas_element(canvas_id)
+fn get_canvas_element_by_id_or_die(canvas_id: &str) -> web_sys::HtmlCanvasElement {
+    get_canvas_element_by_id(canvas_id)
         .unwrap_or_else(|| panic!("Failed to find canvas with id {canvas_id:?}"))
 }
 
-fn canvas_origin(canvas_id: &str) -> egui::Pos2 {
-    let rect = canvas_element(canvas_id)
-        .unwrap()
-        .get_bounding_client_rect();
+fn canvas_origin(canvas: &web_sys::HtmlCanvasElement) -> egui::Pos2 {
+    let rect = canvas.get_bounding_client_rect();
     egui::pos2(rect.left() as f32, rect.top() as f32)
 }
 
-fn canvas_size_in_points(canvas_id: &str) -> egui::Vec2 {
-    let canvas = canvas_element(canvas_id).unwrap();
+fn canvas_size_in_points(canvas: &web_sys::HtmlCanvasElement) -> egui::Vec2 {
     let pixels_per_point = native_pixels_per_point();
     egui::vec2(
         canvas.width() as f32 / pixels_per_point,
@@ -127,8 +124,10 @@ fn canvas_size_in_points(canvas_id: &str) -> egui::Vec2 {
     )
 }
 
-fn resize_canvas_to_screen_size(canvas_id: &str, max_size_points: egui::Vec2) -> Option<()> {
-    let canvas = canvas_element(canvas_id)?;
+fn resize_canvas_to_screen_size(
+    canvas: &web_sys::HtmlCanvasElement,
+    max_size_points: egui::Vec2,
+) -> Option<()> {
     let parent = canvas.parent_element()?;
 
     // Prefer the client width and height so that if the parent
diff --git a/crates/eframe/src/web/text_agent.rs b/crates/eframe/src/web/text_agent.rs
index 0bf8b532b7c8..5dbbe5b4b925 100644
--- a/crates/eframe/src/web/text_agent.rs
+++ b/crates/eframe/src/web/text_agent.rs
@@ -5,7 +5,7 @@ use std::{cell::Cell, rc::Rc};
 
 use wasm_bindgen::prelude::*;
 
-use super::{canvas_element, AppRunner, WebRunner};
+use super::{AppRunner, WebRunner};
 
 static AGENT_ID: &str = "egui_text_agent";
 
@@ -121,7 +121,7 @@ pub fn update_text_agent(runner: &mut AppRunner) -> Option<()> {
     let window = web_sys::window()?;
     let document = window.document()?;
     let input: HtmlInputElement = document.get_element_by_id(AGENT_ID)?.dyn_into().unwrap();
-    let canvas_style = canvas_element(runner.canvas_id())?.style();
+    let canvas_style = runner.canvas().style();
 
     if runner.mutable_text_under_cursor {
         let is_already_editing = input.hidden();
@@ -205,14 +205,16 @@ fn is_mobile() -> Option<bool> {
 // candidate window moves following text element (agent),
 // so it appears that the IME candidate window moves with text cursor.
 // On mobile devices, there is no need to do that.
-pub fn move_text_cursor(ime: Option<egui::output::IMEOutput>, canvas_id: &str) -> Option<()> {
+pub fn move_text_cursor(
+    ime: Option<egui::output::IMEOutput>,
+    canvas: web_sys::HtmlCanvasElement,
+) -> Option<()> {
     let style = text_agent().style();
     // Note: moving agent on mobile devices will lead to unpredictable scroll.
     if is_mobile() == Some(false) {
         ime.as_ref().and_then(|ime| {
             let egui::Pos2 { x, y } = ime.cursor_rect.left_top();
 
-            let canvas = canvas_element(canvas_id)?;
             let bounding_rect = text_agent().get_bounding_client_rect();
             let y = (y + (canvas.scroll_top() + canvas.offset_top()) as f32)
                 .min(canvas.client_height() as f32 - bounding_rect.height() as f32);
diff --git a/crates/eframe/src/web/web_painter.rs b/crates/eframe/src/web/web_painter.rs
index 9c7631b90eb9..3fc8c89fa0d4 100644
--- a/crates/eframe/src/web/web_painter.rs
+++ b/crates/eframe/src/web/web_painter.rs
@@ -9,8 +9,8 @@ pub(crate) trait WebPainter {
     // where
     //     Self: Sized;
 
-    /// Id of the canvas in use.
-    fn canvas_id(&self) -> &str;
+    /// Reference to the canvas in use.
+    fn canvas(&self) -> web_sys::HtmlCanvasElement;
 
     /// Maximum size of a texture in one direction.
     fn max_texture_side(&self) -> usize;
diff --git a/crates/eframe/src/web/web_painter_glow.rs b/crates/eframe/src/web/web_painter_glow.rs
index cd62758688f9..c6b823f075d0 100644
--- a/crates/eframe/src/web/web_painter_glow.rs
+++ b/crates/eframe/src/web/web_painter_glow.rs
@@ -10,7 +10,6 @@ use super::web_painter::WebPainter;
 
 pub(crate) struct WebPainterGlow {
     canvas: HtmlCanvasElement,
-    canvas_id: String,
     painter: egui_glow::Painter,
 }
 
@@ -20,7 +19,7 @@ impl WebPainterGlow {
     }
 
     pub async fn new(canvas_id: &str, options: &WebOptions) -> Result<Self, String> {
-        let canvas = super::canvas_element_or_die(canvas_id);
+        let canvas = super::get_canvas_element_by_id_or_die(canvas_id);
 
         let (gl, shader_prefix) =
             init_glow_context_from_canvas(&canvas, options.webgl_context_option)?;
@@ -30,11 +29,7 @@ impl WebPainterGlow {
         let painter = egui_glow::Painter::new(gl, shader_prefix, None)
             .map_err(|err| format!("Error starting glow painter: {err}"))?;
 
-        Ok(Self {
-            canvas,
-            canvas_id: canvas_id.to_owned(),
-            painter,
-        })
+        Ok(Self { canvas, painter })
     }
 }
 
@@ -43,8 +38,8 @@ impl WebPainter for WebPainterGlow {
         self.painter.max_texture_side()
     }
 
-    fn canvas_id(&self) -> &str {
-        &self.canvas_id
+    fn canvas(&self) -> HtmlCanvasElement {
+        self.canvas.clone()
     }
 
     fn paint_and_update_textures(
diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs
index 857a1f5404cd..70e9e3ccbf68 100644
--- a/crates/eframe/src/web/web_painter_wgpu.rs
+++ b/crates/eframe/src/web/web_painter_wgpu.rs
@@ -41,7 +41,6 @@ impl HasDisplayHandle for EguiWebWindow {
 
 pub(crate) struct WebPainterWgpu {
     canvas: HtmlCanvasElement,
-    canvas_id: String,
     surface: wgpu::Surface<'static>,
     surface_configuration: wgpu::SurfaceConfiguration,
     render_state: Option<RenderState>,
@@ -163,7 +162,7 @@ impl WebPainterWgpu {
             }
         }
 
-        let canvas = super::canvas_element_or_die(canvas_id);
+        let canvas = super::get_canvas_element_by_id_or_die(canvas_id);
         let surface = instance
             .create_surface(wgpu::SurfaceTarget::Canvas(canvas.clone()))
             .map_err(|err| format!("failed to create wgpu surface: {err}"))?;
@@ -188,7 +187,6 @@ impl WebPainterWgpu {
 
         Ok(Self {
             canvas,
-            canvas_id: canvas_id.to_owned(),
             render_state: Some(render_state),
             surface,
             surface_configuration,
@@ -200,8 +198,8 @@ impl WebPainterWgpu {
 }
 
 impl WebPainter for WebPainterWgpu {
-    fn canvas_id(&self) -> &str {
-        &self.canvas_id
+    fn canvas(&self) -> HtmlCanvasElement {
+        self.canvas.clone()
     }
 
     fn max_texture_side(&self) -> usize {

From 8d9caa4bb0beabfe9fedb9b6ada72ebb7337ed49 Mon Sep 17 00:00:00 2001
From: jprochazk <1665677+jprochazk@users.noreply.github.com>
Date: Wed, 13 Mar 2024 17:27:33 +0100
Subject: [PATCH 3/4] fix lints

---
 crates/eframe/src/web/app_runner.rs | 2 +-
 crates/eframe/src/web/text_agent.rs | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs
index 97a3ac98ce36..d222d964cc7c 100644
--- a/crates/eframe/src/web/app_runner.rs
+++ b/crates/eframe/src/web/app_runner.rs
@@ -268,7 +268,7 @@ impl AppRunner {
         self.mutable_text_under_cursor = mutable_text_under_cursor;
 
         if self.ime != ime {
-            super::text_agent::move_text_cursor(ime, self.canvas());
+            super::text_agent::move_text_cursor(ime, &self.canvas());
             self.ime = ime;
         }
     }
diff --git a/crates/eframe/src/web/text_agent.rs b/crates/eframe/src/web/text_agent.rs
index 5dbbe5b4b925..a879f99d0082 100644
--- a/crates/eframe/src/web/text_agent.rs
+++ b/crates/eframe/src/web/text_agent.rs
@@ -207,7 +207,7 @@ fn is_mobile() -> Option<bool> {
 // On mobile devices, there is no need to do that.
 pub fn move_text_cursor(
     ime: Option<egui::output::IMEOutput>,
-    canvas: web_sys::HtmlCanvasElement,
+    canvas: &web_sys::HtmlCanvasElement,
 ) -> Option<()> {
     let style = text_agent().style();
     // Note: moving agent on mobile devices will lead to unpredictable scroll.

From 096da410850ce27a009abdaecf10df596efd1e92 Mon Sep 17 00:00:00 2001
From: jprochazk <1665677+jprochazk@users.noreply.github.com>
Date: Thu, 14 Mar 2024 09:16:58 +0100
Subject: [PATCH 4/4] suggestions

---
 crates/eframe/src/web/app_runner.rs       |  8 ++++----
 crates/eframe/src/web/events.rs           | 14 ++++++-------
 crates/eframe/src/web/input.rs            |  2 +-
 crates/eframe/src/web/web_painter.rs      |  2 +-
 crates/eframe/src/web/web_painter_glow.rs |  4 ++--
 crates/eframe/src/web/web_painter_wgpu.rs |  4 ++--
 crates/eframe/src/web/web_runner.rs       | 24 +++++++++++------------
 7 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/crates/eframe/src/web/app_runner.rs b/crates/eframe/src/web/app_runner.rs
index d222d964cc7c..31f6e79c01c8 100644
--- a/crates/eframe/src/web/app_runner.rs
+++ b/crates/eframe/src/web/app_runner.rs
@@ -165,7 +165,7 @@ impl AppRunner {
         self.last_save_time = now_sec();
     }
 
-    pub fn canvas(&self) -> web_sys::HtmlCanvasElement {
+    pub fn canvas(&self) -> &web_sys::HtmlCanvasElement {
         self.painter.canvas()
     }
 
@@ -182,8 +182,8 @@ impl AppRunner {
     ///
     /// The result can be painted later with a call to [`Self::run_and_paint`] or [`Self::paint`].
     pub fn logic(&mut self) {
-        super::resize_canvas_to_screen_size(&self.canvas(), self.web_options.max_size_points);
-        let canvas_size = super::canvas_size_in_points(&self.canvas());
+        super::resize_canvas_to_screen_size(self.canvas(), self.web_options.max_size_points);
+        let canvas_size = super::canvas_size_in_points(self.canvas());
         let raw_input = self.input.new_frame(canvas_size);
 
         let full_output = self.egui_ctx.run(raw_input, |egui_ctx| {
@@ -268,7 +268,7 @@ impl AppRunner {
         self.mutable_text_under_cursor = mutable_text_under_cursor;
 
         if self.ime != ime {
-            super::text_agent::move_text_cursor(ime, &self.canvas());
+            super::text_agent::move_text_cursor(ime, self.canvas());
             self.ime = ime;
         }
     }
diff --git a/crates/eframe/src/web/events.rs b/crates/eframe/src/web/events.rs
index e0b41370412c..5e5f7b22622f 100644
--- a/crates/eframe/src/web/events.rs
+++ b/crates/eframe/src/web/events.rs
@@ -267,7 +267,7 @@ pub(crate) fn install_color_scheme_change_event(runner_ref: &WebRunner) -> Resul
 }
 
 pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValue> {
-    let canvas = runner_ref.try_lock().unwrap().canvas();
+    let canvas = runner_ref.try_lock().unwrap().canvas().clone();
 
     {
         let prevent_default_events = [
@@ -296,7 +296,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
             let modifiers = modifiers_from_mouse_event(&event);
             runner.input.raw.modifiers = modifiers;
             if let Some(button) = button_from_mouse_event(&event) {
-                let pos = pos_from_mouse_event(&runner.canvas(), &event);
+                let pos = pos_from_mouse_event(runner.canvas(), &event);
                 let modifiers = runner.input.raw.modifiers;
                 runner.input.raw.events.push(egui::Event::PointerButton {
                     pos,
@@ -323,7 +323,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         |event: web_sys::MouseEvent, runner| {
             let modifiers = modifiers_from_mouse_event(&event);
             runner.input.raw.modifiers = modifiers;
-            let pos = pos_from_mouse_event(&runner.canvas(), &event);
+            let pos = pos_from_mouse_event(runner.canvas(), &event);
             runner.input.raw.events.push(egui::Event::PointerMoved(pos));
             runner.needs_repaint.repaint_asap();
             event.stop_propagation();
@@ -335,7 +335,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         let modifiers = modifiers_from_mouse_event(&event);
         runner.input.raw.modifiers = modifiers;
         if let Some(button) = button_from_mouse_event(&event) {
-            let pos = pos_from_mouse_event(&runner.canvas(), &event);
+            let pos = pos_from_mouse_event(runner.canvas(), &event);
             let modifiers = runner.input.raw.modifiers;
             runner.input.raw.events.push(egui::Event::PointerButton {
                 pos,
@@ -373,7 +373,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         "touchstart",
         |event: web_sys::TouchEvent, runner| {
             let mut latest_touch_pos_id = runner.input.latest_touch_pos_id;
-            let pos = pos_from_touch_event(&runner.canvas(), &event, &mut latest_touch_pos_id);
+            let pos = pos_from_touch_event(runner.canvas(), &event, &mut latest_touch_pos_id);
             runner.input.latest_touch_pos_id = latest_touch_pos_id;
             runner.input.latest_touch_pos = Some(pos);
             let modifiers = runner.input.raw.modifiers;
@@ -396,7 +396,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         "touchmove",
         |event: web_sys::TouchEvent, runner| {
             let mut latest_touch_pos_id = runner.input.latest_touch_pos_id;
-            let pos = pos_from_touch_event(&runner.canvas(), &event, &mut latest_touch_pos_id);
+            let pos = pos_from_touch_event(runner.canvas(), &event, &mut latest_touch_pos_id);
             runner.input.latest_touch_pos_id = latest_touch_pos_id;
             runner.input.latest_touch_pos = Some(pos);
             runner.input.raw.events.push(egui::Event::PointerMoved(pos));
@@ -459,7 +459,7 @@ pub(crate) fn install_canvas_events(runner_ref: &WebRunner) -> Result<(), JsValu
         });
 
         let scroll_multiplier = match unit {
-            egui::MouseWheelUnit::Page => canvas_size_in_points(&runner.canvas()).y,
+            egui::MouseWheelUnit::Page => canvas_size_in_points(runner.canvas()).y,
             egui::MouseWheelUnit::Line => {
                 #[allow(clippy::let_and_return)]
                 let points_per_scroll_line = 8.0; // Note that this is intentionally different from what we use in winit.
diff --git a/crates/eframe/src/web/input.rs b/crates/eframe/src/web/input.rs
index 65b9d1782f0d..4ed0227739a1 100644
--- a/crates/eframe/src/web/input.rs
+++ b/crates/eframe/src/web/input.rs
@@ -61,7 +61,7 @@ fn pos_from_touch(canvas_origin: egui::Pos2, touch: &web_sys::Touch) -> egui::Po
 }
 
 pub fn push_touches(runner: &mut AppRunner, phase: egui::TouchPhase, event: &web_sys::TouchEvent) {
-    let canvas_origin = canvas_origin(&runner.canvas());
+    let canvas_origin = canvas_origin(runner.canvas());
     for touch_idx in 0..event.changed_touches().length() {
         if let Some(touch) = event.changed_touches().item(touch_idx) {
             runner.input.raw.events.push(egui::Event::Touch {
diff --git a/crates/eframe/src/web/web_painter.rs b/crates/eframe/src/web/web_painter.rs
index 3fc8c89fa0d4..e4db8eac3161 100644
--- a/crates/eframe/src/web/web_painter.rs
+++ b/crates/eframe/src/web/web_painter.rs
@@ -10,7 +10,7 @@ pub(crate) trait WebPainter {
     //     Self: Sized;
 
     /// Reference to the canvas in use.
-    fn canvas(&self) -> web_sys::HtmlCanvasElement;
+    fn canvas(&self) -> &web_sys::HtmlCanvasElement;
 
     /// Maximum size of a texture in one direction.
     fn max_texture_side(&self) -> usize;
diff --git a/crates/eframe/src/web/web_painter_glow.rs b/crates/eframe/src/web/web_painter_glow.rs
index c6b823f075d0..b54f6f64423d 100644
--- a/crates/eframe/src/web/web_painter_glow.rs
+++ b/crates/eframe/src/web/web_painter_glow.rs
@@ -38,8 +38,8 @@ impl WebPainter for WebPainterGlow {
         self.painter.max_texture_side()
     }
 
-    fn canvas(&self) -> HtmlCanvasElement {
-        self.canvas.clone()
+    fn canvas(&self) -> &HtmlCanvasElement {
+        &self.canvas
     }
 
     fn paint_and_update_textures(
diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs
index 70e9e3ccbf68..de5ba6011115 100644
--- a/crates/eframe/src/web/web_painter_wgpu.rs
+++ b/crates/eframe/src/web/web_painter_wgpu.rs
@@ -198,8 +198,8 @@ impl WebPainterWgpu {
 }
 
 impl WebPainter for WebPainterWgpu {
-    fn canvas(&self) -> HtmlCanvasElement {
-        self.canvas.clone()
+    fn canvas(&self) -> &HtmlCanvasElement {
+        &self.canvas
     }
 
     fn max_texture_side(&self) -> usize {
diff --git a/crates/eframe/src/web/web_runner.rs b/crates/eframe/src/web/web_runner.rs
index 802935848dca..f39fc0dcace6 100644
--- a/crates/eframe/src/web/web_runner.rs
+++ b/crates/eframe/src/web/web_runner.rs
@@ -29,7 +29,7 @@ pub struct WebRunner {
     events_to_unsubscribe: Rc<RefCell<Vec<EventToUnsubscribe>>>,
 
     /// Used in `destroy` to cancel a pending frame.
-    raf_id: Cell<Option<i32>>,
+    request_animation_frame_id: Cell<Option<i32>>,
 }
 
 impl WebRunner {
@@ -47,7 +47,7 @@ impl WebRunner {
             panic_handler,
             runner: Rc::new(RefCell::new(None)),
             events_to_unsubscribe: Rc::new(RefCell::new(Default::default())),
-            raf_id: Cell::new(None),
+            request_animation_frame_id: Cell::new(None),
         }
     }
 
@@ -115,7 +115,7 @@ impl WebRunner {
     pub fn destroy(&self) {
         self.unsubscribe_from_all_events();
 
-        if let Some(id) = self.raf_id.get() {
+        if let Some(id) = self.request_animation_frame_id.get() {
             let window = web_sys::window().unwrap();
             window.cancel_animation_frame(id).ok();
         }
@@ -199,7 +199,7 @@ impl WebRunner {
             move || events::paint_and_schedule(&runner_ref)
         });
         let id = window.request_animation_frame(closure.as_ref().unchecked_ref())?;
-        self.raf_id.set(Some(id));
+        self.request_animation_frame_id.set(Some(id));
         closure.forget(); // We must forget it, or else the callback is canceled on drop
         Ok(())
     }
@@ -207,19 +207,19 @@ impl WebRunner {
 
 // ----------------------------------------------------------------------------
 
-pub(super) struct TargetEvent {
-    pub target: web_sys::EventTarget,
-    pub event_name: String,
-    pub closure: Closure<dyn FnMut(web_sys::Event)>,
+struct TargetEvent {
+    target: web_sys::EventTarget,
+    event_name: String,
+    closure: Closure<dyn FnMut(web_sys::Event)>,
 }
 
 #[allow(unused)]
-pub(super) struct IntervalHandle {
-    pub handle: i32,
-    pub closure: Closure<dyn FnMut()>,
+struct IntervalHandle {
+    handle: i32,
+    closure: Closure<dyn FnMut()>,
 }
 
-pub(super) enum EventToUnsubscribe {
+enum EventToUnsubscribe {
     TargetEvent(TargetEvent),
 
     #[allow(unused)]