Modified: trunk/Source/WebCore/ChangeLog (287536 => 287537)
--- trunk/Source/WebCore/ChangeLog 2022-01-03 12:49:34 UTC (rev 287536)
+++ trunk/Source/WebCore/ChangeLog 2022-01-03 13:51:08 UTC (rev 287537)
@@ -1,5 +1,55 @@
2022-01-03 Antoine Quint <[email protected]>
+ Refactor code creating css values and lists for animation and transition properties
+ https://bugs.webkit.org/show_bug.cgi?id=234812
+
+ Reviewed by Antti Koivisto.
+
+ For bug 234792 we exposed a series of static functions on ComputedStyleExtractor to share
+ code between CSSPropertyParser.cpp and CSSComputedStyleDeclaration.cpp for the creation of
+ CSS values for CSS Animations properties. Darin suggested some refactoring as part of the
+ review of that bug which was even more appropriate following the fix for bug 234785.
+
+ We now expose a single ComputedStyleExtractor::addCSSValueForAnimationPropertyToList()
+ static method to add the CSSValue for a given animation or transition CSS property to
+ a CSS list, providing an optional Animation to read the value from that, otherwise
+ using the default value.
+
+ This allowed initially for shorter code in CSSPropertyParser::consumeAnimationShorthand()
+ and animationShorthandValue(). Looking at ComputedStyleExtractor::valueForPropertyInStyle(),
+ there were more opportunities to share code and so valueListForAnimationOrTransitionProperty()
+ was added as well.
+
+ * css/CSSComputedStyleDeclaration.cpp:
+ (WebCore::valueForAnimationDuration):
+ (WebCore::valueForAnimationDelay):
+ (WebCore::valueForAnimationIterationCount):
+ (WebCore::valueForAnimationDirection):
+ (WebCore::valueForAnimationFillMode):
+ (WebCore::valueForAnimationPlayState):
+ (WebCore::valueForAnimationName):
+ (WebCore::valueForAnimationTimingFunction):
+ (WebCore::ComputedStyleExtractor::addCSSValueForAnimationPropertyToList):
+ (WebCore::valueListForAnimationOrTransitionProperty):
+ (WebCore::animationShorthandValue):
+ (WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
+ (WebCore::ComputedStyleExtractor::valueForAnimationDuration): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationDelay): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationIterationCount): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationDirection): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationFillMode): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationPlayState): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationName): Deleted.
+ (WebCore::delayValue): Deleted.
+ (WebCore::durationValue): Deleted.
+ (WebCore::ComputedStyleExtractor::valueForAnimationTimingFunction): Deleted.
+ (WebCore::timingFunctionValue): Deleted.
+ * css/CSSComputedStyleDeclaration.h:
+ * css/parser/CSSPropertyParser.cpp:
+ (WebCore::CSSPropertyParser::consumeAnimationShorthand):
+
+2022-01-03 Antoine Quint <[email protected]>
+
Support the "animation" shorthand property in the computed style
https://bugs.webkit.org/show_bug.cgi?id=234785
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (287536 => 287537)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2022-01-03 12:49:34 UTC (rev 287536)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2022-01-03 13:51:08 UTC (rev 287537)
@@ -1345,17 +1345,17 @@
return valueList;
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationDuration(double duration)
+static Ref<CSSPrimitiveValue> valueForAnimationDuration(double duration)
{
return CSSValuePool::singleton().createValue(duration, CSSUnitType::CSS_S);
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationDelay(double delay)
+static Ref<CSSPrimitiveValue> valueForAnimationDelay(double delay)
{
return CSSValuePool::singleton().createValue(delay, CSSUnitType::CSS_S);
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationIterationCount(double iterationCount)
+static Ref<CSSPrimitiveValue> valueForAnimationIterationCount(double iterationCount)
{
if (iterationCount == Animation::IterationCountInfinite)
return CSSValuePool::singleton().createIdentifierValue(CSSValueInfinite);
@@ -1362,7 +1362,7 @@
return CSSValuePool::singleton().createValue(iterationCount, CSSUnitType::CSS_NUMBER);
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationDirection(Animation::AnimationDirection direction)
+static Ref<CSSPrimitiveValue> valueForAnimationDirection(Animation::AnimationDirection direction)
{
switch (direction) {
case Animation::AnimationDirectionNormal:
@@ -1376,7 +1376,7 @@
}
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationFillMode(AnimationFillMode fillMode)
+static Ref<CSSPrimitiveValue> valueForAnimationFillMode(AnimationFillMode fillMode)
{
switch (fillMode) {
case AnimationFillMode::None:
@@ -1390,7 +1390,7 @@
}
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationPlayState(AnimationPlayState playState)
+static Ref<CSSPrimitiveValue> valueForAnimationPlayState(AnimationPlayState playState)
{
switch (playState) {
case AnimationPlayState::Playing:
@@ -1400,7 +1400,7 @@
}
}
-Ref<CSSPrimitiveValue> ComputedStyleExtractor::valueForAnimationName(const Animation::Name& name)
+static Ref<CSSPrimitiveValue> valueForAnimationName(const Animation::Name& name)
{
if (name.isIdentifier)
return CSSValuePool::singleton().createCustomIdent(name.string);
@@ -1407,34 +1407,8 @@
return CSSValuePool::singleton().createValue(name.string, CSSUnitType::CSS_STRING);
}
-static Ref<CSSValueList> delayValue(const AnimationList* animationList)
+static Ref<CSSValue> valueForAnimationTimingFunction(const TimingFunction& timingFunction)
{
- auto list = CSSValueList::createCommaSeparated();
- if (animationList) {
- for (size_t i = 0; i < animationList->size(); ++i)
- list->append(ComputedStyleExtractor::valueForAnimationDelay(animationList->animation(i).delay()));
- } else {
- // Note that initialAnimationDelay() is used for both transitions and animations
- list->append(ComputedStyleExtractor::valueForAnimationDelay(Animation::initialDelay()));
- }
- return list;
-}
-
-static Ref<CSSValueList> durationValue(const AnimationList* animationList)
-{
- auto list = CSSValueList::createCommaSeparated();
- if (animationList) {
- for (size_t i = 0; i < animationList->size(); ++i)
- list->append(ComputedStyleExtractor::valueForAnimationDuration(animationList->animation(i).duration()));
- } else {
- // Note that initialAnimationDuration() is used for both transitions and animations
- list->append(ComputedStyleExtractor::valueForAnimationDuration(Animation::initialDuration()));
- }
- return list;
-}
-
-Ref<CSSValue> ComputedStyleExtractor::valueForAnimationTimingFunction(const TimingFunction& timingFunction)
-{
switch (timingFunction.type()) {
case TimingFunction::CubicBezierFunction: {
auto& function = downcast<CubicBezierTimingFunction>(timingFunction);
@@ -1473,15 +1447,39 @@
}
}
-static Ref<CSSValueList> timingFunctionValue(const AnimationList* animationList)
+void ComputedStyleExtractor::addValueForAnimationPropertyToList(CSSValueList& list, CSSPropertyID property, const Animation* animation)
{
+ if (property == CSSPropertyAnimationDuration || property == CSSPropertyTransitionDuration)
+ list.append(valueForAnimationDuration(animation ? animation->duration() : Animation::initialDuration()));
+ else if (property == CSSPropertyAnimationDelay || property == CSSPropertyTransitionDelay)
+ list.append(valueForAnimationDelay(animation ? animation->delay() : Animation::initialDelay()));
+ else if (property == CSSPropertyAnimationIterationCount)
+ list.append(valueForAnimationIterationCount(animation ? animation->iterationCount() : Animation::initialIterationCount()));
+ else if (property == CSSPropertyAnimationDirection)
+ list.append(valueForAnimationDirection(animation ? animation->direction() : Animation::initialDirection()));
+ else if (property == CSSPropertyAnimationFillMode)
+ list.append(valueForAnimationFillMode(animation ? animation->fillMode() : Animation::initialFillMode()));
+ else if (property == CSSPropertyAnimationPlayState)
+ list.append(valueForAnimationPlayState(animation ? animation->playState() : Animation::initialPlayState()));
+ else if (property == CSSPropertyAnimationName)
+ list.append(valueForAnimationName(animation ? animation->name() : Animation::initialName()));
+ else if (property == CSSPropertyAnimationTimingFunction || property == CSSPropertyTransitionTimingFunction) {
+ if (animation)
+ list.append(valueForAnimationTimingFunction(*animation->timingFunction()));
+ else
+ list.append(valueForAnimationTimingFunction(CubicBezierTimingFunction::defaultTimingFunction()));
+ } else
+ ASSERT_NOT_REACHED();
+}
+
+static Ref<CSSValueList> valueListForAnimationOrTransitionProperty(CSSPropertyID property, const AnimationList* animationList)
+{
auto list = CSSValueList::createCommaSeparated();
if (animationList) {
for (size_t i = 0; i < animationList->size(); ++i)
- list->append(ComputedStyleExtractor::valueForAnimationTimingFunction(*animationList->animation(i).timingFunction()));
+ ComputedStyleExtractor::addValueForAnimationPropertyToList(list.get(), property, &animationList->animation(i));
} else
- // Note that initialAnimationTimingFunction() is used for both transitions and animations
- list->append(ComputedStyleExtractor::valueForAnimationTimingFunction(Animation::initialTimingFunction()));
+ ComputedStyleExtractor::addValueForAnimationPropertyToList(list.get(), property, nullptr);
return list;
}
@@ -1492,14 +1490,9 @@
for (size_t i = 0; i < animationList->size(); ++i) {
const auto& animation = animationList->animation(i);
auto childList = CSSValueList::createSpaceSeparated();
- childList->append(ComputedStyleExtractor::valueForAnimationDuration(animation.duration()));
- childList->append(ComputedStyleExtractor::valueForAnimationTimingFunction(*animation.timingFunction()));
- childList->append(ComputedStyleExtractor::valueForAnimationDelay(animation.delay()));
- childList->append(ComputedStyleExtractor::valueForAnimationIterationCount(animation.iterationCount()));
- childList->append(ComputedStyleExtractor::valueForAnimationDirection(animation.direction()));
- childList->append(ComputedStyleExtractor::valueForAnimationFillMode(animation.fillMode()));
- childList->append(ComputedStyleExtractor::valueForAnimationPlayState(animation.playState()));
- childList->append(ComputedStyleExtractor::valueForAnimationName(animation.name()));
+ auto shorthand = shorthandForProperty(CSSPropertyAnimation);
+ for (auto longhand : shorthand)
+ ComputedStyleExtractor::addValueForAnimationPropertyToList(childList.get(), longhand, &animation);
parentList->append(childList);
}
}
@@ -3590,61 +3583,14 @@
case CSSPropertyAnimation:
return animationShorthandValue(style.animations());
case CSSPropertyAnimationDelay:
- return delayValue(style.animations());
- case CSSPropertyAnimationDirection: {
- auto list = CSSValueList::createCommaSeparated();
- const AnimationList* t = style.animations();
- if (t) {
- for (size_t i = 0; i < t->size(); ++i)
- list->append(valueForAnimationDirection(t->animation(i).direction()));
- } else
- list->append(cssValuePool.createIdentifierValue(CSSValueNormal));
- return list;
- }
+ case CSSPropertyAnimationDirection:
case CSSPropertyAnimationDuration:
- return durationValue(style.animations());
- case CSSPropertyAnimationFillMode: {
- auto list = CSSValueList::createCommaSeparated();
- const AnimationList* t = style.animations();
- if (t) {
- for (size_t i = 0; i < t->size(); ++i)
- list->append(valueForAnimationFillMode(t->animation(i).fillMode()));
- } else
- list->append(cssValuePool.createIdentifierValue(CSSValueNone));
- return list;
- }
- case CSSPropertyAnimationIterationCount: {
- auto list = CSSValueList::createCommaSeparated();
- const AnimationList* t = style.animations();
- if (t) {
- for (size_t i = 0; i < t->size(); ++i)
- list->append(valueForAnimationIterationCount(t->animation(i).iterationCount()));
- } else
- list->append(cssValuePool.createValue(Animation::initialIterationCount(), CSSUnitType::CSS_NUMBER));
- return list;
- }
- case CSSPropertyAnimationName: {
- auto list = CSSValueList::createCommaSeparated();
- const AnimationList* t = style.animations();
- if (t) {
- for (size_t i = 0; i < t->size(); ++i)
- list->append(valueForAnimationName(t->animation(i).name()));
- } else
- list->append(cssValuePool.createIdentifierValue(CSSValueNone));
- return list;
- }
- case CSSPropertyAnimationPlayState: {
- auto list = CSSValueList::createCommaSeparated();
- const AnimationList* t = style.animations();
- if (t) {
- for (size_t i = 0; i < t->size(); ++i)
- list->append(valueForAnimationPlayState(t->animation(i).playState()));
- } else
- list->append(cssValuePool.createIdentifierValue(CSSValueRunning));
- return list;
- }
+ case CSSPropertyAnimationFillMode:
+ case CSSPropertyAnimationIterationCount:
+ case CSSPropertyAnimationName:
+ case CSSPropertyAnimationPlayState:
case CSSPropertyAnimationTimingFunction:
- return timingFunctionValue(style.animations());
+ return valueListForAnimationOrTransitionProperty(propertyID, style.animations());
case CSSPropertyAppearance:
return cssValuePool.createValue(style.appearance());
case CSSPropertyAspectRatio:
@@ -3837,13 +3783,11 @@
return nullptr;
return computedRotate(renderer, style);
case CSSPropertyTransitionDelay:
- return delayValue(style.transitions());
case CSSPropertyTransitionDuration:
- return durationValue(style.transitions());
+ case CSSPropertyTransitionTimingFunction:
+ return valueListForAnimationOrTransitionProperty(propertyID, style.transitions());
case CSSPropertyTransitionProperty:
return transitionPropertyValue(style.transitions());
- case CSSPropertyTransitionTimingFunction:
- return timingFunctionValue(style.transitions());
case CSSPropertyTransition: {
if (auto* animationList = style.transitions()) {
auto transitionsList = CSSValueList::createCommaSeparated();
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h (287536 => 287537)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h 2022-01-03 12:49:34 UTC (rev 287536)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.h 2022-01-03 13:51:08 UTC (rev 287537)
@@ -78,14 +78,7 @@
static Ref<CSSFontStyleValue> fontNonKeywordStyleFromStyleValue(FontSelectionValue);
static Ref<CSSFontStyleValue> fontStyleFromStyleValue(std::optional<FontSelectionValue>, FontStyleAxis);
- static Ref<CSSPrimitiveValue> valueForAnimationDuration(double);
- static Ref<CSSValue> valueForAnimationTimingFunction(const TimingFunction&);
- static Ref<CSSPrimitiveValue> valueForAnimationDelay(double);
- static Ref<CSSPrimitiveValue> valueForAnimationIterationCount(double);
- static Ref<CSSPrimitiveValue> valueForAnimationDirection(Animation::AnimationDirection);
- static Ref<CSSPrimitiveValue> valueForAnimationFillMode(AnimationFillMode);
- static Ref<CSSPrimitiveValue> valueForAnimationPlayState(AnimationPlayState);
- static Ref<CSSPrimitiveValue> valueForAnimationName(const Animation::Name&);
+ static void addValueForAnimationPropertyToList(CSSValueList&, CSSPropertyID, const Animation*);
private:
// The styled element is either the element passed into
Modified: trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp (287536 => 287537)
--- trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp 2022-01-03 12:49:34 UTC (rev 287536)
+++ trunk/Source/WebCore/css/parser/CSSPropertyParser.cpp 2022-01-03 13:51:08 UTC (rev 287537)
@@ -1658,27 +1658,10 @@
// FIXME: This will make invalid longhands, see crbug.com/386459
for (size_t i = 0; i < longhandCount; ++i) {
if (!parsedLonghand[i]) {
- auto property = shorthand.properties()[i];
- if (property == CSSPropertyAnimationDuration)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationDuration(Animation::initialDuration()));
- else if (property == CSSPropertyAnimationTimingFunction)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationTimingFunction(CubicBezierTimingFunction::defaultTimingFunction()));
- else if (property == CSSPropertyAnimationDelay)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationDelay(Animation::initialDelay()));
- else if (property == CSSPropertyAnimationIterationCount)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationIterationCount(Animation::initialIterationCount()));
- else if (property == CSSPropertyAnimationDirection)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationDirection(Animation::initialDirection()));
- else if (property == CSSPropertyAnimationFillMode)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationFillMode(Animation::initialFillMode()));
- else if (property == CSSPropertyAnimationPlayState)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationPlayState(Animation::initialPlayState()));
- else if (property == CSSPropertyAnimationName)
- longhands[i]->append(ComputedStyleExtractor::valueForAnimationName(Animation::initialName()));
- else {
- ASSERT(shorthand.id() == CSSPropertyTransition);
+ if (shorthand.id() == CSSPropertyTransition)
longhands[i]->append(CSSValuePool::singleton().createImplicitInitialValue());
- }
+ else
+ ComputedStyleExtractor::addValueForAnimationPropertyToList(*longhands[i], shorthand.properties()[i], nullptr);
}
parsedLonghand[i] = false;
}
