Bug 107460 - Move HTML Attribute case-sensitivity logic out of SelectorChecker to HTMLDocument.
Summary: Move HTML Attribute case-sensitivity logic out of SelectorChecker to HTMLDocu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 89879
  Show dependency tree / graph
 
Reported: 2013-01-21 10:19 PST by Dimitri Glazkov (Google)
Modified: 2013-01-21 21:08 PST (History)
8 users (show)

See Also:

Attachments
Patch (10.41 KB, patch)
2013-01-21 10:21 PST, Dimitri Glazkov (Google) 		no flags Details | Formatted Diff | Diff
Patch for landing (10.37 KB, patch)
2013-01-21 20:53 PST, Dimitri Glazkov (Google) 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2013-01-21 10:19:03 PST 
Move HTML Attribute case-sensitivity logic out of SelectorChecker to HTMLDocument.
Comment 1 Dimitri Glazkov (Google) 2013-01-21 10:21:08 PST 
Created attachment 183803 [details]
Patch
Comment 2 Eric Seidel (no email) 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. :-)
Comment 3 Darin Adler 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.
Comment 4 Dimitri Glazkov (Google) 2013-01-21 20:53:35 PST 
Created attachment 183880 [details]
Patch for landing
Comment 5 WebKit Review Bot 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>
Comment 6 WebKit Review Bot 2013-01-21 21:08:49 PST 
All reviewed patches have been landed.  Closing bug.