[webkit-changes] [284602] trunk

Thu, 21 Oct 2021 05:40:44 -0700

Title: [284602] trunk
Revision
284602
Author
[email protected]
Date
2021-10-21 05:40:06 -0700 (Thu, 21 Oct 2021)

Log Message

CSSPropertyZoom needs wrapper that ensures it's always blended into a positive value.
https://bugs.webkit.org/show_bug.cgi?id=232020
<rdar://problem/84469930>

Reviewed by Antti Koivisto.

Source/WebCore:

Test: webanimations/zoom-animation-crash.html

The "zoom" CSS property is expected to be always larger than 0, so let's ensure we do not allow
values <= 0 while blending. To do so we repurpose NonNegativeFloatPropertyWrapper to now take
an argument specifying if the blended value should be non-negative or positive.

* animation/CSSPropertyAnimation.cpp:
(WebCore::FloatPropertyWrapper::FloatPropertyWrapper):
(WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
(WebCore::NonNegativeFloatPropertyWrapper::NonNegativeFloatPropertyWrapper): Deleted.

LayoutTests:

Add a test that would crash prior to the source change.

* webanimations/zoom-animation-crash-expected.txt: Added.
* webanimations/zoom-animation-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (284601 => 284602)


--- trunk/LayoutTests/ChangeLog	2021-10-21 12:28:57 UTC (rev 284601)
+++ trunk/LayoutTests/ChangeLog	2021-10-21 12:40:06 UTC (rev 284602)
@@ -1,3 +1,16 @@
+2021-10-21  Antoine Quint  <[email protected]>
+
+        CSSPropertyZoom needs wrapper that ensures it's always blended into a positive value.
+        https://bugs.webkit.org/show_bug.cgi?id=232020
+        <rdar://problem/84469930>
+
+        Reviewed by Antti Koivisto.
+
+        Add a test that would crash prior to the source change.
+
+        * webanimations/zoom-animation-crash-expected.txt: Added.
+        * webanimations/zoom-animation-crash.html: Added.
+
 2021-10-21  Enrique Ocaña González  <[email protected]>
 
         REGRESSION(r282059) [GStreamer] Test media/media-source/media-source-stalled-holds-sleep-assertion.html timeouts

Added: trunk/LayoutTests/webanimations/zoom-animation-crash-expected.txt (0 => 284602)


--- trunk/LayoutTests/webanimations/zoom-animation-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/webanimations/zoom-animation-crash-expected.txt	2021-10-21 12:40:06 UTC (rev 284602)
@@ -0,0 +1 @@
+This test passes if it does not crash.

Added: trunk/LayoutTests/webanimations/zoom-animation-crash.html (0 => 284602)


--- trunk/LayoutTests/webanimations/zoom-animation-crash.html	                        (rev 0)
+++ trunk/LayoutTests/webanimations/zoom-animation-crash.html	2021-10-21 12:40:06 UTC (rev 284602)
@@ -0,0 +1,29 @@
+<style>
+
+@keyframes anim {
+    50% { zoom: 0.25 }
+}
+
+div {
+    animation: anim 20ms cubic-bezier(0, 1000, 0, 1) forwards;
+}
+
+span {
+    line-height: 10px;
+}
+
+</style>
+<div><span>This test passes if it does not crash.</span></div>
+<script>
+
+if (window.testRunner) {
+    window.testRunner.dumpAsText();
+    window.testRunner.waitUntilDone();
+}
+
+document.querySelector("div").addEventListener("animationend", () => {
+    if (window.testRunner)
+        window.testRunner.notifyDone();
+});
+
+</script>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (284601 => 284602)


--- trunk/Source/WebCore/ChangeLog	2021-10-21 12:28:57 UTC (rev 284601)
+++ trunk/Source/WebCore/ChangeLog	2021-10-21 12:40:06 UTC (rev 284602)
@@ -1,5 +1,24 @@
 2021-10-21  Antoine Quint  <[email protected]>
 
+        CSSPropertyZoom needs wrapper that ensures it's always blended into a positive value.
+        https://bugs.webkit.org/show_bug.cgi?id=232020
+        <rdar://problem/84469930>
+
+        Reviewed by Antti Koivisto.
+
+        Test: webanimations/zoom-animation-crash.html
+
+        The "zoom" CSS property is expected to be always larger than 0, so let's ensure we do not allow
+        values <= 0 while blending. To do so we repurpose NonNegativeFloatPropertyWrapper to now take
+        an argument specifying if the blended value should be non-negative or positive.
+
+        * animation/CSSPropertyAnimation.cpp:
+        (WebCore::FloatPropertyWrapper::FloatPropertyWrapper):
+        (WebCore::CSSPropertyAnimationWrapperMap::CSSPropertyAnimationWrapperMap):
+        (WebCore::NonNegativeFloatPropertyWrapper::NonNegativeFloatPropertyWrapper): Deleted.
+
+2021-10-21  Antoine Quint  <[email protected]>
+
         Pass CompositeOperation to CSSPropertyAnimation::blendProperties and through more blending functions
         https://bugs.webkit.org/show_bug.cgi?id=232069

Modified: trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp (284601 => 284602)


--- trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp	2021-10-21 12:28:57 UTC (rev 284601)
+++ trunk/Source/WebCore/animation/CSSPropertyAnimation.cpp	2021-10-21 12:40:06 UTC (rev 284602)
@@ -2130,11 +2130,18 @@
     std::optional<T> m_minValue;
 };
 
