Bug 128728 – Do not attempt to synchronize attributes when no Simple Selector could match an animatable attribute

• Component: New Bugs
• Status: RESOLVED FIXED

[Benjamin Poulain]

Do not attempt to synchronize attributes when no Simple Selector could match an animatable attribute

[Benjamin Poulain]

Created attachment 224052 [details]
Patch

[Andreas Kling]

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.

[Benjamin Poulain]

(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 :)

[Andreas Kling]

(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.

[Benjamin Poulain]

Comment on attachment 224052 [details]
Patch

Clearing flags on attachment: 224052

Committed r164123: <http://trac.webkit.org/changeset/164123>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.