[webkit-reviews] review granted: [Bug 121899] Optimize strings copies in srcset parser : [Attachment 213362] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Romain Perier
<romain.perier at gmail.com>'s request for review:
Bug 121899: Optimize strings copies in srcset parser
https://bugs.webkit.org/show_bug.cgi?id=121899

Attachment 213362: Patch
https://bugs.webkit.org/attachment.cgi?id=213362&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=213362&action=review


> Source/WebCore/html/parser/HTMLParserIdioms.cpp:306
> +    ImageWithScale() : imageURLStart(0), imageURLLength(0), scaleFactor(1.0)
{ }

I think that the preferred style would be the same as in non-inline case, with
each initializer and each brace on its own line.

And we don't add .0 to literal values. Especially in this case, because 1.0 is
a double, but scaleFactor is a float.

    ImageWithScale()
	: imageURLStart(0)
	, imageURLLength(0)
	, scaleFactor(1)
     {
     }

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:414
>	   ImageWithScale image;
> -	   image.imageURL = srcAttribute;
> -	   image.scaleFactor = 1.0;
> -
>	   imageCandidates.append(image);

This is getting somewhat convoluted. Please rename "image" to
"srcPlaceholderImage" for a little more clarity.

> Source/WebCore/html/parser/HTMLParserIdioms.cpp:426
> +    const ImageWithScale &lastCandidate = imageCandidates.last();

The "&" should be attached to type name, 

const ImageWithScale& lastCandidate = imageCandidates.last();