[Webkit-unassigned] [Bug 68633] Optimize matching of common pseudo classes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68633





--- Comment #5 from Antti Koivisto <koivisto at iki.fi>  2011-09-22 13:17:56 PST ---
(In reply to comment #4)
> I’m getting increasingly annoyed that all of these functions use “get” in a way that does not match usual WebKit naming style.

I'll rename them.

> While we can use an anonymous namespace to get internal linkage, normally we just use “static” to do that instead. Or maybe that doesn’t work because of the use of these functions as template arguments?

Last time I tried static (or anything else than this) it either failed to compile or failed to inline as a template argument.

> Should we unroll the loop a little just to make the loop structure less confusing? It would make it possible to get rid of the boolean. I think that if the relation check and match checks were named inline helper functions it would be easy to unroll a bit and keep the code looking great.

Yeah, it is ugly. I'll unroll.

> Is there code we could make this share with? It seems unpleasant to have those forcePseudoState calls in multiple places.

Maybe.

> Is there a good reason the Element* is const? I think using const* for elements usually just gets in the way and makes for wordier code.

It does verify and communicate that the funcition does not modify the element. It is bit awkward as it is not followed consistently elsewhere.


> > Source/WebCore/css/SelectorChecker.cpp:1315
> > +        return element->isLink() && (m_isMatchingVisitedPseudoClass || InspectorInstrumentation::forcePseudoState(const_cast<Element*>(element), CSSSelector::PseudoVisited));
> 
> Wow, that forcePseudoState thing is strange!

It is just copy code from checkOneSelector. I assume it makes some sense. Might deserve an inline function of its own at least.

Thanks for review.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.