Bug 195949 - Remove the SVG property tear off objects of SVGAnimatedRect
Summary: Remove the SVG property tear off objects of SVGAnimatedRect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on: 195863
Blocks: 191237 195960
  Show dependency treegraph
 
Reported: 2019-03-19 09:45 PDT by Said Abou-Hallawa
Modified: 2019-03-19 18:00 PDT (History)
6 users (show)

See Also:


Attachments
Patch (50.34 KB, patch)
2019-03-19 09:52 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-highsierra (2.62 MB, application/zip)
2019-03-19 10:52 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (3.12 MB, application/zip)
2019-03-19 11:52 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-highsierra (2.29 MB, application/zip)
2019-03-19 11:54 PDT, Build Bot
no flags Details
Patch (52.28 KB, patch)
2019-03-19 11:56 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch for review (70.21 KB, patch)
2019-03-19 12:38 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (54.42 KB, patch)
2019-03-19 16:53 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2019-03-19 09:45:48 PDT
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.
Comment 1 Said Abou-Hallawa 2019-03-19 09:52:37 PDT
Created attachment 365174 [details]
Patch
Comment 2 Build Bot 2019-03-19 10:52:47 PDT
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
Comment 3 Build Bot 2019-03-19 10:52:49 PDT
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
Comment 4 Build Bot 2019-03-19 11:52:25 PDT
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
Comment 5 Build Bot 2019-03-19 11:52:27 PDT
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
Comment 6 Build Bot 2019-03-19 11:54:02 PDT
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
Comment 7 Build Bot 2019-03-19 11:54:04 PDT
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
Comment 8 Said Abou-Hallawa 2019-03-19 11:56:05 PDT
Created attachment 365199 [details]
Patch
Comment 9 Said Abou-Hallawa 2019-03-19 12:38:00 PDT
Created attachment 365207 [details]
Patch for review
Comment 10 Simon Fraser (smfr) 2019-03-19 15:49:36 PDT
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.
Comment 11 Said Abou-Hallawa 2019-03-19 16:53:31 PDT
Created attachment 365263 [details]
Patch
Comment 12 Said Abou-Hallawa 2019-03-19 16:55:18 PDT
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.
Comment 13 WebKit Commit Bot 2019-03-19 17:59:56 PDT
Comment on attachment 365263 [details]
Patch

Clearing flags on attachment: 365263

Committed r243183: <https://trac.webkit.org/changeset/243183>
Comment 14 WebKit Commit Bot 2019-03-19 17:59:58 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 Radar WebKit Bug Importer 2019-03-19 18:00:25 PDT
<rdar://problem/49044304>