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 499661 18a3482ffcbac7c1b7c1b4f16246665baeab2971
parent 499660 2f412849ad38683187cff35d9dcfa57951eeadb4
child 499662 d86e80f3d1771818f831b56713f760648652aa66
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersboris
bugs1497406
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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,