Introduce more layout helpers to make selection_for_layout be safe

servo · Apr 1, 2020 · d9e4f7a · d9e4f7a
1 parent 28e5abe
commit d9e4f7a
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 18 deletions.
diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs
@@ -706,8 +706,7 @@ impl HTMLInputElement {
 pub trait LayoutHTMLInputElementHelpers<'dom> {
     fn value_for_layout(self) -> Cow<'dom, str>;
     fn size_for_layout(self) -> u32;
-    #[allow(unsafe_code)]
-    unsafe fn selection_for_layout(self) -> Option<Range<usize>>;
+    fn selection_for_layout(self) -> Option<Range<usize>>;
     fn checked_state_for_layout(self) -> bool;
     fn indeterminate_state_for_layout(self) -> bool;
 }
@@ -730,6 +729,15 @@ impl<'dom> LayoutDom<'dom, HTMLInputElement> {
     fn input_type(self) -> InputType {
         unsafe { self.unsafe_get().input_type.get() }
     }
+
+    fn textinput_sorted_selection_offsets_range(self) -> Range<UTF8Bytes> {
+        unsafe {
+            self.unsafe_get()
+                .textinput
+                .borrow_for_layout()
+                .sorted_selection_offsets_range()
+        }
+    }
 }
 
 impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElement> {
@@ -778,18 +786,17 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem
         unsafe { self.unsafe_get().size.get() }
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn selection_for_layout(self) -> Option<Range<usize>> {
+    fn selection_for_layout(self) -> Option<Range<usize>> {
         if !self.upcast::<Element>().focus_state() {
             return None;
         }
 
-        let textinput = (*self.unsafe_get()).textinput.borrow_for_layout();
+        let sorted_selection_offsets_range = self.textinput_sorted_selection_offsets_range();
 
         match self.input_type() {
             InputType::Password => {
                 let text = self.get_raw_textinput_value();
-                let sel = UTF8Bytes::unwrap_range(textinput.sorted_selection_offsets_range());
+                let sel = UTF8Bytes::unwrap_range(sorted_selection_offsets_range);
 
                 // Translate indices from the raw value to indices in the replacement value.
                 let char_start = text[..sel.start].chars().count();
@@ -798,9 +805,9 @@ impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElem
                 let bytes_per_char = PASSWORD_REPLACEMENT_CHAR.len_utf8();
                 Some(char_start * bytes_per_char..char_end * bytes_per_char)
             },
-            input_type if input_type.is_textual() => Some(UTF8Bytes::unwrap_range(
-                textinput.sorted_selection_offsets_range(),
-            )),
+            input_type if input_type.is_textual() => {
+                Some(UTF8Bytes::unwrap_range(sorted_selection_offsets_range))
+            },
             _ => None,
         }
     }

diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs
@@ -57,8 +57,7 @@ pub struct HTMLTextAreaElement {
 
 pub trait LayoutHTMLTextAreaElementHelpers {
     fn value_for_layout(self) -> String;
-    #[allow(unsafe_code)]
-    unsafe fn selection_for_layout(self) -> Option<Range<usize>>;
+    fn selection_for_layout(self) -> Option<Range<usize>>;
     fn get_cols(self) -> u32;
     fn get_rows(self) -> u32;
 }
@@ -74,6 +73,15 @@ impl<'dom> LayoutDom<'dom, HTMLTextAreaElement> {
         }
     }
 
+    fn textinput_sorted_selection_offsets_range(self) -> Range<UTF8Bytes> {
+        unsafe {
+            self.unsafe_get()
+                .textinput
+                .borrow_for_layout()
+                .sorted_selection_offsets_range()
+        }
+    }
+
     fn placeholder(self) -> &'dom str {
         unsafe { self.unsafe_get().placeholder.borrow_for_layout() }
     }
@@ -94,14 +102,12 @@ impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> {
         }
     }
 
-    #[allow(unsafe_code)]
-    unsafe fn selection_for_layout(self) -> Option<Range<usize>> {
+    fn selection_for_layout(self) -> Option<Range<usize>> {
         if !self.upcast::<Element>().focus_state() {
             return None;
         }
-        let textinput = (*self.unsafe_get()).textinput.borrow_for_layout();
         Some(UTF8Bytes::unwrap_range(
-            textinput.sorted_selection_offsets_range(),
+            self.textinput_sorted_selection_offsets_range(),
         ))
     }
 

diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs
@@ -1478,14 +1478,13 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> {
         panic!("not text!")
     }
 
-    #[allow(unsafe_code)]
     fn selection(self) -> Option<Range<usize>> {
         if let Some(area) = self.downcast::<HTMLTextAreaElement>() {
-            return unsafe { area.selection_for_layout() };
+            return area.selection_for_layout();
         }
 
         if let Some(input) = self.downcast::<HTMLInputElement>() {
-            return unsafe { input.selection_for_layout() };
+            return input.selection_for_layout();
         }
 
         None