| Summary: | Do not attempt to synchronize attributes when no Simple Selector could match an animatable attribute | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||
| Component: | New Bugs | Assignee: | Benjamin Poulain <benjamin> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | barraclough, commit-queue, d-r, fmalita, ggaren, gyuyoung.kim, kling, pdr, schenney, sergio | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Benjamin Poulain
2014-02-13 00:42:33 PST
Created attachment 224052 [details]
Patch
Comment on attachment 224052 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224052&action=review r=me. This patch makes me wonder: Would it be better to skip special codegen for reifying the "style" attribute, and just make it drop to C++ slow path? No sane content will use a [style="foo"] selector anyway. > Source/WebCore/cssjit/SelectorCompiler.cpp:974 > + for (unsigned i = 0; i < fragment.attributes.size(); ++i) { range for! > Source/WebCore/cssjit/SelectorCompiler.cpp:976 > + const CSSSelector* attributeSelector = fragment.attributes[i]; > + const QualifiedName& selectorAttributeName = attributeSelector->attribute(); I wish SelectorFragment's would hand out const CSSSelector& instead. (In reply to comment #2) > This patch makes me wonder: Would it be better to skip special codegen for reifying the "style" attribute, and just make it drop to C++ slow path? No sane content will use a [style="foo"] selector anyway. I did not bother implementing anything fast, generateSynchronizeStyleAttribute() is just a dumb function call: http://trac.webkit.org/browser/trunk/Source/WebCore/cssjit/SelectorCompiler.cpp#L954 :) (In reply to comment #3) > (In reply to comment #2) > > This patch makes me wonder: Would it be better to skip special codegen for reifying the "style" attribute, and just make it drop to C++ slow path? No sane content will use a [style="foo"] selector anyway. > > I did not bother implementing anything fast, generateSynchronizeStyleAttribute() is just a dumb function call: http://trac.webkit.org/browser/trunk/Source/WebCore/cssjit/SelectorCompiler.cpp#L954 :) Ah, that looks cool. I was typing up a comment about how a fragment that can match both style and some SVG animatable attribute could combine the two bit checks into a single mask check, but that's just weird and not worth the complexity. Comment on attachment 224052 [details] Patch Clearing flags on attachment: 224052 Committed r164123: <http://trac.webkit.org/changeset/164123> All reviewed patches have been landed. Closing bug. |