Bug 195949 – Remove the SVG property tear off objects of SVGAnimatedRect

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Said Abou-Hallawa]

SVGRect is an SVG type, so we need to introduce two classes to represent it and its animated property SVGAnimatedRect.

-- SVGValueProperty: It is derived from SVGProperty and it will be the  base class of all the SVG type based properties. It is basically a RefCounted object with m_value member. For SVGRect, m_value will be of type FloatRect.

-- SVGAnimatedValueProperty: It is derived from SVGAnimatedProperty and it will be the base class of all the animated SVG type based properties. It is RefCounted object with two RefCounted members. These two members are of type SVGValueProperty. For SVGAnimatedRect, baseVal and animVal will be of type SVGRect.

SVGAnimatedValueProperty will be responsible of:

1) Provide access to its baseVal and animVal. Note the same interface is used internally and used by the DOM.

2) Managing the animation of the property by starting and stopping it.

3) Coordinating the changes in its baseVal and animVal with the owner element.

[Said Abou-Hallawa]

Created attachment 365174 [details]
Patch

[EWS Watchlist]

Comment on attachment 365174 [details]
Patch

Attachment 365174 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/11567502

New failing tests:
svg/stroke/animated-non-scaling-stroke.html
imported/mozilla/svg/as-image/background-resize-4.html

[EWS Watchlist]

Created attachment 365185 [details]
Archive of layout-test-results from ews102 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 365174 [details]
Patch

Attachment 365174 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11568205

New failing tests:
svg/stroke/animated-non-scaling-stroke.html
imported/mozilla/svg/as-image/background-resize-4.html

[EWS Watchlist]

Created attachment 365195 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 365174 [details]
Patch

Attachment 365174 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11567948

New failing tests:
svg/stroke/animated-non-scaling-stroke.html
imported/mozilla/svg/as-image/background-resize-4.html

[EWS Watchlist]

Created attachment 365197 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Said Abou-Hallawa]

Created attachment 365199 [details]
Patch

[Said Abou-Hallawa]

Created attachment 365207 [details]
Patch for review

[Simon Fraser (smfr)]

Comment on attachment 365199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365199&action=review

> Source/WebCore/ChangeLog:22
> +        SVGAnimatedValueProperty will be responsible of:

responsible for

> Source/WebCore/svg/properties/SVGAnimatedPropertyAnimatorImpl.h:84
> +        return std::unique_ptr<SVGAnimatedRectAnimator>(new SVGAnimatedRectAnimator(attributeName, animated, animationMode, calcMode, isAccumulated, isAdditive));

std::make_unique. Seeing a 'new' in the code should ring alarm bells.

[Said Abou-Hallawa]

Created attachment 365263 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 365199 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=365199&action=review

>> Source/WebCore/ChangeLog:22
>> +        SVGAnimatedValueProperty will be responsible of:
> 
> responsible for

Fixed.

>> Source/WebCore/svg/properties/SVGAnimatedPropertyAnimatorImpl.h:84
>> +        return std::unique_ptr<SVGAnimatedRectAnimator>(new SVGAnimatedRectAnimator(attributeName, animated, animationMode, calcMode, isAccumulated, isAdditive));
> 
> std::make_unique. Seeing a 'new' in the code should ring alarm bells.

std::make_unique is now used. But I had to make the constructor of this class and its base class be public.

[WebKit Commit Bot]

Comment on attachment 365263 [details]
Patch

Clearing flags on attachment: 365263

Committed r243183: <https://trac.webkit.org/changeset/243183>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/49044304>