Bug 1497406 - Use helper function to set length and copy into nsTArrays of PODs from Rust r=boris
authorCameron McCormack <cam@mcc.id.au>
Tue, 09 Oct 2018 08:49:51 +0000
changeset 489546 18a3482ffcbac7c1b7c1b4f16246665baeab2971
parent 489545 2f412849ad38683187cff35d9dcfa57951eeadb4
child 489547 d86e80f3d1771818f831b56713f760648652aa66
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersboris
bugs1497406
milestone64.0a1
Bug 1497406 - Use helper function to set length and copy into nsTArrays of PODs from Rust r=boris Differential Revision: https://phabricator.services.mozilla.com/D8058
servo/components/style/gecko_bindings/sugar/ns_t_array.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/stylesheets/font_feature_values_rule.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/gecko_bindings/sugar/ns_t_array.rs
+++ b/servo/components/style/gecko_bindings/sugar/ns_t_array.rs
@@ -77,32 +77,47 @@ impl<T> nsTArray<T> {
     where
         T: Copy,
     {
         unsafe { self.clear() }
     }
 
     /// Resize and set the length of the array to `len`.
     ///
-    /// unsafe because the array may contain uninitialized members.
+    /// unsafe because this may leave the array with uninitialized elements.
     ///
-    /// This will not call constructors, if you need that, either manually add
+    /// This will not call constructors.  If you need that, either manually add
     /// bindings or run the typed `EnsureCapacity` call on the gecko side.
     pub unsafe fn set_len(&mut self, len: u32) {
         // this can leak
         debug_assert!(len >= self.len() as u32);
         self.ensure_capacity(len as usize);
         let header = self.header_mut();
         header.mLength = len;
     }
 
     /// Resizes an array containing only POD elements
     ///
+    /// unsafe because this may leave the array with uninitialized elements.
+    ///
     /// This will not leak since it only works on POD types (and thus doesn't assert)
     pub unsafe fn set_len_pod(&mut self, len: u32)
     where
         T: Copy,
     {
         self.ensure_capacity(len as usize);
         let header = self.header_mut();
         header.mLength = len;
     }
+
+    /// Collects the given iterator into this array.
+    ///
+    /// Not unsafe because we won't leave uninitialized elements in the array.
+    pub fn assign_from_iter_pod<I>(&mut self, iter: I)
+    where
+        T: Copy,
+        I: ExactSizeIterator + Iterator<Item = T>,
+    {
+        debug_assert!(iter.len() <= 0xFFFFFFFF);
+        unsafe { self.set_len_pod(iter.len() as u32); }
+        self.iter_mut().zip(iter).for_each(|(r, v)| *r = v);
+    }
 }
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -1368,45 +1368,32 @@ impl Clone for ${style_struct.gecko_stru
     <% impl_simple_copy(ident, gecko_ffi_name) %>
 
     #[allow(non_snake_case)]
     pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
         From::from(self.gecko.${gecko_ffi_name})
     }
 </%def>
 
-<%def name="impl_font_settings(ident, tag_type, value_type, gecko_value_type)">
+<%def name="impl_font_settings(ident, gecko_type, tag_type, value_type, gecko_value_type)">
     <%
     gecko_ffi_name = to_camel_case_lower(ident)
     %>
 
     pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
-        let current_settings = &mut self.gecko.mFont.${gecko_ffi_name};
-        current_settings.clear_pod();
-
-        unsafe { current_settings.set_len_pod(v.0.len() as u32) };
-
-        for (current, other) in current_settings.iter_mut().zip(v.0.iter()) {
-            current.mTag = other.tag.0;
-            current.mValue = other.value as ${gecko_value_type};
-        }
+        let iter = v.0.iter().map(|other| structs::${gecko_type} {
+            mTag: other.tag.0,
+            mValue: other.value as ${gecko_value_type},
+        });
+        self.gecko.mFont.${gecko_ffi_name}.assign_from_iter_pod(iter);
     }
 
     pub fn copy_${ident}_from(&mut self, other: &Self) {
-        let current_settings = &mut self.gecko.mFont.${gecko_ffi_name};
-        let other_settings = &other.gecko.mFont.${gecko_ffi_name};
-        let settings_length = other_settings.len() as u32;
-
-        current_settings.clear_pod();
-        unsafe { current_settings.set_len_pod(settings_length) };
-
-        for (current, other) in current_settings.iter_mut().zip(other_settings.iter()) {
-            current.mTag = other.mTag;
-            current.mValue = other.mValue;
-        }
+        let iter = other.gecko.mFont.${gecko_ffi_name}.iter().map(|s| *s);
+        self.gecko.mFont.${gecko_ffi_name}.assign_from_iter_pod(iter);
     }
 
     pub fn reset_${ident}(&mut self, other: &Self) {
         self.copy_${ident}_from(other)
     }
 
     pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
         use values::generics::font::{FontSettings, FontTag, ${tag_type}};
