[Webkit-unassigned] [Bug 121899] Optimize strings copies in srcset parser

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=121899





--- Comment #5 from Alexey Proskuryakov <ap at webkit.org>  2013-09-26 12:33:13 PST ---
(From update of attachment 212732)
View in context: https://bugs.webkit.org/attachment.cgi?id=212732&action=review

Yes, this looks much better to me. A few questions/comment below:

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:311
>      bool operator==(const ImageWithScale& image) const

Is the operator== actually needed? I don't immediately see where it's used, although I could have easily overlooked that.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:418
> +        image.imageURLStart = 0;
> +        image.imageURLLength = 0;

Can we add a constructor to ImageWithScale instead?

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:433
> +    return imageCandidates.last().hasNoImage() ? srcAttribute : String(srcsetAttribute.characters() + imageCandidates.last().imageURLStart, imageCandidates.last().imageURLLength);

Please use srcsetAttribute.substring() instead. That's more readable, and could help future optimizations.

Also, I'd use a local variable for imageCandidates.last().

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.