[webkit-reviews] review granted: [Bug 195863] Remove the SVG property tear off objects for SVGStringList : [Attachment 364990] Patch for review

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 195863: Remove the SVG property tear off objects for SVGStringList
https://bugs.webkit.org/show_bug.cgi?id=195863

Attachment 364990: Patch for review

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




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 364990
  --> https://bugs.webkit.org/attachment.cgi?id=364990
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?