-class NonNegativeFloatPropertyWrapper : public PropertyWrapper<float> {
+
+class FloatPropertyWrapper : public PropertyWrapper<float> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    NonNegativeFloatPropertyWrapper(CSSPropertyID property, float (RenderStyle::*getter)() const, void (RenderStyle::*setter)(float))
+    enum class ValueRange : uint8_t {
+        All,
+        NonNegative,
+        Positive
+    };
+    FloatPropertyWrapper(CSSPropertyID property, float (RenderStyle::*getter)() const, void (RenderStyle::*setter)(float), ValueRange valueRange = ValueRange::All)
         : PropertyWrapper<float>(property, getter, setter)
+        , m_valueRange(valueRange)
     {
     }
 
@@ -2142,8 +2149,15 @@
     void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const override
     {
         auto blendedValue = blendFunc(value(from), value(to), context);
-        (destination.*m_setter)(blendedValue > 0 ? blendedValue : 0);
+        if (m_valueRange == ValueRange::NonNegative && blendedValue <= 0)
+            blendedValue = 0;
+        else if (m_valueRange == ValueRange::Positive && blendedValue < 0)
+            blendedValue = std::numeric_limits<float>::epsilon();
+        (destination.*m_setter)(blendedValue);
     }
+
+private:
+    ValueRange m_valueRange;
 };
 
 class VerticalAlignWrapper final : public LengthPropertyWrapper {
@@ -2195,11 +2209,11 @@
     }
 };
 
-class PerspectiveWrapper final : public NonNegativeFloatPropertyWrapper {
+class PerspectiveWrapper final : public FloatPropertyWrapper {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     PerspectiveWrapper()
-        : NonNegativeFloatPropertyWrapper(CSSPropertyPerspective, &RenderStyle::perspective, &RenderStyle::setPerspective)
+        : FloatPropertyWrapper(CSSPropertyPerspective, &RenderStyle::perspective, &RenderStyle::setPerspective, FloatPropertyWrapper::ValueRange::NonNegative)
     {
     }
 
@@ -2208,7 +2222,7 @@
     {
         if (!from.hasPerspective() || !to.hasPerspective())
             return false;
-        return NonNegativeFloatPropertyWrapper::canInterpolate(from, to, compositeOperation);
+        return FloatPropertyWrapper::canInterpolate(from, to, compositeOperation);
     }
 
     void blend(RenderStyle& destination, const RenderStyle& from, const RenderStyle& to, const CSSPropertyBlendingContext& context) const final
@@ -2216,7 +2230,7 @@
         if (context.isDiscrete)
             (destination.*m_setter)(context.progress ? value(to) : value(from));
         else
-            NonNegativeFloatPropertyWrapper::blend(destination, from, to, context);
+            FloatPropertyWrapper::blend(destination, from, to, context);
     }
 };
 
