NEW258781
Store 'marker*' identifiers as AtomString 
https://bugs.webkit.org/show_bug.cgi?id=258781
Summary Store 'marker*' identifiers as AtomString
Ahmad Saleem
Reported 2023-07-02 02:24:56 PDT
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
Comment 1 2023-07-09 02:25:16 PDT
<rdar://problem/111971816>
Rob Buis
Comment 2 2024-03-13 04:39:27 PDT
This seems useful for both legacy and LBSE.
Darin Adler
Comment 3 2026-05-11 11:56:07 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.