Bug 195905 - Remove the SVG tear off objects for SVGPoint, SVGPointList and SVGAnimatedPointList
Summary: Remove the SVG tear off objects for SVGPoint, SVGPointList and SVGAnimatedPoi...
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: 196065
Blocks: 191237 196084
  Show dependency treegraph
 
Reported: 2019-03-18 13:08 PDT by Said Abou-Hallawa
Modified: 2019-03-24 10:17 PDT (History)
8 users (show)

See Also:


Attachments
Patch (307.30 KB, patch)
2019-03-18 13:11 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch for review (98.18 KB, patch)
2019-03-18 13:13 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (91.30 KB, patch)
2019-03-21 01:57 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-18 13:08:51 PDT
This requires adding new classes for the SVG property and SVG property list types. It also requires new accessor, new animator and new animation function for accessing and animating the SVGAnimatedPointList. SVGPointList will not be backed by SVGPointListValues.
Comment 1 Said Abou-Hallawa 2019-03-18 13:11:31 PDT
Created attachment 365048 [details]
Patch
Comment 2 Said Abou-Hallawa 2019-03-18 13:13:12 PDT
Created attachment 365050 [details]
Patch for review
Comment 3 Jon Lee 2019-03-20 12:44:06 PDT
Is a new patch going to be uploaded?
Comment 4 Said Abou-Hallawa 2019-03-21 01:57:58 PDT
Created attachment 365520 [details]
Patch
Comment 5 Simon Fraser (smfr) 2019-03-21 13:17:22 PDT
Comment on attachment 365520 [details]
Patch

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

> Source/WebCore/svg/SVGPointList.h:82
> +            append(SVGPoint::create(FloatPoint(xPos, yPos)));

append(SVGPoint::create({ xPos, yPos }));
Comment 6 Radar WebKit Bug Importer 2019-03-21 13:42:04 PDT
<rdar://problem/49121824>
Comment 7 Said Abou-Hallawa 2019-03-21 15:22:30 PDT
Committed r243336: <https://trac.webkit.org/changeset/243336>
Comment 8 Michael Catanzaro 2019-03-24 10:11:00 PDT
It introduced a warning spam. Please remember to be careful about pessimizing move:

DerivedSources/ForwardingHeaders/wtf/StdLibExtras.h:572:64: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
  572 | #define WTFMove(value) std::move<WTF::CheckMoveParameter>(value)
      |                                                                ^
../../Source/WebCore/svg/properties/SVGPropertyList.h:120:16: note: in expansion of macro ‘WTFMove’
  120 |         return WTFMove(item);
      |                ^~~~~~~
DerivedSources/ForwardingHeaders/wtf/StdLibExtras.h:572:64: note: remove ‘std::move’ call
  572 | #define WTFMove(value) std::move<WTF::CheckMoveParameter>(value)
      |                                                                ^
../../Source/WebCore/svg/properties/SVGPropertyList.h:120:16: note: in expansion of macro ‘WTFMove’
  120 |         return WTFMove(item);
      |                ^~~~~~~
Comment 9 Michael Catanzaro 2019-03-24 10:17:09 PDT
Committed r243421: <https://trac.webkit.org/changeset/243421>