Web: forbid additional functions in favor of caching them (#3219)

clippy.toml

@@ -6,6 +6,7 @@ disallowed-methods = [
     { path = "web_sys::HtmlCanvasElement::set_height", reason = "Winit shouldn't touch the internal canvas size" },
     { path = "web_sys::Window::document", reason = "cache this to reduce calls to JS" },
     { path = "web_sys::Window::get_computed_style", reason = "cache this to reduce calls to JS" },
+    { path = "web_sys::HtmlElement::style", reason = "cache this to reduce calls to JS" },
     { path = "web_sys::Element::request_fullscreen", reason = "Doesn't account for compatibility with Safari" },
     { path = "web_sys::Document::exit_fullscreen", reason = "Doesn't account for compatibility with Safari" },
     { path = "web_sys::Document::fullscreen_element", reason = "Doesn't account for compatibility with Safari" },

src/platform_impl/web/web_sys/canvas.rs

@@ -49,12 +49,18 @@ pub struct Common {
     pub document: Document,
     /// Note: resizing the HTMLCanvasElement should go through `backend::set_canvas_size` to ensure the DPI factor is maintained.
     pub raw: HtmlCanvasElement,
-    style: CssStyleDeclaration,
+    style: Style,
     old_size: Rc<Cell<PhysicalSize<u32>>>,
     current_size: Rc<Cell<PhysicalSize<u32>>>,
     fullscreen_handler: Rc<FullscreenHandler>,
 }
 
+#[derive(Clone)]
+pub struct Style {
+    read: CssStyleDeclaration,
+    write: CssStyleDeclaration,
+}
+
 impl Canvas {
     pub fn create(
         id: WindowId,
@@ -90,12 +96,7 @@ impl Canvas {
                 .map_err(|_| os_error!(OsError("Failed to set a tabindex".to_owned())))?;
         }
 
-        #[allow(clippy::disallowed_methods)]
+        let style = Style::new(&window, &canvas);
-        let style = window
-            .get_computed_style(&canvas)
-            .expect("Failed to obtain computed style")
-            // this can't fail: we aren't using a pseudo-element
-            .expect("Invalid pseudo-element");
 
         let common = Common {
             window: window.clone(),
@@ -178,9 +179,7 @@ impl Canvas {
             y: bounds.y(),
         };
 
-        if self.document().contains(Some(self.raw()))
+        if self.document().contains(Some(self.raw())) && self.style().get("display") != "none" {
-            && self.style().get_property_value("display").unwrap() != "none"
-        {
             position.x += super::style_size_property(self.style(), "border-left-width")
                 + super::style_size_property(self.style(), "padding-left");
             position.y += super::style_size_property(self.style(), "border-top-width")
@@ -226,7 +225,7 @@ impl Canvas {
     }
 
     #[inline]
-    pub fn style(&self) -> &CssStyleDeclaration {
+    pub fn style(&self) -> &Style {
         &self.common.style
     }
 
@@ -563,3 +562,37 @@ impl Common {
         })
     }
 }
+
+impl Style {
+    fn new(window: &web_sys::Window, canvas: &HtmlCanvasElement) -> Self {
+        #[allow(clippy::disallowed_methods)]
+        let read = window
+            .get_computed_style(canvas)
+            .expect("Failed to obtain computed style")
+            // this can't fail: we aren't using a pseudo-element
+            .expect("Invalid pseudo-element");
+
+        #[allow(clippy::disallowed_methods)]
+        let write = canvas.style();
+
+        Self { read, write }
+    }
+
+    pub(crate) fn get(&self, property: &str) -> String {
+        self.read
+            .get_property_value(property)
+            .expect("Invalid property")
+    }
+
+    pub(crate) fn remove(&self, property: &str) {
+        self.write
+            .remove_property(property)
+            .expect("Property is read only");
+    }
+
+    pub(crate) fn set(&self, property: &str, value: &str) {
+        self.write
+            .set_property(property, value)
+            .expect("Property is read only");
+    }
+}

src/platform_impl/web/web_sys/mod.rs

@@ -10,6 +10,7 @@ mod resize_scaling;
 mod schedule;
 
 pub use self::canvas::Canvas;
+use self::canvas::Style;
 pub use self::event::ButtonsState;
 pub use self::event_handle::EventListenerHandle;
 pub use self::resize_scaling::ResizeScaleHandle;
@@ -17,9 +18,7 @@ pub use self::schedule::Schedule;
 
 use crate::dpi::{LogicalPosition, LogicalSize};
 use wasm_bindgen::closure::Closure;
-use web_sys::{
+use web_sys::{Document, HtmlCanvasElement, PageTransitionEvent, VisibilityState};
-    CssStyleDeclaration, Document, HtmlCanvasElement, PageTransitionEvent, VisibilityState,
-};
 
 pub fn throw(msg: &str) {
     wasm_bindgen::throw_str(msg);
@@ -51,8 +50,8 @@ pub fn scale_factor(window: &web_sys::Window) -> f64 {
     window.device_pixel_ratio()
 }
 
-fn fix_canvas_size(style: &CssStyleDeclaration, mut size: LogicalSize<f64>) -> LogicalSize<f64> {
+fn fix_canvas_size(style: &Style, mut size: LogicalSize<f64>) -> LogicalSize<f64> {
-    if style.get_property_value("box-sizing").unwrap() == "border-box" {
+    if style.get("box-sizing") == "border-box" {
         size.width += style_size_property(style, "border-left-width")
             + style_size_property(style, "border-right-width")
             + style_size_property(style, "padding-left")
@@ -69,76 +68,68 @@ fn fix_canvas_size(style: &CssStyleDeclaration, mut size: LogicalSize<f64>) -> L
 pub fn set_canvas_size(
     document: &Document,
     raw: &HtmlCanvasElement,
-    style: &CssStyleDeclaration,
+    style: &Style,
     new_size: LogicalSize<f64>,
 ) {
-    if !document.contains(Some(raw)) || style.get_property_value("display").unwrap() == "none" {
+    if !document.contains(Some(raw)) || style.get("display") == "none" {
         return;
     }
 
     let new_size = fix_canvas_size(style, new_size);
 
-    set_canvas_style_property(raw, "width", &format!("{}px", new_size.width));
+    style.set("width", &format!("{}px", new_size.width));
-    set_canvas_style_property(raw, "height", &format!("{}px", new_size.height));
+    style.set("height", &format!("{}px", new_size.height));
 }
 
 pub fn set_canvas_min_size(
     document: &Document,
     raw: &HtmlCanvasElement,
-    style: &CssStyleDeclaration,
+    style: &Style,
     dimensions: Option<LogicalSize<f64>>,
 ) {
     if let Some(dimensions) = dimensions {
-        if !document.contains(Some(raw)) || style.get_property_value("display").unwrap() == "none" {
+        if !document.contains(Some(raw)) || style.get("display") == "none" {
             return;
         }
 
         let new_size = fix_canvas_size(style, dimensions);
 
-        set_canvas_style_property(raw, "min-width", &format!("{}px", new_size.width));
+        style.set("min-width", &format!("{}px", new_size.width));
-        set_canvas_style_property(raw, "min-height", &format!("{}px", new_size.height));
+        style.set("min-height", &format!("{}px", new_size.height));
     } else {
-        style
+        style.remove("min-width");
-            .remove_property("min-width")
+        style.remove("min-height");
-            .expect("Property is read only");
-        style
-            .remove_property("min-height")
-            .expect("Property is read only");
     }
 }
 
 pub fn set_canvas_max_size(
     document: &Document,
     raw: &HtmlCanvasElement,
-    style: &CssStyleDeclaration,
+    style: &Style,
     dimensions: Option<LogicalSize<f64>>,
 ) {
     if let Some(dimensions) = dimensions {
-        if !document.contains(Some(raw)) || style.get_property_value("display").unwrap() == "none" {
+        if !document.contains(Some(raw)) || style.get("display") == "none" {
             return;
         }
 
         let new_size = fix_canvas_size(style, dimensions);
 
-        set_canvas_style_property(raw, "max-width", &format!("{}px", new_size.width));
+        style.set("max-width", &format!("{}px", new_size.width));
-        set_canvas_style_property(raw, "max-height", &format!("{}px", new_size.height));
+        style.set("max-height", &format!("{}px", new_size.height));
     } else {
-        style
+        style.remove("max-width");
-            .remove_property("max-width")
+        style.remove("max-height");
-            .expect("Property is read only");
-        style
-            .remove_property("max-height")
-            .expect("Property is read only");
     }
 }
 
 pub fn set_canvas_position(
     document: &Document,
     raw: &HtmlCanvasElement,
-    style: &CssStyleDeclaration,
+    style: &Style,
     mut position: LogicalPosition<f64>,
 ) {
-    if document.contains(Some(raw)) && style.get_property_value("display").unwrap() != "none" {
+    if document.contains(Some(raw)) && style.get("display") != "none" {
         position.x -= style_size_property(style, "margin-left")
             + style_size_property(style, "border-left-width")
             + style_size_property(style, "padding-left");
@@ -147,30 +138,21 @@ pub fn set_canvas_position(
             + style_size_property(style, "padding-top");
     }
 
-    set_canvas_style_property(raw, "position", "fixed");
+    style.set("position", "fixed");
-    set_canvas_style_property(raw, "left", &format!("{}px", position.x));
+    style.set("left", &format!("{}px", position.x));
-    set_canvas_style_property(raw, "top", &format!("{}px", position.y));
+    style.set("top", &format!("{}px", position.y));
 }
 
 /// This function will panic if the element is not inserted in the DOM
 /// or is not a CSS property that represents a size in pixel.
-pub fn style_size_property(style: &CssStyleDeclaration, property: &str) -> f64 {
+pub fn style_size_property(style: &Style, property: &str) -> f64 {
-    let prop = style
+    let prop = style.get(property);
-        .get_property_value(property)
-        .expect("Found invalid property");
     prop.strip_suffix("px")
         .expect("Element was not inserted into the DOM or is not a size in pixel")
         .parse()
         .expect("CSS property is not a size in pixel")
 }
 
-pub fn set_canvas_style_property(raw: &HtmlCanvasElement, property: &str, value: &str) {
-    let style = raw.style();
-    style
-        .set_property(property, value)
-        .unwrap_or_else(|err| panic!("error: {err:?}\nFailed to set {property}"))
-}
-
 pub fn is_dark_mode(window: &web_sys::Window) -> Option<bool> {
     window
         .match_media("(prefers-color-scheme: dark)")

src/platform_impl/web/web_sys/resize_scaling.rs

@@ -2,14 +2,14 @@ use js_sys::{Array, Object};
 use wasm_bindgen::prelude::{wasm_bindgen, Closure};
 use wasm_bindgen::{JsCast, JsValue};
 use web_sys::{
-    CssStyleDeclaration, Document, HtmlCanvasElement, MediaQueryList, ResizeObserver,
+    Document, HtmlCanvasElement, MediaQueryList, ResizeObserver, ResizeObserverBoxOptions,
-    ResizeObserverBoxOptions, ResizeObserverEntry, ResizeObserverOptions, ResizeObserverSize,
+    ResizeObserverEntry, ResizeObserverOptions, ResizeObserverSize, Window,
-    Window,
 };
 
 use crate::dpi::{LogicalSize, PhysicalSize};
 
 use super::super::backend;
+use super::canvas::Style;
 use super::media_query_handle::MediaQueryListHandle;
 
 use std::cell::{Cell, RefCell};
@@ -22,7 +22,7 @@ impl ResizeScaleHandle {
         window: Window,
         document: Document,
         canvas: HtmlCanvasElement,
-        style: CssStyleDeclaration,
+        style: Style,
         scale_handler: S,
         resize_handler: R,
     ) -> Self
@@ -51,7 +51,7 @@ struct ResizeScaleInternal {
     window: Window,
     document: Document,
     canvas: HtmlCanvasElement,
-    style: CssStyleDeclaration,
+    style: Style,
     mql: MediaQueryListHandle,
     observer: ResizeObserver,
     _observer_closure: Closure<dyn FnMut(Array, ResizeObserver)>,
@@ -65,7 +65,7 @@ impl ResizeScaleInternal {
         window: Window,
         document: Document,
         canvas: HtmlCanvasElement,
-        style: CssStyleDeclaration,
+        style: Style,
         scale_handler: S,
         resize_handler: R,
     ) -> Rc<RefCell<Self>>
@@ -152,9 +152,7 @@ impl ResizeScaleInternal {
     }
 
     fn notify(&mut self) {
-        if !self.document.contains(Some(&self.canvas))
+        if !self.document.contains(Some(&self.canvas)) || self.style.get("display") == "none" {
-            || self.style.get_property_value("display").unwrap() == "none"
-        {
             let size = PhysicalSize::new(0, 0);
 
             if self.notify_scale.replace(false) {
@@ -180,7 +178,7 @@ impl ResizeScaleInternal {
             backend::style_size_property(&self.style, "height"),
         );
 
-        if self.style.get_property_value("box-sizing").unwrap() == "border-box" {
+        if self.style.get("box-sizing") == "border-box" {
             size.width -= backend::style_size_property(&self.style, "border-left-width")
                 + backend::style_size_property(&self.style, "border-right-width")
                 + backend::style_size_property(&self.style, "padding-left")
@@ -246,10 +244,7 @@ impl ResizeScaleInternal {
             .get(0)
             .unchecked_into();
 
-        let writing_mode = self
+        let writing_mode = self.style.get("writing-mode");
-            .style
-            .get_property_value("writing-mode")
-            .expect("`writing-mode` is a valid CSS property");
 
         // means the canvas is not inserted into the DOM
         if writing_mode.is_empty() {

src/platform_impl/web/window.rs

@@ -196,7 +196,7 @@ impl Inner {
     #[inline]
     pub fn set_cursor_icon(&self, cursor: CursorIcon) {
         *self.previous_pointer.borrow_mut() = cursor.name();
-        backend::set_canvas_style_property(self.canvas.borrow().raw(), "cursor", cursor.name());
+        self.canvas.borrow().style().set("cursor", cursor.name());
     }
 
     #[inline]
@@ -223,13 +223,12 @@ impl Inner {
     #[inline]
     pub fn set_cursor_visible(&self, visible: bool) {
         if !visible {
-            backend::set_canvas_style_property(self.canvas.borrow().raw(), "cursor", "none");
+            self.canvas.borrow().style().set("cursor", "none");
         } else {
-            backend::set_canvas_style_property(
+            self.canvas
-                self.canvas.borrow().raw(),
+                .borrow()
-                "cursor",
+                .style()
-                &self.previous_pointer.borrow(),
+                .set("cursor", &self.previous_pointer.borrow());
-            );
         }
     }