Bug 1414000 - Assert that either the pres context is nullptr OR that there are no properties when filling in base styles; r=hiro
authorBrian Birtles <birtles@gmail.com>
Wed, 20 Dec 2017 17:34:57 +0900
changeset 397049 2b5bd02642c44cae62932d75c5d0fbe3486c7c93
parent 397048 cd534162b8f47e02dbb10f6a23e2b9d72d4deac2
child 397050 ca8505867b1aff500a589371083d990d5cc01877
push id33123
push userncsoregi@mozilla.com
push dateThu, 21 Dec 2017 10:00:47 +0000
treeherdermozilla-central@06a19fbe2581 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1414000, 1407898
milestone59.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 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);
   }