Bug 121695

Summary: REGRESSION (r156140): Srcset tests are frequently crashing
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, dino, esprehn+autocc, romain.perier, yoav
Priority: P2 Keywords: MakingBotsRed, Regression
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 121609    
Bug Blocks:    
Attachments:
Description Flags
proposed fix dino: review+

Alexey Proskuryakov
Reported 2013-09-20 10:48:29 PDT
I think that URL bytes need to be copied now.
Attachments
proposed fix (8.22 KB, patch)
2013-09-20 11:08 PDT, Alexey Proskuryakov 		dino: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Romain Perier
Comment 1 2013-09-20 10:55:58 PDT
I confirm the issue, I have a patch locally, the URL assignment Must be copied using String::substring. It solves The problem. Could you assign me this bug please ? Thanks in advance
Romain Perier
Comment 2 2013-09-20 10:58:43 PDT
It should be fixed this week end, or feel free To fix it yourself if this is really important :)
Alexey Proskuryakov
Comment 3 2013-09-20 11:08:18 PDT
Created attachment 212196 [details] proposed fix
Alexey Proskuryakov
Comment 4 2013-09-20 11:13:06 PDT
Yes, it's fairly urgent. I want bots to be very very green. Also made a bunch of style fixes to make this code better align with WebKit style.
Alexey Proskuryakov
Comment 5 2013-09-20 11:30:53 PDT
Committed <http://trac.webkit.org/r156183>.
Darin Adler
Comment 6 2013-09-20 11:37:37 PDT
Comment on attachment 212196 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=212196&action=review > Source/WebCore/html/parser/HTMLParserIdioms.cpp:393 > + image.imageURL = String(srcsetAttribute.characters() + imageURLStart, imageURLEnd - imageURLStart); Might be clearer to use the substring function to do this. Even possibly leaves the door open for future optimization where we share the character backing store between the original string and the substring.
Darin Adler
Comment 7 2013-09-20 11:38:09 PDT
Comment on attachment 212196 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=212196&action=review > Source/WebCore/html/parser/HTMLParserIdioms.cpp:384 > + imageScaleFactor = charactersToFloat(srcsetAttribute.characters() + imageScaleStart, scaleFactorLengthWithoutUnit, &validScaleFactor); A shame that this forces an 8-bit string to be converted to 16-bit. Not new to this patch, but unpleasant.
Note You need to log in before you can comment on or make changes to this bug.