Bug 128728 - Do not attempt to synchronize attributes when no Simple Selector could match an animatable attribute
Summary: Do not attempt to synchronize attributes when no Simple Selector could match ...
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: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-13 00:42 PST by Benjamin Poulain
Modified: 2014-02-14 12:53 PST (History)
10 users (show)

See Also:

Attachments
Patch (23.17 KB, patch)
2014-02-13 00:50 PST, Benjamin Poulain 		no flags Details | Formatted Diff | Diff
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 Benjamin Poulain 2014-02-13 00:42:33 PST 
Do not attempt to synchronize attributes when no Simple Selector could match an animatable attribute
Comment 1 Benjamin Poulain 2014-02-13 00:50:02 PST 
Created attachment 224052 [details]
Patch
Comment 2 Andreas Kling 2014-02-14 00:24:41 PST 
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.
Comment 3 Benjamin Poulain 2014-02-14 00:30:08 PST 
(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 :)
Comment 4 Andreas Kling 2014-02-14 00:42:21 PST 
(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 5 Benjamin Poulain 2014-02-14 12:53:16 PST 
Comment on attachment 224052 [details]
Patch

Clearing flags on attachment: 224052

Committed r164123: <http://trac.webkit.org/changeset/164123>
Comment 6 Benjamin Poulain 2014-02-14 12:53:18 PST 
All reviewed patches have been landed.  Closing bug.