Bug 107460

Summary: Move HTML Attribute case-sensitivity logic out of SelectorChecker to HTMLDocument.
Product: WebKit Reporter: Dimitri Glazkov (Google) <dglazkov>
Component: New BugsAssignee: Dimitri Glazkov (Google) <dglazkov>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, cmarcelo, eric, koivisto, macpherson, menard, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89879    
Attachments:
Description Flags
Patch
none
Patch for landing none

Dimitri Glazkov (Google)
Reported 2013-01-21 10:19:03 PST
Move HTML Attribute case-sensitivity logic out of SelectorChecker to HTMLDocument.
Attachments
Patch (10.41 KB, patch)
2013-01-21 10:21 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Patch for landing (10.37 KB, patch)
2013-01-21 20:53 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2013-01-21 10:21:08 PST
Created attachment 183803 [details] Patch
Eric Seidel (no email)
Comment 2 2013-01-21 11:03:38 PST
Comment on attachment 183803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183803&action=review This is no longer inline, but I doubt it matters. The more I look at style resolve perf the more I realize its the tree walk which is so expensive (that, and a few super hot functions, like cansharestylewith) > Source/WebCore/html/HTMLDocument.cpp:344 > + HashSet<AtomicStringImpl*>* attrSet = new HashSet<AtomicStringImpl*>; I don't think this needs to be atomicstringimpl anymore. That was an old limitation of our collection classes, but now we have traits for atomicstring. I also don't think it hurts to use impl either. :-)
Darin Adler
Comment 3 2013-01-21 17:11:46 PST
Comment on attachment 183803 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183803&action=review >> Source/WebCore/html/HTMLDocument.cpp:344 >> + HashSet<AtomicStringImpl*>* attrSet = new HashSet<AtomicStringImpl*>; > > I don't think this needs to be atomicstringimpl anymore. That was an old limitation of our collection classes, but now we have traits for atomicstring. I also don't think it hurts to use impl either. :-) Well, AtomicString would mean we’d ref and deref each item as it goes into the set, and even ref and deref keys as we check if they are in the map. So I still think this going one level lower to avoid all the ref and deref is worthwhile.
Dimitri Glazkov (Google)
Comment 4 2013-01-21 20:53:35 PST
Created attachment 183880 [details] Patch for landing
WebKit Review Bot
Comment 5 2013-01-21 21:08:45 PST
Comment on attachment 183880 [details] Patch for landing Clearing flags on attachment: 183880 Committed r140382: <http://trac.webkit.org/changeset/140382>
WebKit Review Bot
Comment 6 2013-01-21 21:08:49 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.