Bug 258781
| Summary: | Store 'marker*' identifiers as AtomString | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Ahmad Saleem <ahmad.saleem792> |
| Component: | SVG | Assignee: | Nobody <webkit-unassigned> |
| Status: | NEW | ||
| Severity: | Normal | CC: | darin, rbuis, sabouhallawa, simon.fraser, webkit-bug-importer, zimmermann |
| Priority: | P2 | Keywords: | InRadar |
| Version: | WebKit Nightly Build | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=44144 | ||
Ahmad Saleem
Hi Team,
While going through Blink's commit, I came across where they moved 'markers' to AtomString:
Blink Commit: https://chromium.googlesource.com/chromium/blink/+/599ce87bff640bcb680db17ed7fff3c482742483
WebKit Source:
< SVGRenderStyle.h >
https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L82-L84 , https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L127-L129 , https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L169-L171 , https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L440 , https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L446 , https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyle.h#L452
< SVGRenderStyleDefs.h >
https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/rendering/style/SVGRenderStyleDefs.h#L276-L278
< CSSPropertyAnimation.cpp >
https://github.com/WebKit/WebKit/blob/68167894313ea917099c52a92e4b1ab03a277abf/Source/WebCore/animation/CSSPropertyAnimation.cpp#L3765-L3767
etc.
I just wanted to raise, so if there is any benefits, we can also change to AtomString but if not, we can mark this as 'RESOLVED INVALID'.
I am CCing Darin because I have seen his Webki-Dev mailgroup on String vs AtomString so I think he can give some good insight.
Thanks!
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Radar WebKit Bug Importer
<rdar://problem/111971816>
Rob Buis
This seems useful for both legacy and LBSE.
Darin Adler
I suspect that switching to AtomString here is not important, but is OK. Strings parsed from the webpage are almost certainly already in the AtomString table even if CSS doesn’t store the markers as AtomString so there’s likely no significant improvement, but it seems clear and harmless to use AtomString.