Bug 1414000 - Assert that either the pres context is nullptr OR that there are no properties when filling in base styles; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 20 Dec 2017 17:34:57 +0900
changeset 713318 a11a2b18e51cc0bc4043c8cdc0ee6ef1def9afc1
parent 712870 7c4579e705c4a3a3610183fe6f44affff3ad57ef
child 744324 af109dff9322017ec346bed5a3d4d6aa8c0edfdf
push id93631
push userbmo:bbirtles@mozilla.com
push dateWed, 20 Dec 2017 08:39:14 +0000
reviewershiro
bugs1414000, 1407898
milestone59.0a1
Bug 1414000 - Assert that either the pres context is nullptr OR that there are no properties when filling in base styles; r?hiro The call stack where this assertion would otherwise fail is as follows: KeyframeEffectReadOnly::UpdateProperties KeyframeEffectReadOnly::DoUpdateProperties KeyframeEffectReadOnly::BuildProperties KeyframeUtils::GetAnimationPropertiesFromKeyframes KeyframeUtils.cpp::GetComputedKeyframeValues KeyframeEffectReadOnly::EnsureBaseStyles In bug 1407898 we made GetComputedKeyframes return an empty list when the pres context is nullptr so if we get a null pres context in EnsureBaseStyles (which uses the same method for getting the pres context: nsContentUtils::GetContextForContent) we know that |aProperties| will be empty. Also, if |aProperties| is empty we're not going to dereferences |presContext| so we don't need to assert that it is non-null. I have not included the crashtest in this patch for the same reason as described in bug 1407898 comment 6. MozReview-Commit-ID: 6OZ2yJfRLMV
dom/animation/KeyframeEffectReadOnly.cpp
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -518,20 +518,29 @@ KeyframeEffectReadOnly::EnsureBaseStyles
   if (!mTarget) {
     return;
   }
 
   mBaseStyleValuesForServo.Clear();
 
   nsPresContext* presContext =
     nsContentUtils::GetContextForContent(mTarget->mElement);
-  MOZ_ASSERT(presContext,
-             "nsPresContext should not be nullptr since this EnsureBaseStyles "
-             "supposed to be called right after getting computed values with "
-             "a valid nsPresContext");
+  // If |aProperties| is empty we're not going to dereference |presContext| so
+  // we don't care if it is nullptr.
+  //
+  // We could just return early when |aProperties| is empty and save looking up
+  // the pres context, but that won't save any effort normally since we don't
+  // call this function if we have no keyframes to begin with. Furthermore, the
+  // case where |presContext| is nullptr is so rare (we've only ever seen in
+  // fuzzing, and even then we've never been able to reproduce it reliably)
+  // it's not worth the runtime cost of an extra branch.
+  MOZ_ASSERT(presContext || aProperties.IsEmpty(),
+             "Typically presContext should not be nullptr but if it is"
+             " we should have also failed to calculate the computed values"
+             " passed-in as aProperties");
 
   RefPtr<ServoStyleContext> baseStyleContext;
   for (const AnimationProperty& property : aProperties) {
     EnsureBaseStyle(property,
                     presContext,
                     aComputedValues,
                     baseStyleContext);
   }