@@ -2356,10 +2370,10 @@
 
         new PropertyWrapperFlex(),
 
-        new NonNegativeFloatPropertyWrapper(CSSPropertyBorderLeftWidth, &RenderStyle::borderLeftWidth, &RenderStyle::setBorderLeftWidth),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyBorderRightWidth, &RenderStyle::borderRightWidth, &RenderStyle::setBorderRightWidth),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyBorderTopWidth, &RenderStyle::borderTopWidth, &RenderStyle::setBorderTopWidth),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyBorderBottomWidth, &RenderStyle::borderBottomWidth, &RenderStyle::setBorderBottomWidth),
+        new FloatPropertyWrapper(CSSPropertyBorderLeftWidth, &RenderStyle::borderLeftWidth, &RenderStyle::setBorderLeftWidth, FloatPropertyWrapper::ValueRange::NonNegative),
+        new FloatPropertyWrapper(CSSPropertyBorderRightWidth, &RenderStyle::borderRightWidth, &RenderStyle::setBorderRightWidth, FloatPropertyWrapper::ValueRange::NonNegative),
+        new FloatPropertyWrapper(CSSPropertyBorderTopWidth, &RenderStyle::borderTopWidth, &RenderStyle::setBorderTopWidth, FloatPropertyWrapper::ValueRange::NonNegative),
+        new FloatPropertyWrapper(CSSPropertyBorderBottomWidth, &RenderStyle::borderBottomWidth, &RenderStyle::setBorderBottomWidth, FloatPropertyWrapper::ValueRange::NonNegative),
         new LengthPropertyWrapper(CSSPropertyMarginLeft, &RenderStyle::marginLeft, &RenderStyle::setMarginLeft),
         new LengthPropertyWrapper(CSSPropertyMarginRight, &RenderStyle::marginRight, &RenderStyle::setMarginRight),
         new LengthPropertyWrapper(CSSPropertyMarginTop, &RenderStyle::marginTop, &RenderStyle::setMarginTop),
@@ -2406,14 +2420,14 @@
         new LengthVariantPropertyWrapper<GapLength>(CSSPropertyRowGap, &RenderStyle::rowGap, &RenderStyle::setRowGap),
         new AutoPropertyWrapper<unsigned short>(CSSPropertyColumnCount, &RenderStyle::columnCount, &RenderStyle::setColumnCount, &RenderStyle::hasAutoColumnCount, &RenderStyle::setHasAutoColumnCount, 1),
         new AutoPropertyWrapper<float>(CSSPropertyColumnWidth, &RenderStyle::columnWidth, &RenderStyle::setColumnWidth, &RenderStyle::hasAutoColumnWidth, &RenderStyle::setHasAutoColumnWidth, 0),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyWebkitBorderHorizontalSpacing, &RenderStyle::horizontalBorderSpacing, &RenderStyle::setHorizontalBorderSpacing),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyWebkitBorderVerticalSpacing, &RenderStyle::verticalBorderSpacing, &RenderStyle::setVerticalBorderSpacing),
