mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-10-11 04:15:43 +00:00
c0aa9c7ed3
This assertion was originally added in bug 1353208 because in that bug we changed the type of nsSMILCompositor::mCachedBaseValue from nsAutoPtr<nsSMILValue> to just nsSMILValue. When using nsAutoPtr, mCachedBaseValue had two null states: one where the pointer is null, and one where the pointed-to nsSMILValue is null. Coalescing these two states simplifies the code but there is one case where the difference is significant as described in the commit message for that changeset (mozilla-central changeset ad7060dae117): "There's a subtle difference in behavior with regards to the first sample. Previously we would compare the (initially) null mCachedBaseValue pointer with the passed-in nsSMILValue and set mForceCompositing to true. With this patch, however, we will only set mForceCompositing to true if the passed-in mCachedBaseValue is not null." That is, if the base value we get back is a null nsSMILValue, previously we would set mForceCompositing to true unconditionally, but with the changes in bug 1353208 we would only set that to true if the passed-in nsSMILValue was not null. We believed that would never matter since the passed-in nsSMILValue would never be null if we called GetBaseValue. Quoting from that same commit message: "... if we do call GetBaseValue the result should not be a null nsSMILValue (except in some OOM cases where we don't really care if we miss a sample). This patch adds an assertion to check that GetBaseValue does, in fact, return a non-null value. (I checked the code and this appears to be the case. Even in error cases we typically return an empty nsSMILValue of a non-null type. For example, the early return in nsSMILCSSProperty::GetBaseValue() does this.)" We added an assertion to validate that assumption but the crashtest included in this patch demonstrates a case where it does not hold (specifically, when nsStyleUtil::CSPAllowsInlineStyle returns false, nsCSSProperty::GetBaseValue will return a null nsSMILValue). That would seem to suggest that there is at least one case where we might fail to set mForceIsCompositing to true and hence fail to update style on this first sample (and presumably thereonwards too since future comparisons of mCachedBaseValue will compare equal). However, for the case of an initial sample mForceCompositing should already be set to true since set we update mForceCompositing in nsSMILCompositor::GetFirstFuncToAffectSandwich() and will make it true if *anything* in the animation function has changed and at this point, the initial sample, *everything* will have changed. Hence, I believe dropping this assertion is acceptable. I have confirmed that in the crashtest in this patch, during the first sample mForceCompositing is set to true. I would create a reftest to test the behavior on the first sample but, at least for the specific case where inline style is disabled due to CSP, not updating style *is* the expected behavior so there will be no difference in behavior regardless of whether or not the mForceCompositing flag is set. MozReview-Commit-ID: Li0pZEH2PNl --HG-- extra : rebase_source : a1c12a019b8481600afa4295447dc1e6fb281b22 |
||
|---|---|---|
| .. | ||
| crashtests | ||
| test | ||
| moz.build | ||
| nsISMILAttr.h | ||
| nsISMILType.h | ||
| nsSMILAnimationController.cpp | ||
| nsSMILAnimationController.h | ||
| nsSMILAnimationFunction.cpp | ||
| nsSMILAnimationFunction.h | ||
| nsSMILCompositor.cpp | ||
| nsSMILCompositor.h | ||
| nsSMILCompositorTable.h | ||
| nsSMILCSSProperty.cpp | ||
| nsSMILCSSProperty.h | ||
| nsSMILCSSValueType.cpp | ||
| nsSMILCSSValueType.h | ||
| nsSMILFloatType.cpp | ||
| nsSMILFloatType.h | ||
| nsSMILInstanceTime.cpp | ||
| nsSMILInstanceTime.h | ||
| nsSMILInterval.cpp | ||
| nsSMILInterval.h | ||
| nsSMILKeySpline.cpp | ||
| nsSMILKeySpline.h | ||
| nsSMILMilestone.h | ||
| nsSMILNullType.cpp | ||
| nsSMILNullType.h | ||
| nsSMILParserUtils.cpp | ||
| nsSMILParserUtils.h | ||
| nsSMILRepeatCount.cpp | ||
| nsSMILRepeatCount.h | ||
| nsSMILSetAnimationFunction.cpp | ||
| nsSMILSetAnimationFunction.h | ||
| nsSMILTargetIdentifier.h | ||
| nsSMILTimeContainer.cpp | ||
| nsSMILTimeContainer.h | ||
| nsSMILTimedElement.cpp | ||
| nsSMILTimedElement.h | ||
| nsSMILTimeValue.cpp | ||
| nsSMILTimeValue.h | ||
| nsSMILTimeValueSpec.cpp | ||
| nsSMILTimeValueSpec.h | ||
| nsSMILTimeValueSpecParams.h | ||
| nsSMILTypes.h | ||
| nsSMILValue.cpp | ||
| nsSMILValue.h | ||
| SMILBoolType.cpp | ||
| SMILBoolType.h | ||
| SMILEnumType.cpp | ||
| SMILEnumType.h | ||
| SMILIntegerType.cpp | ||
| SMILIntegerType.h | ||
| SMILStringType.cpp | ||
| SMILStringType.h | ||
| TimeEvent.cpp | ||
| TimeEvent.h | ||