Bug 1245748 - Allow StyleAnimationValue::UncomputeValue to produce values whose storage is independent of the passed-in computed value; r?heycam
When we go to switch CSS Animations over to using
KeyframeEffectReadOnly::SetFrames we will need a way to represent any filled-in
from/to values as nsCSSValue objects. These objects are built from the current
computed style. We currently use StyleAnimationValue::ExtractComputedValue for
this which returns a StyleAnimationValue. In order to convert this to an
nsCSSValue we can use StyleAnimationValue::UncomputeValue. However, in some
cases, the nsCSSValue objects returned by that method are dependent on the
passed-in StyleAnimationValue object.
This patch adds an overload to UncomputeValue that takes an rvalue
StyleAnimationValue reference and produces an nsCSSValue that is independent
of the StyleAnimationValue through a combination of copying data and
transferring ownership of data.
This patch also adjusts the return value for the case of filter and shadow
lists when the list is empty so that we return a none value in this case.
These are the only list types which are allowed to have a null list value.
Not only does this produce the correct result when these values are serialized
(the initial value for 'filter', 'text-shadow', and 'box-shadow' is 'none') it
also means that UncomputeValue should never return an nsCSSValue whose unit is
null which is important because when we later pass that value to BuildStyleRule
it will treat a null nsCSSValue as an error case (specifically, "longhand failed
to parse").
MozReview-Commit-ID: 4RoCn39ntiJ
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -2857,18 +2857,24 @@ StyleAnimationValue::UncomputeValue(nsCS
case eUnit_CSSRect: {
nsCSSRect& rect = aSpecifiedValue.SetRectValue();
rect = *aComputedValue.GetCSSRectValue();
} break;
case eUnit_Dasharray:
case eUnit_Shadow:
case eUnit_Filter:
case eUnit_BackgroundPosition:
- aSpecifiedValue.
- SetDependentListValue(aComputedValue.GetCSSValueListValue());
+ {
+ nsCSSValueList* computedList = aComputedValue.GetCSSValueListValue();
+ if (computedList) {
+ aSpecifiedValue.SetDependentListValue(computedList);
+ } else {
+ aSpecifiedValue.SetNoneValue();
+ }
+ }
break;
case eUnit_Transform:
aSpecifiedValue.
SetSharedListValue(aComputedValue.GetCSSValueSharedListValue());
break;
case eUnit_CSSValuePairList:
aSpecifiedValue.
SetDependentPairListValue(aComputedValue.GetCSSValuePairListValue());
@@ -2876,16 +2882,51 @@ StyleAnimationValue::UncomputeValue(nsCS
default:
return false;
}
return true;
}
bool
StyleAnimationValue::UncomputeValue(nsCSSProperty aProperty,
+ StyleAnimationValue&& aComputedValue,
+ nsCSSValue& aSpecifiedValue)
+{
+ Unit unit = aComputedValue.GetUnit();
+ switch (unit) {
+ case eUnit_Dasharray:
+ case eUnit_Shadow:
+ case eUnit_Filter:
+ case eUnit_BackgroundPosition:
+ {
+ UniquePtr<nsCSSValueList> computedList =
+ aComputedValue.TakeCSSValueListValue();
+ if (computedList) {
+ aSpecifiedValue.AdoptListValue(computedList.release());
+ } else {
+ aSpecifiedValue.SetNoneValue();
+ }
+ }
+ break;
+ case eUnit_CSSValuePairList:
+ {
+ UniquePtr<nsCSSValuePairList> computedList =
+ aComputedValue.TakeCSSValuePairListValue();
+ MOZ_ASSERT(computedList, "Pair list should never be null");
+ aSpecifiedValue.AdoptPairListValue(computedList.release());
+ }
+ break;
+ default:
+ return UncomputeValue(aProperty, aComputedValue, aSpecifiedValue);
+ }
+ return true;
+}
+
+bool
+StyleAnimationValue::UncomputeValue(nsCSSProperty aProperty,
const StyleAnimationValue& aComputedValue,
nsAString& aSpecifiedValue)
{
aSpecifiedValue.Truncate(); // Clear outparam, if it's not already empty
if (aComputedValue.GetUnit() == eUnit_UnparsedString) {
aComputedValue.GetStringValue(aSpecifiedValue);
return true;
--- a/layout/style/StyleAnimationValue.h
+++ b/layout/style/StyleAnimationValue.h
@@ -4,16 +4,17 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
/* Utilities for animation of computed style values */
#ifndef mozilla_StyleAnimationValue_h_
#define mozilla_StyleAnimationValue_h_
#include "mozilla/gfx/MatrixFwd.h"
+#include "mozilla/UniquePtr.h"
#include "nsStringFwd.h"
#include "nsStringBuffer.h"
#include "nsCoord.h"
#include "nsColor.h"
#include "nsCSSProps.h"
#include "nsCSSValue.h"
class nsIFrame;
@@ -192,30 +193,37 @@ public:
nsStyleContext* aStyleContext,
const nsCSSValue& aSpecifiedValue,
bool aUseSVGMode,
nsTArray<PropertyStyleAnimationValuePair>& aResult);
/**
* Creates a specified value for the given computed value.
*
- * The first overload fills in an nsCSSValue object; the second
- * produces a string. The nsCSSValue result may depend on objects
- * owned by the |aComputedValue| object, so users of that variant
+ * The first two overloads fill in an nsCSSValue object; the third
+ * produces a string. For the overload that takes a const
+ * StyleAnimationValue& reference, the nsCSSValue result may depend on
+ * objects owned by the |aComputedValue| object, so users of that variant
* must keep |aComputedValue| alive longer than |aSpecifiedValue|.
+ * The overload that takes an rvalue StyleAnimationValue reference
+ * transfers ownership for some resources such that the |aComputedValue|
+ * does not depend on the lifetime of |aSpecifiedValue|.
*
* @param aProperty The property whose value we're uncomputing.
* @param aComputedValue The computed value to be converted.
* @param [out] aSpecifiedValue The resulting specified value.
* @return true on success, false on failure.
*/
static bool UncomputeValue(nsCSSProperty aProperty,
const StyleAnimationValue& aComputedValue,
nsCSSValue& aSpecifiedValue);
static bool UncomputeValue(nsCSSProperty aProperty,
+ StyleAnimationValue&& aComputedValue,
+ nsCSSValue& aSpecifiedValue);
+ static bool UncomputeValue(nsCSSProperty aProperty,
const StyleAnimationValue& aComputedValue,
nsAString& aSpecifiedValue);
/**
* Gets the computed value for the given property from the given style
* context.
*
* @param aProperty The property whose value we're looking up.
@@ -361,16 +369,29 @@ public:
aBuffer.Truncate();
uint32_t len = NS_strlen(GetBufferValue(mValue.mString));
mValue.mString->ToString(len, aBuffer);
}
/// @return the scale for this value, calculated with reference to @aForFrame.
gfxSize GetScaleValue(const nsIFrame* aForFrame) const;
+ UniquePtr<nsCSSValueList> TakeCSSValueListValue() {
+ nsCSSValueList* list = GetCSSValueListValue();
+ mValue.mCSSValueList = nullptr;
+ mUnit = eUnit_Null;
+ return UniquePtr<nsCSSValueList>(list);
+ }
+ UniquePtr<nsCSSValuePairList> TakeCSSValuePairListValue() {
+ nsCSSValuePairList* list = GetCSSValuePairListValue();
+ mValue.mCSSValuePairList = nullptr;
+ mUnit = eUnit_Null;
+ return UniquePtr<nsCSSValuePairList>(list);
+ }
+
explicit StyleAnimationValue(Unit aUnit = eUnit_Null) : mUnit(aUnit) {
NS_ASSERTION(aUnit == eUnit_Null || aUnit == eUnit_Normal ||
aUnit == eUnit_Auto || aUnit == eUnit_None,
"must be valueless unit");
}
StyleAnimationValue(const StyleAnimationValue& aOther)
: mUnit(eUnit_Null) { *this = aOther; }
StyleAnimationValue(StyleAnimationValue&& aOther)
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -616,16 +616,31 @@ void nsCSSValue::SetDependentListValue(n
{
Reset();
if (aList) {
mUnit = eCSSUnit_ListDep;
mValue.mListDependent = aList;
}
}
+void
+nsCSSValue::AdoptListValue(nsCSSValueList*&& aValue)
+{
+ // We have to copy the first element since for owned lists the first
+ // element should be an nsCSSValueList_heap object.
+ SetListValue();
+ // FIXME: If nsCSSValue gets a swap method or move assignment operator,
+ // we should use that here to avoid allocating an extra value.
+ mValue.mList->mValue = aValue->mValue;
+ mValue.mList->mNext = aValue->mNext;
+ aValue->mNext = nullptr;
+ delete aValue;
+ aValue = nullptr;
+}
+
nsCSSValuePairList* nsCSSValue::SetPairListValue()
{
Reset();
mUnit = eCSSUnit_PairList;
mValue.mPairList = new nsCSSValuePairList_heap;
mValue.mPairList->AddRef();
return mValue.mPairList;
}
@@ -634,16 +649,32 @@ void nsCSSValue::SetDependentPairListVal
{
Reset();
if (aList) {
mUnit = eCSSUnit_PairListDep;
mValue.mPairListDependent = aList;
}
}
+void
+nsCSSValue::AdoptPairListValue(nsCSSValuePairList*&& aValue)
+{
+ // We have to copy the first element since for owned pair lists the first
+ // element should be an nsCSSValuePairList_heap object.
+ SetPairListValue();
+ // FIXME: If nsCSSValue gets a swap method or move assignment operator,
+ // we should use that here to avoid allocating extra values.
+ mValue.mPairList->mXValue = aValue->mXValue;
+ mValue.mPairList->mYValue = aValue->mYValue;
+ mValue.mPairList->mNext = aValue->mNext;
+ aValue->mNext = nullptr;
+ delete aValue;
+ aValue = nullptr;
+}
+
void nsCSSValue::SetAutoValue()
{
Reset();
mUnit = eCSSUnit_Auto;
}
void nsCSSValue::SetInheritValue()
{
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -713,16 +713,20 @@ public:
void SetDummyInheritValue();
// These are a little different - they allocate storage for you and
// return a handle.
nsCSSRect& SetRectValue();
nsCSSValueList* SetListValue();
nsCSSValuePairList* SetPairListValue();
+ // These take ownership of the passed-in resource.
+ void AdoptListValue(nsCSSValueList*&& aValue);
+ void AdoptPairListValue(nsCSSValuePairList*&& aValue);
+
void StartImageLoad(nsIDocument* aDocument) const; // Only pretend const
// Initializes as a function value with the specified function id.
Array* InitFunction(nsCSSKeyword aFunctionId, uint32_t aNumArgs);
// Checks if this is a function value with the specified function id.
bool EqualsFunction(nsCSSKeyword aFunctionId) const;
// Returns an already addrefed buffer. Guaranteed to return non-null.
@@ -986,28 +990,28 @@ public:
// This has to be here so that the relationship between nsCSSValueList
// and nsCSSValueList_heap is visible.
inline nsCSSValueList*
nsCSSValue::GetListValue()
{
if (mUnit == eCSSUnit_List)
return mValue.mList;
else {
- MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a pairlist value");
+ MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a list value");
return mValue.mListDependent;
}
}
inline const nsCSSValueList*
nsCSSValue::GetListValue() const
{
if (mUnit == eCSSUnit_List)
return mValue.mList;
else {
- MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a pairlist value");
+ MOZ_ASSERT(mUnit == eCSSUnit_ListDep, "not a list value");
return mValue.mListDependent;
}
}
struct nsCSSRect {
nsCSSRect(void);
nsCSSRect(const nsCSSRect& aCopy);
~nsCSSRect();