+        new FloatPropertyWrapper(CSSPropertyWebkitBorderHorizontalSpacing, &RenderStyle::horizontalBorderSpacing, &RenderStyle::setHorizontalBorderSpacing, FloatPropertyWrapper::ValueRange::NonNegative),
+        new FloatPropertyWrapper(CSSPropertyWebkitBorderVerticalSpacing, &RenderStyle::verticalBorderSpacing, &RenderStyle::setVerticalBorderSpacing, FloatPropertyWrapper::ValueRange::NonNegative),
         new AutoPropertyWrapper<int>(CSSPropertyZIndex, &RenderStyle::specifiedZIndex, &RenderStyle::setSpecifiedZIndex, &RenderStyle::hasAutoSpecifiedZIndex, &RenderStyle::setHasAutoSpecifiedZIndex),
         new PositivePropertyWrapper<unsigned short>(CSSPropertyOrphans, &RenderStyle::orphans, &RenderStyle::setOrphans),
         new PositivePropertyWrapper<unsigned short>(CSSPropertyWidows, &RenderStyle::widows, &RenderStyle::setWidows),
         new LengthPropertyWrapper(CSSPropertyLineHeight, &RenderStyle::specifiedLineHeight, &RenderStyle::setLineHeight),
         new PropertyWrapper<float>(CSSPropertyOutlineOffset, &RenderStyle::outlineOffset, &RenderStyle::setOutlineOffset),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyOutlineWidth, &RenderStyle::outlineWidth, &RenderStyle::setOutlineWidth),
+        new FloatPropertyWrapper(CSSPropertyOutlineWidth, &RenderStyle::outlineWidth, &RenderStyle::setOutlineWidth, FloatPropertyWrapper::ValueRange::NonNegative),
         new PropertyWrapper<float>(CSSPropertyLetterSpacing, &RenderStyle::letterSpacing, &RenderStyle::setLetterSpacing),
         new LengthPropertyWrapper(CSSPropertyWordSpacing, &RenderStyle::wordSpacing, &RenderStyle::setWordSpacing),
         new TextIndentWrapper,
@@ -2430,7 +2444,7 @@
         new LengthVariantPropertyWrapper<LengthSize>(CSSPropertyBorderBottomLeftRadius, &RenderStyle::borderBottomLeftRadius, &RenderStyle::setBorderBottomLeftRadius),
         new LengthVariantPropertyWrapper<LengthSize>(CSSPropertyBorderBottomRightRadius, &RenderStyle::borderBottomRightRadius, &RenderStyle::setBorderBottomRightRadius),
         new PropertyWrapper<Visibility>(CSSPropertyVisibility, &RenderStyle::visibility, &RenderStyle::setVisibility),
-        new PropertyWrapper<float>(CSSPropertyZoom, &RenderStyle::zoom, &RenderStyle::setZoomWithoutReturnValue),
+        new FloatPropertyWrapper(CSSPropertyZoom, &RenderStyle::zoom, &RenderStyle::setZoomWithoutReturnValue, FloatPropertyWrapper::ValueRange::Positive),
 
         new ClipWrapper,
 
@@ -2505,8 +2519,8 @@
         new PropertyWrapperVisitedAffectedColor(CSSPropertyTextDecorationColor, &RenderStyle::textDecorationColor, &RenderStyle::setTextDecorationColor, &RenderStyle::visitedLinkTextDecorationColor, &RenderStyle::setVisitedLinkTextDecorationColor),
 
         new LengthPropertyWrapper(CSSPropertyFlexBasis, &RenderStyle::flexBasis, &RenderStyle::setFlexBasis, { LengthPropertyWrapper::Flags::NegativeLengthsAreInvalid }),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyFlexGrow, &RenderStyle::flexGrow, &RenderStyle::setFlexGrow),
-        new NonNegativeFloatPropertyWrapper(CSSPropertyFlexShrink, &RenderStyle::flexShrink, &RenderStyle::setFlexShrink),
+        new FloatPropertyWrapper(CSSPropertyFlexGrow, &RenderStyle::flexGrow, &RenderStyle::setFlexGrow, FloatPropertyWrapper::ValueRange::NonNegative),
+        new FloatPropertyWrapper(CSSPropertyFlexShrink, &RenderStyle::flexShrink, &RenderStyle::setFlexShrink, FloatPropertyWrapper::ValueRange::NonNegative),
         new PropertyWrapper<int>(CSSPropertyOrder, &RenderStyle::order, &RenderStyle::setOrder),
 
         new TabSizePropertyWrapper,

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to