Bug 1458814: Remove a bit of trivially dead code. r?hiro draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 03 May 2018 18:41:17 +0200
changeset 791165 38745eca45f00132fd644cbbbef1e7eb0bf39d05
parent 791164 fc61ca4f024976349f8ed7b57ec2f3431c4e55d2
child 791166 a9db16d667e3b1cca378050eac3f581fff971986
push id108716
push userbmo:emilio@crisal.io
push dateThu, 03 May 2018 16:54:43 +0000
reviewershiro
bugs1458814
milestone61.0a1
Bug 1458814: Remove a bit of trivially dead code. r?hiro MozReview-Commit-ID: GG41v4TejBU
dom/smil/nsSMILCSSValueType.cpp
layout/style/StyleAnimationValue.cpp
servo/ports/geckolib/glue.rs
--- a/dom/smil/nsSMILCSSValueType.cpp
+++ b/dom/smil/nsSMILCSSValueType.cpp
@@ -35,49 +35,43 @@ using mozilla::StyleAnimationValue;
 typedef AutoTArray<RefPtr<RawServoAnimationValue>, 1> ServoAnimationValues;
 
 /*static*/ nsSMILCSSValueType nsSMILCSSValueType::sSingleton;
 
 struct ValueWrapper {
   ValueWrapper(nsCSSPropertyID aPropID, const AnimationValue& aValue)
     : mPropID(aPropID)
   {
-    if (aValue.mServo) {
-      mServoValues.AppendElement(aValue.mServo);
-      return;
-    }
-    MOZ_CRASH("old style system disabled");
+    MOZ_ASSERT(!aValue.IsNull());
+    mServoValues.AppendElement(aValue.mServo);
   }
   ValueWrapper(nsCSSPropertyID aPropID,
                const RefPtr<RawServoAnimationValue>& aValue)
     : mPropID(aPropID), mServoValues{(aValue)} {}
   ValueWrapper(nsCSSPropertyID aPropID, ServoAnimationValues&& aValues)
     : mPropID(aPropID), mServoValues{aValues} {}
 
   bool operator==(const ValueWrapper& aOther) const
   {
     if (mPropID != aOther.mPropID) {
       return false;
     }
 
-    if (!mServoValues.IsEmpty()) {
-      size_t len = mServoValues.Length();
-      if (len != aOther.mServoValues.Length()) {
+    MOZ_ASSERT(!mServoValues.IsEmpty());
+    size_t len = mServoValues.Length();
+    if (len != aOther.mServoValues.Length()) {
+      return false;
+    }
+    for (size_t i = 0; i < len; i++) {
+      if (!Servo_AnimationValue_DeepEqual(mServoValues[i],
+                                          aOther.mServoValues[i])) {
         return false;
       }
-      for (size_t i = 0; i < len; i++) {
-        if (!Servo_AnimationValue_DeepEqual(mServoValues[i],
-                                            aOther.mServoValues[i])) {
-          return false;
-        }
-      }
-      return true;
     }
-
-    MOZ_CRASH("old style system disabled");
+    return true;
   }
 
   bool operator!=(const ValueWrapper& aOther) const
   {
     return !(*this == aOther);
   }
 
   nsCSSPropertyID mPropID;
@@ -301,28 +295,21 @@ AddOrAccumulate(nsSMILValue& aDest, cons
       property == eCSSProperty_stroke_dasharray) {
     return false;
   }
   // Skip font shorthand since it includes font-size-adjust.
   if (property == eCSSProperty_font) {
     return false;
   }
 
-  bool isServo = valueToAddWrapper
-                 ? !valueToAddWrapper->mServoValues.IsEmpty()
-                 : !destWrapper->mServoValues.IsEmpty();
-  if (isServo) {
-    return AddOrAccumulateForServo(aDest,
-                                   valueToAddWrapper,
-                                   destWrapper,
-                                   aCompositeOp,
-                                   aCount);
-  }
-
-  MOZ_CRASH("old style system disabled");
+  return AddOrAccumulateForServo(aDest,
+                                 valueToAddWrapper,
+                                 destWrapper,
+                                 aCompositeOp,
+                                 aCount);
 }
 
 nsresult
 nsSMILCSSValueType::SandwichAdd(nsSMILValue& aDest,
                                 const nsSMILValue& aValueToAdd) const
 {
   return AddOrAccumulate(aDest, aValueToAdd, CompositeOperation::Add, 1)
          ? NS_OK
@@ -384,22 +371,17 @@ nsSMILCSSValueType::ComputeDistance(cons
 {
   MOZ_ASSERT(aFrom.mType == aTo.mType,
              "Trying to compare different types");
   MOZ_ASSERT(aFrom.mType == this, "Unexpected source type");
 
   const ValueWrapper* fromWrapper = ExtractValueWrapper(aFrom);
   const ValueWrapper* toWrapper = ExtractValueWrapper(aTo);
   MOZ_ASSERT(toWrapper, "expecting non-null endpoint");
-
-  if (!toWrapper->mServoValues.IsEmpty()) {
-    return ComputeDistanceForServo(fromWrapper, *toWrapper, aDistance);
-  }
-
-  MOZ_CRASH("old style system disabled");
+  return ComputeDistanceForServo(fromWrapper, *toWrapper, aDistance);
 }
 
 
 static nsresult
 InterpolateForServo(const ValueWrapper* aStartWrapper,
                     const ValueWrapper& aEndWrapper,
                     double aUnitDistance,
                     nsSMILValue& aResult)
@@ -463,25 +445,20 @@ nsSMILCSSValueType::Interpolate(const ns
   MOZ_ASSERT(aResult.mType == this, "Unexpected result type");
   MOZ_ASSERT(aUnitDistance >= 0.0 && aUnitDistance <= 1.0,
              "unit distance value out of bounds");
   MOZ_ASSERT(!aResult.mU.mPtr, "expecting barely-initialized outparam");
 
   const ValueWrapper* startWrapper = ExtractValueWrapper(aStartVal);
   const ValueWrapper* endWrapper = ExtractValueWrapper(aEndVal);
   MOZ_ASSERT(endWrapper, "expecting non-null endpoint");
-
-  if (!endWrapper->mServoValues.IsEmpty()) {
-    return InterpolateForServo(startWrapper,
-                               *endWrapper,
-                               aUnitDistance,
-                               aResult);
-  }
-
-  MOZ_CRASH("old style system disabled");
+  return InterpolateForServo(startWrapper,
+                             *endWrapper,
+                             aUnitDistance,
+                             aResult);
 }
 
 // Helper function to extract presContext
 static nsPresContext*
 GetPresContextForElement(Element* aElem)
 {
   nsIDocument* doc = aElem->GetUncomposedDoc();
   if (!doc) {
@@ -608,20 +585,16 @@ nsSMILCSSValueType::ValueToString(const 
 {
   MOZ_ASSERT(aValue.mType == &nsSMILCSSValueType::sSingleton,
              "Unexpected SMIL value type");
   const ValueWrapper* wrapper = ExtractValueWrapper(aValue);
   if (!wrapper) {
     return;
   }
 
-  if (wrapper->mServoValues.IsEmpty()) {
-    MOZ_CRASH("old style system disabled");
-  }
-
   if (nsCSSProps::IsShorthand(wrapper->mPropID)) {
     // In case of shorthand on servo, we iterate over all mServoValues array
     // since we have multiple AnimationValues in the array for each longhand
     // component.
     Servo_Shorthand_AnimationValues_Serialize(wrapper->mPropID,
                                               &wrapper->mServoValues,
                                               &aString);
     return;
@@ -686,28 +659,22 @@ nsSMILCSSValueType::FinalizeValue(nsSMIL
   }
 
   const ValueWrapper* valueToMatchWrapper = ExtractValueWrapper(aValueToMatch);
   if (!valueToMatchWrapper) {
     MOZ_ASSERT_UNREACHABLE("Value to match is empty");
     return;
   }
 
-  bool isServo = !valueToMatchWrapper->mServoValues.IsEmpty();
-
-  if (isServo) {
-    ServoAnimationValues zeroValues;
-    zeroValues.SetCapacity(valueToMatchWrapper->mServoValues.Length());
+  ServoAnimationValues zeroValues;
+  zeroValues.SetCapacity(valueToMatchWrapper->mServoValues.Length());
 
-    for (auto& valueToMatch : valueToMatchWrapper->mServoValues) {
-      RefPtr<RawServoAnimationValue> zeroValue =
-        Servo_AnimationValues_GetZeroValue(valueToMatch).Consume();
-      if (!zeroValue) {
-        return;
-      }
-      zeroValues.AppendElement(Move(zeroValue));
+  for (auto& valueToMatch : valueToMatchWrapper->mServoValues) {
+    RefPtr<RawServoAnimationValue> zeroValue =
+      Servo_AnimationValues_GetZeroValue(valueToMatch).Consume();
+    if (!zeroValue) {
+      return;
     }
-    aValue.mU.mPtr = new ValueWrapper(valueToMatchWrapper->mPropID,
-                                      Move(zeroValues));
-  } else {
-    MOZ_CRASH("old style system disabled");
+    zeroValues.AppendElement(Move(zeroValue));
   }
+  aValue.mU.mPtr = new ValueWrapper(valueToMatchWrapper->mPropID,
+                                    Move(zeroValues));
 }
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -113,102 +113,76 @@ AnimationValue::operator!=(const Animati
 {
   return !operator==(aOther);
 }
 
 float
 AnimationValue::GetOpacity() const
 {
   MOZ_ASSERT(mServo);
-  if (mServo) {
-    return Servo_AnimationValue_GetOpacity(mServo);
-  }
-  MOZ_CRASH("old style system disabled");
+  return Servo_AnimationValue_GetOpacity(mServo);
 }
 
 already_AddRefed<const nsCSSValueSharedList>
 AnimationValue::GetTransformList() const
 {
   MOZ_ASSERT(mServo);
 
   RefPtr<nsCSSValueSharedList> transform;
-  if (mServo) {
-    Servo_AnimationValue_GetTransform(mServo, &transform);
-  } else {
-    MOZ_CRASH("old style system disabled");
-  }
+  Servo_AnimationValue_GetTransform(mServo, &transform);
   return transform.forget();
 }
 
 Size
 AnimationValue::GetScaleValue(const nsIFrame* aFrame) const
 {
   MOZ_ASSERT(mServo);
-
-  if (mServo) {
-    RefPtr<nsCSSValueSharedList> list;
-    Servo_AnimationValue_GetTransform(mServo, &list);
-    return nsStyleTransformMatrix::GetScaleValue(list, aFrame);
-  }
-  MOZ_CRASH("old style system disabled");
+  RefPtr<nsCSSValueSharedList> list;
+  Servo_AnimationValue_GetTransform(mServo, &list);
+  return nsStyleTransformMatrix::GetScaleValue(list, aFrame);
 }
 
 void
 AnimationValue::SerializeSpecifiedValue(nsCSSPropertyID aProperty,
                                         nsAString& aString) const
 {
   MOZ_ASSERT(mServo);
-
-  if (mServo) {
-    Servo_AnimationValue_Serialize(mServo, aProperty, &aString);
-    return;
-  }
-
-  MOZ_CRASH("old style system disabled");
+  Servo_AnimationValue_Serialize(mServo, aProperty, &aString);
 }
 
 bool
 AnimationValue::IsInterpolableWith(nsCSSPropertyID aProperty,
                                    const AnimationValue& aToValue) const
 {
   if (IsNull() || aToValue.IsNull()) {
     return false;
   }
 
   MOZ_ASSERT(mServo);
   MOZ_ASSERT(aToValue.mServo);
-
-  if (mServo) {
-    return Servo_AnimationValues_IsInterpolable(mServo, aToValue.mServo);
-  }
-
-  MOZ_CRASH("old style system disabled");
+  return Servo_AnimationValues_IsInterpolable(mServo, aToValue.mServo);
 }
 
 double
 AnimationValue::ComputeDistance(nsCSSPropertyID aProperty,
                                 const AnimationValue& aOther,
                                 ComputedStyle* aComputedStyle) const
 {
   if (IsNull() || aOther.IsNull()) {
     return 0.0;
   }
 
   MOZ_ASSERT(mServo);
   MOZ_ASSERT(aOther.mServo);
 
-  double distance= 0.0;
-  if (mServo) {
-    distance = Servo_AnimationValues_ComputeDistance(mServo, aOther.mServo);
-    return distance < 0.0
-           ? 0.0
-           : distance;
-  }
-
-  MOZ_CRASH("old style system disabled");
+  double distance =
+    Servo_AnimationValues_ComputeDistance(mServo, aOther.mServo);
+  return distance < 0.0
+         ? 0.0
+         : distance;
 }
 
 /* static */ AnimationValue
 AnimationValue::FromString(nsCSSPropertyID aProperty,
                            const nsAString& aValue,
                            Element* aElement)
 {
   MOZ_ASSERT(aElement);
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -4515,17 +4515,16 @@ impl<'a> Iterator for PrioritizedPropert
 #[no_mangle]
 pub extern "C" fn Servo_GetComputedKeyframeValues(
     keyframes: RawGeckoKeyframeListBorrowed,
     element: RawGeckoElementBorrowed,
     style: ComputedStyleBorrowed,
     raw_data: RawServoStyleSetBorrowed,
     computed_keyframes: RawGeckoComputedKeyframeValuesListBorrowedMut
 ) {
-    use std::mem;
     use style::properties::LonghandIdSet;
 
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
     let metrics = get_metrics_provider_for_product();
 
     let element = GeckoElement(element);
     let parent_element = element.inheritance_parent();
     let parent_data = parent_element.as_ref().and_then(|e| e.borrow_data());
@@ -4576,20 +4575,16 @@ pub extern "C" fn Servo_GetComputedKeyfr
                 if seen.contains(property) {
                     return;
                 }
                 seen.insert(property);
 
                 // This is safe since we immediately write to the uninitialized values.
                 unsafe { animation_values.set_len((property_index + 1) as u32) };
                 animation_values[property_index].mProperty = property.to_nscsspropertyid();
-                // We only make sure we have enough space for this variable,
-                // but didn't construct a default value for StyleAnimationValue,
-                // so we should zero it to avoid getting undefined behaviors.
-                animation_values[property_index].mValue.mGecko = unsafe { mem::zeroed() };
                 match value {
                     Some(v) => {
                         animation_values[property_index].mValue.mServo.set_arc_leaky(Arc::new(v));
                     },
                     None => {
                         animation_values[property_index].mValue.mServo.mRawPtr = ptr::null_mut();
                     },
                 }