Address review comments

components/style/properties/helpers.mako.rs

@@ -33,6 +33,8 @@
     </%call>
 </%def>
 
+// FIXME (Manishearth): Add computed_value_as_specified argument
+// and handle the empty case correctly
 <%doc>
     To be used in cases where we have a grammar like
     "<thing> [ , <thing> ]*", but only support a single value
@@ -54,13 +56,14 @@
             }
             pub mod computed_value {
                 use super::single_value;
-                #[derive(Debug, Clone, PartialEq, HeapSizeOf)]
+                #[derive(Debug, Clone, PartialEq)]
+                #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
                 pub struct T(pub Vec<single_value::computed_value::T>);
             }
 
             impl ToCss for computed_value::T {
                 fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                    if self.0.len() >= 1 {
+                    if !self.0.is_empty() {
                         try!(self.0[0].to_css(dest));
                     }
                     for i in self.0.iter().skip(1) {
@@ -71,12 +74,13 @@
                 }
             }
 
-            #[derive(Debug, Clone, PartialEq, HeapSizeOf)]
+            #[derive(Debug, Clone, PartialEq)]
+            #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
             pub struct SpecifiedValue(pub Vec<single_value::SpecifiedValue>);
 
             impl ToCss for SpecifiedValue {
                 fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                    if self.0.len() >= 1 {
+                    if !self.0.is_empty() {
                         try!(self.0[0].to_css(dest))
                     }
                     for i in self.0.iter().skip(1) {

ports/geckolib/gecko_bindings/bindings.rs

@@ -1,6 +1,10 @@
 /* automatically generated by rust-bindgen */
 
 use heapsize::HeapSizeOf;
+pub enum nsINode {}
+pub enum nsIDocument {}
+pub enum nsIPrincipal {}
+pub enum nsIURI {}
 use structs::nsStyleFont;
 unsafe impl Send for nsStyleFont {}
 unsafe impl Sync for nsStyleFont {}
@@ -117,6 +121,8 @@ use structs::nsStyleImageLayers;
 unsafe impl Send for nsStyleImageLayers {}
 unsafe impl Sync for nsStyleImageLayers {}
 impl HeapSizeOf for nsStyleImageLayers { fn heap_size_of_children(&self) -> usize { 0 } }
+use structs::nsStyleImageLayers_Layer as Layer;
+use structs::nsStyleImageLayers_LayerType as LayerType;
 use structs::SheetParsingMode;
 use structs::nsMainThreadPtrHandle;
 use structs::nsMainThreadPtrHolder;
@@ -126,13 +132,9 @@ use structs::FontFamilyList;
 use structs::FontFamilyType;
 use structs::nsIAtom;
 
-pub enum nsINode { }
 pub type RawGeckoNode = nsINode;
-pub enum nsIPrincipal { }
-pub enum nsIURI { }
 pub enum Element { }
 pub type RawGeckoElement = Element;
-pub enum nsIDocument { }
 pub type RawGeckoDocument = nsIDocument;
 pub enum ServoNodeData { }
 pub enum ServoComputedValues { }
@@ -247,10 +249,12 @@ extern "C" {
     pub fn Gecko_GetNodeFlags(node: *mut RawGeckoNode) -> u32;
     pub fn Gecko_SetNodeFlags(node: *mut RawGeckoNode, flags: u32);
     pub fn Gecko_UnsetNodeFlags(node: *mut RawGeckoNode, flags: u32);
-    pub fn Gecko_ArrayEnsureCapacity(array: *mut ::std::os::raw::c_void,
+    pub fn Gecko_EnsureTArrayCapacity(array: *mut ::std::os::raw::c_void,
-                                     capacity: usize, size: usize);
+                                      capacity: usize, elem_size: usize);
-    pub fn Gecko_ImageLayers_EnsureLength(layers: *mut nsStyleImageLayers,
+    pub fn Gecko_EnsureImageLayersLength(layers: *mut nsStyleImageLayers,
-                                          len: usize);
+                                         len: usize);
+    pub fn Gecko_InitializeImageLayer(layer: *mut Layer,
+                                      layer_type: LayerType);
     pub fn Servo_StylesheetFromUTF8Bytes(bytes: *const u8, length: u32,
                                          parsing_mode: SheetParsingMode,
                                          base: *mut ThreadSafeURIHolder,

ports/geckolib/gecko_bindings/sugar/ns_style_auto_array.rs

@@ -1,5 +1,9 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
 use std::iter::{once, Chain, Once, IntoIterator};
-use std::slice::{IterMut};
+use std::slice::IterMut;
 use structs::nsStyleAutoArray;
 
 impl<T> nsStyleAutoArray<T> {
@@ -7,6 +11,9 @@ impl<T> nsStyleAutoArray<T> {
         once(&mut self.mFirstElement).chain(self.mOtherElements.iter_mut())
     }
 
+    // Note that often structs containing autoarrays will have
+    // additional member fields that contain the length, which must be kept
+    // in sync
     pub fn len(&self) -> usize {
         1 + self.mOtherElements.len()
     }
@@ -18,4 +25,4 @@ impl<'a, T> IntoIterator for &'a mut nsStyleAutoArray<T> {
     fn into_iter(self) -> Self::IntoIter {
         self.iter_mut()
     }
-}
+}

ports/geckolib/gecko_bindings/sugar/ns_t_array.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use bindings::Gecko_ArrayEnsureCapacity;
+use bindings::Gecko_EnsureTArrayCapacity;
 use std::mem;
 use std::ops::{Deref, DerefMut};
 use std::os::raw::c_void;
@@ -35,9 +35,10 @@ impl<T> nsTArray<T> {
         debug_assert!(!self.mBuffer.is_null());
         unsafe { mem::transmute(self.mBuffer) }
     }
-    fn header_mut <'a>(&'a mut self) -> &'a mut nsTArrayHeader {
+    // unsafe, since header may be in shared static or something
+    unsafe fn header_mut<'a>(&'a mut self) -> &'a mut nsTArrayHeader {
         debug_assert!(!self.mBuffer.is_null());
-        unsafe { mem::transmute(self.mBuffer) }
+        mem::transmute(self.mBuffer)
     }
 
     #[inline]
@@ -48,12 +49,17 @@ impl<T> nsTArray<T> {
 
     fn ensure_capacity(&mut self, cap: usize) {
         unsafe {
-            Gecko_ArrayEnsureCapacity(self as *mut nsTArray<T> as *mut c_void, cap, mem::size_of::<T>())
+            Gecko_EnsureTArrayCapacity(self as *mut nsTArray<T> as *mut c_void, cap, mem::size_of::<T>())
         }
     }
 
     // unsafe because the array may contain uninits
+    // This will not call constructors, 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 mut header = self.header_mut();
         header.mLength = len;

ports/geckolib/gecko_bindings/tools/regen.py

@@ -119,11 +119,15 @@ COMPILATION_TARGETS = {
             "nsStyleOutline", "nsStyleXUL", "nsStyleSVGReset", "nsStyleColumn",
             "nsStyleEffects", "nsStyleImage", "nsStyleGradient",
             "nsStyleCoord", "nsStyleGradientStop", "nsStyleImageLayers",
+            "nsStyleImageLayers::Layer", "nsStyleImageLayers::LayerType",
 
             "SheetParsingMode", "nsMainThreadPtrHandle",
             "nsMainThreadPtrHolder", "nscolor", "nsFont", "FontFamilyList",
             "FontFamilyType", "nsIAtom",
         ],
+        "void_types": [
+            "nsINode", "nsIDocument", "nsIPrincipal", "nsIURI",
+        ]
     }
 }
 
@@ -226,23 +230,33 @@ def build(objdir, target_name, kind_name=None,
         for ty in current_target["opaque_types"]:
             flags.append("-opaque-type")
             flags.append(ty)
+    if "void_types" in current_target:
+        for ty in current_target["void_types"]:
+            flags.append("-raw-line")
+            flags.append("pub enum {} {{}}".format(ty))
 
     if "structs_types" in current_target:
         for ty in current_target["structs_types"]:
+            ty_fragments = ty.split("::")
+            mangled_name = ty.replace("::", "_")
             flags.append("-blacklist-type")
-            flags.append(ty)
+            flags.append(ty_fragments[-1])
             flags.append("-raw-line")
-            flags.append("use structs::{};".format(ty))
+            if len(ty_fragments) > 1:
+                flags.append("use structs::{} as {};".format(mangled_name, ty_fragments[-1]))
+            else:
+                flags.append("use structs::{};".format(mangled_name))
             # TODO: this is hacky, figure out a better way to do it without
             # hardcoding everything...
-            if ty.startswith("nsStyle"):
+            if ty_fragments[-1].startswith("nsStyle"):
                 flags.extend([
                     "-raw-line",
-                    "unsafe impl Send for {} {{}}".format(ty),
+                    "unsafe impl Send for {} {{}}".format(ty_fragments[-1]),
                     "-raw-line",
-                    "unsafe impl Sync for {} {{}}".format(ty),
+                    "unsafe impl Sync for {} {{}}".format(ty_fragments[-1]),
                     "-raw-line",
-                    "impl HeapSizeOf for {} {{ fn heap_size_of_children(&self) -> usize {{ 0 }} }}".format(ty)
+                    "impl HeapSizeOf for {} {{ fn heap_size_of_children(&self) -> usize {{ 0 }} }}"
+                    .format(ty_fragments[-1])
                 ])
 
     flags.append("-o")

ports/geckolib/properties.mako.rs

@@ -19,10 +19,10 @@ use gecko_bindings::bindings::Gecko_Destroy_${style_struct.gecko_ffi_name};
 use gecko_bindings::bindings::{Gecko_CopyMozBindingFrom, Gecko_CopyListStyleTypeFrom};
 use gecko_bindings::bindings::{Gecko_SetMozBinding, Gecko_SetListStyleType};
 use gecko_bindings::bindings::{Gecko_SetNullImageValue, Gecko_SetGradientImageValue};
-use gecko_bindings::bindings::{Gecko_ImageLayers_EnsureLength, Gecko_CreateGradient};
+use gecko_bindings::bindings::{Gecko_EnsureImageLayersLength, Gecko_CreateGradient};
 use gecko_bindings::bindings::{Gecko_CopyImageValueFrom, Gecko_CopyFontFamilyFrom};
 use gecko_bindings::bindings::{Gecko_FontFamilyList_AppendGeneric, Gecko_FontFamilyList_AppendNamed};
-use gecko_bindings::bindings::{Gecko_FontFamilyList_Clear};
+use gecko_bindings::bindings::{Gecko_FontFamilyList_Clear, Gecko_InitializeImageLayer};
 use gecko_bindings::structs;
 use glue::ArcHelpers;
 use std::fmt::{self, Debug};
@@ -947,6 +947,7 @@ fn static_assert() {
     }
 
     fn set_background_image(&mut self, images: longhands::background_image::computed_value::T) {
+        use gecko_bindings::structs::nsStyleImageLayers_LayerType as LayerType;
         use gecko_bindings::structs::{NS_STYLE_GRADIENT_SHAPE_LINEAR, NS_STYLE_GRADIENT_SIZE_FARTHEST_CORNER};
         use gecko_bindings::structs::nsStyleCoord;
         use style::values::computed::Image;
@@ -958,11 +959,10 @@ fn static_assert() {
             for image in &mut self.gecko.mImage.mLayers {
                 Gecko_SetNullImageValue(&mut image.mImage)
             }
-        }
+            Gecko_EnsureImageLayersLength(&mut self.gecko.mImage, images.0.len());
-
+            for image in &mut self.gecko.mImage.mLayers {
-
+                Gecko_InitializeImageLayer(image, LayerType::Background);
-        unsafe {
+            }
-            Gecko_ImageLayers_EnsureLength(&mut self.gecko.mImage, images.0.len());
         }
 
         self.gecko.mImage.mImageCount = cmp::max(self.gecko.mImage.mLayers.len() as u32,