Bug 105963

Summary: Make ClassList::reset's purpose obvious and don't keep quirks string when not needed
Product: WebKit Reporter: Elliott Sprehn <esprehn>
Component: New BugsAssignee: Elliott Sprehn <esprehn>
Status: RESOLVED FIXED    
Severity: Normal CC: arv, cmarcelo, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Elliott Sprehn
Reported 2013-01-02 15:57:43 PST
Make ClassList::reset's purpose obvious and don't leak
Attachments
Patch (5.29 KB, patch)
2013-01-02 16:06 PST, Elliott Sprehn 		no flags
DetailsFormatted DiffDiff
Patch (5.35 KB, patch)
2013-01-02 16:18 PST, Elliott Sprehn 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Elliott Sprehn
Comment 1 2013-01-02 16:06:59 PST
Created attachment 181096 [details] Patch
Elliott Sprehn
Comment 2 2013-01-02 16:15:53 PST
We retain the SpaceSplitString when moving an Element from a quirks document to a non-quirks document if you did: e = quirksDoc.createElement(...); e.classList.contains("x"); standardsDoc.appendChild(e); The SpaceSplitString for quirks is still inside the ClassList but is unused and won't be freed until the node is freed. This isn't strictly a leak since we do clean it up eventually, but it does mean the ClassList is larger than necessary (by 1 ptr).
Elliott Sprehn
Comment 3 2013-01-02 16:18:47 PST
Created attachment 181103 [details] Patch
Elliott Sprehn
Comment 4 2013-01-02 18:42:17 PST
Comment on attachment 181103 [details] Patch Dunno what's up with the mac EWS but I ran the tests it complains about again (I ran them all already) and they passed.
Erik Arvidsson
Comment 5 2013-01-02 18:51:33 PST
Comment on attachment 181103 [details] Patch LGTM too
WebKit Review Bot
Comment 6 2013-01-02 19:00:50 PST
Comment on attachment 181103 [details] Patch Clearing flags on attachment: 181103 Committed r138691: <http://trac.webkit.org/changeset/138691>
WebKit Review Bot
Comment 7 2013-01-02 19:00:54 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.