Bug 73995

Summary: StringBuilderTest.Append and StringBuilderTest.ToStringPreserveCapacity are failing.
Product: WebKit Reporter: Vsevolod Vlasov <vsevik>
Component: Web Template FrameworkAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ggaren, msaboff, oliver, rniwa, wangxianzhu
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch with fix and test re-enabling ggaren: review+

Vsevolod Vlasov
Reported 2011-12-07 04:26:54 PST
StringBuilderTest.Append StringBuilderTest.ToStringPreserveCapacity are failing on Chromium Linux since http://trac.webkit.org/changeset/102017.
Attachments
Patch with fix and test re-enabling (7.10 KB, patch)
2011-12-07 17:42 PST, Michael Saboff 		ggaren: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 3 2011-12-07 10:19:47 PST
*** Bug 73983 has been marked as a duplicate of this bug. ***
Michael Saboff
Comment 4 2011-12-07 10:27:56 PST
The problem is that when an 8 bit StringBuilder is accessed using the "characters()" method, a 16 bit copy is created. If more data is appended to the StringBuilder, the 16 bit shadow copy is not updated or invalidated.
Michael Saboff
Comment 5 2011-12-07 17:42:09 PST
Created attachment 118293 [details] Patch with fix and test re-enabling
Darin Adler
Comment 6 2011-12-07 17:45:23 PST
Comment on attachment 118293 [details] Patch with fix and test re-enabling View in context: https://bugs.webkit.org/attachment.cgi?id=118293&action=review > Source/JavaScriptCore/wtf/text/StringBuilder.h:209 > + mutable unsigned m_valid16BitShadowlen; I’d spell out length and capitalize it: m_valid16BitShadowLength > Source/JavaScriptCore/wtf/text/StringImpl.cpp:223 > if (hasTerminatingNullCharacter()) { > - len--; > - m_copyData16[len] = '\0'; > + end--; > + m_copyData16[end] = '\0'; > } Should this really go inside the helper function? Is this covered by test cases? Also, why not just up convert the 8-bit terminating null to the 16-bit terminating null.
Ryosuke Niwa
Comment 7 2011-12-07 17:45:49 PST
Comment on attachment 118293 [details] Patch with fix and test re-enabling View in context: https://bugs.webkit.org/attachment.cgi?id=118293&action=review > Source/JavaScriptCore/ChangeLog:14 > + Reviewed by NOBODY (OOPS!). This line should appear above the change description.
Ryosuke Niwa
Comment 8 2011-12-07 17:51:29 PST
Oops, sorry. It appears that I've cleared Darin's r+.
Michael Saboff
Comment 9 2011-12-07 18:20:15 PST
Committed r102298: <http://trac.webkit.org/changeset/102298>
Note You need to log in before you can comment on or make changes to this bug.