Address review comments

4 files changed, 15 insertions, 13 deletions

diff --git a/components/style/gecko_conversions.rs b/components/style/gecko_conversions.rs
index ca43c5b07e0..95ab9eb8b00 100644
--- a/components/style/gecko_conversions.rs
+++ b/components/style/gecko_conversions.rs
@@ -14,7 +14,7 @@ impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {
         let has_percentage = other.percentage.is_some();
         nsStyleCoord_CalcValue {
-            mLength: other.length.map(|l| l.0).unwrap_or(0),
+            mLength: other.length.map_or(0, |l| l.0),
             mPercent: other.percentage.unwrap_or(0.0),
             mHasPercent: has_percentage,
         }
diff --git a/ports/geckolib/gecko_bindings/sugar/ns_style_coord.rs b/ports/geckolib/gecko_bindings/sugar/ns_style_coord.rs
index d90c2dd2b39..ef03f4d7b41 100644
--- a/ports/geckolib/gecko_bindings/sugar/ns_style_coord.rs
+++ b/ports/geckolib/gecko_bindings/sugar/ns_style_coord.rs
@@ -25,7 +25,7 @@ impl nsStyleUnion {
     /// reset() the union before calling this
     pub unsafe fn set_calc_value(&mut self, unit: &mut nsStyleUnit, v: nsStyleCoord_CalcValue) {
         // Calc should have been cleaned up
-        assert!(*unit != nsStyleUnit::eStyleUnit_Calc);
+        debug_assert!(*unit != nsStyleUnit::eStyleUnit_Calc);
         Gecko_SetStyleCoordCalcValue(unit, self, v);
     }
 
@@ -33,22 +33,22 @@ impl nsStyleUnion {
         (*self.as_calc())._base
     }
 
-    pub unsafe fn addref_opt(&mut self, unit: &nsStyleUnit) {
+    pub unsafe fn addref_if_calc(&mut self, unit: &nsStyleUnit) {
         if *unit == nsStyleUnit::eStyleUnit_Calc {
             Gecko_AddRefCalcArbitraryThread(self.as_calc_mut());
         }
     }
 
-    pub unsafe fn as_calc_mut(&mut self) -> &mut nsStyleCoord_Calc {
+    unsafe fn as_calc_mut(&mut self) -> &mut nsStyleCoord_Calc {
         transmute(*self.mPointer.as_mut() as *mut nsStyleCoord_Calc)
     }
-    pub unsafe fn as_calc(&self) -> &nsStyleCoord_Calc {
+    unsafe fn as_calc(&self) -> &nsStyleCoord_Calc {
         transmute(*self.mPointer.as_ref() as *const nsStyleCoord_Calc)
     }
 }
 
 impl nsStyleCoord {
-    pub unsafe fn addref_opt(&mut self) {
-        self.mValue.addref_opt(&self.mUnit);
+    pub unsafe fn addref_if_calc(&mut self) {
+        self.mValue.addref_if_calc(&self.mUnit);
     }
 }
diff --git a/ports/geckolib/properties.mako.rs b/ports/geckolib/properties.mako.rs
index d4c0fbadf19..748aeca4f02 100644
--- a/ports/geckolib/properties.mako.rs
+++ b/ports/geckolib/properties.mako.rs
@@ -320,7 +320,7 @@ def set_gecko_property(ffi_name, expr):
         unsafe { self.gecko.${union_ffi_name}.reset(&mut self.gecko.${unit_ffi_name}) };
         self.gecko.${unit_ffi_name} =  other.gecko.${unit_ffi_name};
         self.gecko.${union_ffi_name} = other.gecko.${union_ffi_name};
-        unsafe { self.gecko.${union_ffi_name}.addref_opt(&self.gecko.${unit_ffi_name}) };
+        unsafe { self.gecko.${union_ffi_name}.addref_if_calc(&self.gecko.${unit_ffi_name}) };
     }
     % if need_clone:
         fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
@@ -354,8 +354,8 @@ ${impl_split_style_coord(ident,
         self.gecko.${x_union_ffi_name} = other.gecko.${x_union_ffi_name};
         self.gecko.${y_unit_ffi_name} = other.gecko.${y_unit_ffi_name};
         self.gecko.${y_union_ffi_name} = other.gecko.${y_union_ffi_name};
-        unsafe { self.gecko.${x_union_ffi_name}.addref_opt(&self.gecko.${x_unit_ffi_name}) };
-        unsafe { self.gecko.${y_union_ffi_name}.addref_opt(&self.gecko.${y_unit_ffi_name}) };
+        unsafe { self.gecko.${x_union_ffi_name}.addref_if_calc(&self.gecko.${x_unit_ffi_name}) };
+        unsafe { self.gecko.${y_union_ffi_name}.addref_if_calc(&self.gecko.${y_unit_ffi_name}) };
     }
     % if need_clone:
         fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
@@ -631,10 +631,12 @@ fn static_assert() {
     }
 
     fn copy_z_index_from(&mut self, other: &Self) {
-        unsafe { self.gecko.mZIndex.mValue.reset(&mut self.gecko.mZIndex.mUnit) };
+        use gecko_bindings::structs::nsStyleUnit;
+        // z-index is never a calc(). If it were, we'd be leaking here, so
+        // assert that it isn't.
+        debug_assert!(self.gecko.mZIndex.mUnit != nsStyleUnit::eStyleUnit_Calc);
         self.gecko.mZIndex.mUnit = other.gecko.mZIndex.mUnit;
         self.gecko.mZIndex.mValue = other.gecko.mZIndex.mValue;
-        unsafe { self.gecko.mZIndex.mValue.addref_opt(&self.gecko.mZIndex.mUnit) };
     }
 
     fn clone_z_index(&self) -> longhands::z_index::computed_value::T {
diff --git a/ports/geckolib/values.rs b/ports/geckolib/values.rs
index b1fb37f2246..b44d78c084e 100644
--- a/ports/geckolib/values.rs
+++ b/ports/geckolib/values.rs
@@ -46,7 +46,7 @@ impl StyleCoordHelpers for nsStyleCoord {
         unsafe { self.mValue.reset(&mut self.mUnit) };
         self.mUnit = other.mUnit;
         self.mValue = other.mValue;
-        unsafe { self.addref_opt(); }
+        unsafe { self.addref_if_calc(); }
     }
 
     #[inline]