Bug 195863 - Remove the SVG property tear off objects for SVGStringList
Summary: Remove the SVG property tear off objects for SVGStringList
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: 195862
Blocks: 191237 195949
  Show dependency treegraph
 
Reported: 2019-03-17 12:57 PDT by Said Abou-Hallawa
Modified: 2019-03-21 01:59 PDT (History)
8 users (show)

See Also:


Attachments
Patch (218.71 KB, patch)
2019-03-17 12:59 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch for review (65.24 KB, patch)
2019-03-17 13:03 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (218.83 KB, patch)
2019-03-17 13:09 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (219.02 KB, patch)
2019-03-17 17:48 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch for review (65.54 KB, patch)
2019-03-17 18:10 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (66.37 KB, patch)
2019-03-18 18:31 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-17 12:57:38 PDT
Currently some of SVGElements store SVGStringListValues which is an array of Strings. When the DOM requests an interface of SVGStringList a tear off object of type SVGStaticListPropertyTearOff is created to encapsulate SVGStringListValues. Like what we did for SVGAnimatedInteger and SVGAnimatedBoolean, we are going to store Ref<SVGStringList> in the SVGElement.
Comment 1 Said Abou-Hallawa 2019-03-17 12:59:26 PDT
Created attachment 364972 [details]
Patch
Comment 2 Said Abou-Hallawa 2019-03-17 13:03:20 PDT
Created attachment 364973 [details]
Patch for review
Comment 3 Said Abou-Hallawa 2019-03-17 13:09:52 PDT
Created attachment 364974 [details]
Patch
Comment 4 Said Abou-Hallawa 2019-03-17 17:48:38 PDT
Created attachment 364989 [details]
Patch
Comment 5 Said Abou-Hallawa 2019-03-17 18:10:51 PDT
Created attachment 364990 [details]
Patch for review
Comment 6 Simon Fraser (smfr) 2019-03-18 13:46:13 PDT
Comment on attachment 364990 [details]
Patch for review

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

> Source/WebCore/ChangeLog:12
> +           attribute name in SVGPropertyRegistery. It will also commits changes

it will also commit

> Source/WebCore/svg/SVGElement.h:160
> +    void commitPropertyChange(SVGProperty*) override;

Can SVGProperty be a reference?

> Source/WebCore/svg/SVGStringList.h:39
> +    static Ref<SVGStringList> create(SVGPropertyOwner* owner)

Can SVGPropertyOwner be a reference?

> Source/WebCore/svg/SVGStringList.h:59
> +        const UChar* end = ptr + data.length();

Are we sure that upconvertedCharacters() returns the same number of characters as data contains?

> Source/WebCore/svg/SVGViewElement.h:58
> +    Ref<SVGStringList> m_viewTarget { SVGStringList::create(this) };

Please initialize in the constructor.

> Source/WebCore/svg/properties/SVGList.h:2
> + * Copyright (C) 2018-2019 Apple Inc.  All rights reserved.

Just 2019

> Source/WebCore/svg/properties/SVGPrimitiveList.h:2
> + * Copyright (C) 2018-2019 Apple Inc.  All rights reserved.

2019

> Source/WebCore/svg/properties/SVGProperty.h:37
> +    void attach(SVGPropertyOwner* owner, SVGPropertyAccess access)

Can SVGPropertyOwner be a reference?
Comment 7 Said Abou-Hallawa 2019-03-18 18:31:43 PDT
Created attachment 365099 [details]
Patch
Comment 8 WebKit Commit Bot 2019-03-18 21:23:33 PDT
Comment on attachment 365099 [details]
Patch

Clearing flags on attachment: 365099

Committed r243130: <https://trac.webkit.org/changeset/243130>
Comment 9 WebKit Commit Bot 2019-03-18 21:23:35 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-03-18 21:24:17 PDT
<rdar://problem/49006248>
Comment 11 Michael Catanzaro 2019-03-19 06:46:30 PDT
Committed r243137: <https://trac.webkit.org/changeset/243137>