[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 #8 from Romain Perier <romain.perier at gmail.com>  2013-09-27 00:08:36 PST ---
(In reply to comment #5)
> (From update of attachment 212732 [details])
> 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.

Not sure, it was probably used for sorting ... but we use a "comp" function for this... not really clear. Removing it does not introduce regressions.
Good catch.

> 
> > Source/WebCore/html/parser/HTMLParserIdioms.cpp:418
> > +        image.imageURLStart = 0;
> > +        image.imageURLLength = 0;
> 
> Can we add a constructor to ImageWithScale instead?

sure, it's more readable and probably faster

> 
> > 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.

OK, makes sense
> 
> Also, I'd use a local variable for imageCandidates.last().

OK

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