Changeset 253017 in webkit


Ignore:
Timestamp:
Dec 2, 2019 4:38:16 PM (4 years ago)
Author:
commit-queue@webkit.org
Message:

Crash when animating an enum attribute for multiple instances of an SVG element
https://bugs.webkit.org/show_bug.cgi?id=204766

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2019-12-02
Reviewed by Simon Fraser.

Source/WebCore:

Test: svg/animations/animated-enum-lengthAdjust-instances.svg

All instances of SVG animated properties have to share a single animVal
such that once its value is progressed, all the instances will see the
change. This was not happening for SVGAnimatedDecoratedProperty. To do
that we need to:

-- Make SVGDecoratedProperty be RefCounted.
-- Change the type of SVGAnimatedDecoratedProperty::m_baseVal to

Ref<SVGDecoratedProperty<DecorationType>>.

-- Change the type of SVGAnimatedDecoratedProperty::m_animVal to

RefPtr<SVGDecoratedProperty<DecorationType>>. The master property
creates it and all the instances hold references to the same pointer.

-- Override the virtual methods instanceStartAnimation() and

instanceStopAnimation() of SVGAnimatedDecoratedProperty.

  • svg/properties/SVGAnimatedDecoratedProperty.h:

(WebCore::SVGAnimatedDecoratedProperty::create):
(WebCore::SVGAnimatedDecoratedProperty::SVGAnimatedDecoratedProperty):
(WebCore::SVGAnimatedDecoratedProperty::animVal const):
(WebCore::SVGAnimatedDecoratedProperty::currentValue const):

  • svg/properties/SVGDecoratedEnumeration.h:

(WebCore::SVGDecoratedEnumeration::create):

  • svg/properties/SVGDecoratedProperty.h:

