Introduce <LayoutDom<HTMLImageElement>>::current_request

This safe helper contains the only source of unsafety from the actual image layout helpers methods, making them completely safe.
servo · Apr 1, 2020 · 1cd3d6b · 1cd3d6b
1 parent fc07a51
commit 1cd3d6b
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 43 deletions.
diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs
@@ -1366,53 +1366,40 @@ impl MicrotaskRunnable for ImageElementMicrotask {
 }
 
 pub trait LayoutHTMLImageElementHelpers {
-    #[allow(unsafe_code)]
-    unsafe fn image(self) -> Option<Arc<Image>>;
-    #[allow(unsafe_code)]
-    unsafe fn image_url(self) -> Option<ServoUrl>;
-    #[allow(unsafe_code)]
-    unsafe fn image_density(self) -> Option<f64>;
-    #[allow(unsafe_code)]
-    unsafe fn image_data(self) -> (Option<Arc<Image>>, Option<ImageMetadata>);
+    fn image(self) -> Option<Arc<Image>>;
+    fn image_url(self) -> Option<ServoUrl>;
+    fn image_density(self) -> Option<f64>;
+    fn image_data(self) -> (Option<Arc<Image>>, Option<ImageMetadata>);
     fn get_width(self) -> LengthOrPercentageOrAuto;
     fn get_height(self) -> LengthOrPercentageOrAuto;
 }
 
-impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> {
+impl<'dom> LayoutDom<'dom, HTMLImageElement> {
     #[allow(unsafe_code)]
-    unsafe fn image(self) -> Option<Arc<Image>> {
-        (*self.unsafe_get())
-            .current_request
-            .borrow_for_layout()
-            .image
-            .clone()
+    fn current_request(self) -> &'dom ImageRequest {
+        unsafe { self.unsafe_get().current_request.borrow_for_layout() }
+    }
+}
+
+impl LayoutHTMLImageElementHelpers for LayoutDom<'_, HTMLImageElement> {
+    fn image(self) -> Option<Arc<Image>> {
+        self.current_request().image.clone()
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn image_url(self) -> Option<ServoUrl> {
-        (*self.unsafe_get())
-            .current_request
-            .borrow_for_layout()
-            .parsed_url
-            .clone()
+    fn image_url(self) -> Option<ServoUrl> {
+        self.current_request().parsed_url.clone()
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn image_data(self) -> (Option<Arc<Image>>, Option<ImageMetadata>) {
-        let current_request = (*self.unsafe_get()).current_request.borrow_for_layout();
+    fn image_data(self) -> (Option<Arc<Image>>, Option<ImageMetadata>) {
+        let current_request = self.current_request();
         (
             current_request.image.clone(),
             current_request.metadata.clone(),
         )
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn image_density(self) -> Option<f64> {
-        (*self.unsafe_get())
-            .current_request
-            .borrow_for_layout()
-            .current_pixel_density
-            .clone()
+    fn image_density(self) -> Option<f64> {
+        self.current_request().current_pixel_density.clone()
     }
 
     fn get_width(self) -> LengthOrPercentageOrAuto {

diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
@@ -1493,25 +1493,21 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
 
     #[allow(unsafe_code)]
     fn image_url(self) -> Option<ServoUrl> {
-        unsafe {
-            self.downcast::<HTMLImageElement>()
-                .expect("not an image!")
-                .image_url()
-        }
+        self.downcast::<HTMLImageElement>()
+            .expect("not an image!")
+            .image_url()
     }
 
     #[allow(unsafe_code)]
     fn image_data(self) -> Option<(Option<StdArc<Image>>, Option<ImageMetadata>)> {
-        unsafe { self.downcast::<HTMLImageElement>().map(|e| e.image_data()) }
+        self.downcast::<HTMLImageElement>().map(|e| e.image_data())
     }
 
     #[allow(unsafe_code)]
     fn image_density(self) -> Option<f64> {
-        unsafe {
-            self.downcast::<HTMLImageElement>()
-                .expect("not an image!")
-                .image_density()
-        }
+        self.downcast::<HTMLImageElement>()
+            .expect("not an image!")
+            .image_density()
     }
 
     fn canvas_data(self) -> Option<HTMLCanvasData> {