@@ -2269,18 +2256,18 @@ fn static_assert() {
                              font-feature-settings font-variation-settings
                              -moz-min-font-size-ratio -x-text-zoom"""
 %>
 <%self:impl_trait style_struct_name="Font"
     skip_longhands="${skip_font_longhands}">
 
     // Negative numbers are invalid at parse time, but <integer> is still an
     // i32.
-    <% impl_font_settings("font_feature_settings", "FeatureTagValue", "i32", "u32") %>
-    <% impl_font_settings("font_variation_settings", "VariationValue", "f32", "f32") %>
+    <% impl_font_settings("font_feature_settings", "gfxFontFeature", "FeatureTagValue", "i32", "u32") %>
+    <% impl_font_settings("font_variation_settings", "gfxFontVariation", "VariationValue", "f32", "f32") %>
 
     pub fn fixup_none_generic(&mut self, device: &Device) {
         self.gecko.mFont.systemFont = false;
         unsafe {
             bindings::Gecko_nsStyleFont_FixupNoneGeneric(&mut self.gecko, device.pres_context())
         }
     }
 
@@ -3208,38 +3195,26 @@ fn static_assert() {
 
     ${impl_style_coord("scroll_snap_points_x", "mScrollSnapPointsX")}
     ${impl_style_coord("scroll_snap_points_y", "mScrollSnapPointsY")}
 
     pub fn set_scroll_snap_coordinate<I>(&mut self, v: I)
         where I: IntoIterator<Item = longhands::scroll_snap_coordinate::computed_value::single_value::T>,
               I::IntoIter: ExactSizeIterator
     {
-        let v = v.into_iter();
-
-        unsafe { self.gecko.mScrollSnapCoordinate.set_len_pod(v.len() as u32); }
-        for (gecko, servo) in self.gecko.mScrollSnapCoordinate
-                               .iter_mut()
-                               .zip(v) {
-            gecko.mXPosition = servo.horizontal.into();
-            gecko.mYPosition = servo.vertical.into();
-        }
+        let iter = v.into_iter().map(|c| structs::mozilla::Position {
+            mXPosition: c.horizontal.into(),
+            mYPosition: c.vertical.into(),
+        });
+        self.gecko.mScrollSnapCoordinate.assign_from_iter_pod(iter);
     }
 
     pub fn copy_scroll_snap_coordinate_from(&mut self, other: &Self) {
-        unsafe {
-            self.gecko.mScrollSnapCoordinate
-                .set_len_pod(other.gecko.mScrollSnapCoordinate.len() as u32);
-        }
-
-        for (this, that) in self.gecko.mScrollSnapCoordinate
-                               .iter_mut()
-                               .zip(other.gecko.mScrollSnapCoordinate.iter()) {
-            *this = *that;
-        }
+        let iter = other.gecko.mScrollSnapCoordinate.iter().map(|c| *c);
+        self.gecko.mScrollSnapCoordinate.assign_from_iter_pod(iter);
     }
 
     pub fn reset_scroll_snap_coordinate(&mut self, other: &Self) {
         self.copy_scroll_snap_coordinate_from(other)
     }
 
     pub fn clone_scroll_snap_coordinate(&self) -> longhands::scroll_snap_coordinate::computed_value::T {
         let vec = self.gecko.mScrollSnapCoordinate.iter().map(|f| f.into()).collect();
@@ -4990,22 +4965,22 @@ fn set_style_svg_path(
     // Setup type.
     shape_source.mType = StyleShapeSourceType::Path;
 
     // Setup path.
     let gecko_path = unsafe {
         Gecko_NewStyleSVGPath(shape_source);
         &mut shape_source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap()
     };
-    unsafe { gecko_path.mPath.set_len(servo_path.commands().len() as u32) };
-    debug_assert_eq!(gecko_path.mPath.len(), servo_path.commands().len());
-    for (servo, gecko) in servo_path.commands().iter().zip(gecko_path.mPath.iter_mut()) {
+
+    let iter = servo_path.commands().iter().map(|command| {
         // unsafe: cbindgen ensures the representation is the same.
-        *gecko = unsafe { transmute(*servo) };
-    }
+        unsafe { transmute(*command) }
+    });
+    gecko_path.mPath.assign_from_iter_pod(iter);
 
     // Setup fill-rule.
     // unsafe: cbindgen ensures the representation is the same.
     gecko_path.mFillRule = unsafe { transmute(fill) };
 }
 
 <%def name="impl_shape_source(ident, gecko_ffi_name)">
     pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
--- a/servo/components/style/stylesheets/font_feature_values_rule.rs
+++ b/servo/components/style/stylesheets/font_feature_values_rule.rs
@@ -174,22 +174,17 @@ impl Parse for VectorValues {
 
         Ok(VectorValues(vec))
     }
 }
 
 #[cfg(feature = "gecko")]
 impl ToGeckoFontFeatureValues for VectorValues {
     fn to_gecko_font_feature_values(&self, array: &mut nsTArray<u32>) {
-        unsafe {
-            array.set_len_pod(self.0.len() as u32);
-        }
-        for (dest, value) in array.iter_mut().zip(self.0.iter()) {
-            *dest = *value;
-        }
+        array.assign_from_iter_pod(self.0.iter().map(|v| *v));
     }
 }
 
 /// Parses a list of `FamilyName`s.
 pub fn parse_family_name_list<'i, 't>(
     context: &ParserContext,
     input: &mut Parser<'i, 't>,
 ) -> Result<Vec<FamilyName>, ParseError<'i>> {
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1684,21 +1684,19 @@ fn write_locked_arc<T, R, F>(raw: &<Lock
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_CssRules_ListTypes(
     rules: ServoCssRulesBorrowed,
     result: nsTArrayBorrowed_uintptr_t,
 ) {
     read_locked_arc(rules, |rules: &CssRules| {
-        let iter = rules.0.iter().map(|rule| rule.rule_type() as usize);
-        let (size, upper) = iter.size_hint();
-        debug_assert_eq!(size, upper.unwrap());
-        unsafe { result.set_len(size as u32) };
-        result.iter_mut().zip(iter).fold((), |_, (r, v)| *r = v);
+        result.assign_from_iter_pod(
+            rules.0.iter().map(|rule| rule.rule_type() as usize)
+        );
     })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_CssRules_InsertRule(
     rules: ServoCssRulesBorrowed,
     contents: RawServoStyleSheetContentsBorrowed,
     rule: *const nsACString,
@@ -3335,24 +3333,23 @@ pub extern "C" fn Servo_ComputedValues_G
         // rules.
         if node.importance().important() {
             continue;
         }
 
         result.push(style_rule);
     }
 
-    unsafe { rules.set_len(result.len() as u32) };
-    for (ref src, ref mut dest) in result.into_iter().zip(rules.iter_mut()) {
+    rules.assign_from_iter_pod(result.into_iter().map(|src| {
         src.with_arc(|a| {
             a.with_raw_offset_arc(|arc| {
-                **dest = *Locked::<StyleRule>::arc_as_borrowed(arc);
+                *Locked::<StyleRule>::arc_as_borrowed(arc) as *const _
             })
-        });
-    }
+        })
+    }))
 }
 
 /// See the comment in `Device` to see why it's ok to pass an owned reference to
 /// the pres context (hint: the context outlives the StyleSet, that holds the
 /// device alive).
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_Init(
     pres_context: RawGeckoPresContextBorrowed,