LayoutTests:

  • svg/animations/animated-enum-lengthAdjust-instances-expected.txt: Added.
  • svg/animations/animated-enum-lengthAdjust-instances.svg: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r252998 r253017  
     12019-12-02  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Crash when animating an enum attribute for multiple instances of an SVG element
     4        https://bugs.webkit.org/show_bug.cgi?id=204766
     5
     6        Reviewed by Simon Fraser.
     7
     8        * svg/animations/animated-enum-lengthAdjust-instances-expected.txt: Added.
     9        * svg/animations/animated-enum-lengthAdjust-instances.svg: Added.
     10
    1112019-12-02  Alex Christensen  <achristensen@webkit.org>
    212
  • trunk/Source/WebCore/ChangeLog

    r253016 r253017  
     12019-12-02  Said Abou-Hallawa  <sabouhallawa@apple.com>
     2
     3        Crash when animating an enum attribute for multiple instances of an SVG element
     4        https://bugs.webkit.org/show_bug.cgi?id=204766
     5
     6        Reviewed by Simon Fraser.
     7
     8        Test: svg/animations/animated-enum-lengthAdjust-instances.svg
     9
     10        All instances of SVG animated properties have to share a single animVal
     11        such that once its value is progressed, all the instances will see the
     12        change. This was not happening for SVGAnimatedDecoratedProperty. To do
     13        that we need to:
     14
     15        -- Make SVGDecoratedProperty be RefCounted.
     16        -- Change the type of SVGAnimatedDecoratedProperty::m_baseVal to
     17           Ref<SVGDecoratedProperty<DecorationType>>.
     18        -- Change the type of SVGAnimatedDecoratedProperty::m_animVal to
     19           RefPtr<SVGDecoratedProperty<DecorationType>>. The master property
     20           creates it and all the instances hold references to the same pointer.
     21        -- Override the virtual methods instanceStartAnimation() and
     22           instanceStopAnimation() of SVGAnimatedDecoratedProperty.
     23
     24        * svg/properties/SVGAnimatedDecoratedProperty.h:
     25        (WebCore::SVGAnimatedDecoratedProperty::create):
     26        (WebCore::SVGAnimatedDecoratedProperty::SVGAnimatedDecoratedProperty):
     27        (WebCore::SVGAnimatedDecoratedProperty::animVal const):
     28        (WebCore::SVGAnimatedDecoratedProperty::currentValue const):
     29        * svg/properties/SVGDecoratedEnumeration.h:
     30        (WebCore::SVGDecoratedEnumeration::create):
     31        * svg/properties/SVGDecoratedProperty.h:
     32
    1332019-12-02  Tim Horton  <timothy_horton@apple.com>
    234
  • trunk/Source/WebCore/svg/properties/SVGAnimatedDecoratedProperty.h

    r248846 r253017  
    3737    static Ref<AnimatedProperty> create(SVGElement* contextElement)
    3838    {
    39         return adoptRef(*new AnimatedProperty(contextElement, makeUnique<DecoratedProperty<DecorationType, PropertyType>>()));
     39        return adoptRef(*new AnimatedProperty(contextElement, adoptRef(*new DecoratedProperty<DecorationType, PropertyType>())));
    4040    }
    4141
     
    4646    }
    4747
    48     SVGAnimatedDecoratedProperty(SVGElement* contextElement, std::unique_ptr<SVGDecoratedProperty<DecorationType>>&& baseVal)
     48    SVGAnimatedDecoratedProperty(SVGElement* contextElement, Ref<SVGDecoratedProperty<DecorationType>>&& baseVal)
    4949        : SVGAnimatedProperty(contextElement)
    5050        , m_baseVal(WTFMove(baseVal))
     
    8484    {
    8585        ASSERT_IMPLIES(isAnimating(), m_animVal);
    86         return static_cast<PropertyType>((isAnimating() ? m_animVal : m_baseVal)->value());
     86        return static_cast<PropertyType>((isAnimating() ? *m_animVal : m_baseVal.get()).value());
    8787    }
    8888
     
    112112    PropertyType currentValue() const
    113113    {
    114         return static_cast<PropertyType>((isAnimating() ? m_animVal : m_baseVal)->valueInternal());
     114        ASSERT_IMPLIES(isAnimating(), m_animVal);
     115        return static_cast<PropertyType>((isAnimating() ? *m_animVal : m_baseVal.get()).valueInternal());
    115116    }
    116117
     
    131132    }
    132133
     134    // Controlling the instance animation.
     135    void instanceStartAnimation(SVGAnimatedProperty& animated) override
     136    {
     137        m_animVal = static_cast<decltype(*this)>(animated).m_animVal;
     138        SVGAnimatedProperty::instanceStartAnimation(animated);
     139    }
     140
     141    void instanceStopAnimation() override
     142    {
     143        m_animVal = nullptr;
     144        SVGAnimatedProperty::instanceStopAnimation();
     145    }
     146
    133147protected:
    134     std::unique_ptr<SVGDecoratedProperty<DecorationType>> m_baseVal;
    135     std::unique_ptr<SVGDecoratedProperty<DecorationType>> m_animVal;
     148    Ref<SVGDecoratedProperty<DecorationType>> m_baseVal;
     149    RefPtr<SVGDecoratedProperty<DecorationType>> m_animVal;
    136150    SVGPropertyState m_state { SVGPropertyState::Clean };
    137151};
  • trunk/Source/WebCore/svg/properties/SVGDecoratedEnumeration.h

    r248846 r253017  
    4040    {
    4141        static_assert(std::is_integral<DecorationType>::value, "DecorationType form enum should be integral.");
    42         return makeUnique<SVGDecoratedEnumeration>(value);
     42        return adoptRef(*new SVGDecoratedEnumeration(value));
    4343    }
    4444
     
    5959    }
    6060
    61     std::unique_ptr<SVGDecoratedProperty<DecorationType>> clone() override
     61    Ref<SVGDecoratedProperty<DecorationType>> clone() override
    6262    {
    6363        return create(m_value);
  • trunk/Source/WebCore/svg/properties/SVGDecoratedProperty.h

    r248762 r253017  
    2626#pragma once
    2727
     28#include <wtf/RefCounted.h>
     29
    2830namespace WebCore {
    2931
    3032template<typename DecorationType>
    31 class SVGDecoratedProperty {
     33class SVGDecoratedProperty : public RefCounted<SVGDecoratedProperty<DecorationType>> {
    3234    WTF_MAKE_FAST_ALLOCATED;
    3335public:
     
    4951
    5052    virtual String valueAsString() const = 0;
    51     virtual std::unique_ptr<SVGDecoratedProperty<DecorationType>> clone() = 0;
     53    virtual Ref<SVGDecoratedProperty<DecorationType>> clone() = 0;
    5254};
    5355
Note: See TracChangeset for help on using the changeset viewer.