[Webkit-unassigned] [Bug 43783] Make CSS Style Selector non-recursive

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64069|review?                     |review-
               Flag|                            |




--- Comment #9 from Darin Adler <darin at apple.com>  2010-08-29 12:11:28 PST ---
(From update of attachment 64069)
This looks like a good idea. We need to measure if it makes things faster or slower. Also, can we construct a test case demonstrating that the old algorithm runs out of stack space if nested too deeply, or is that not practical?

> +class CSSStyleSelector::SelectorChecker::CallState {

If this is going to have public data members, lets have it be a struct and not use the "m_" prefix. Or make the data members private.

> +    State m_state;
> +    CSSSelector* m_sel;

Lets spell it out: selector, not sel.

> +    Element* m_e;

Lets spell it out: element, not e.

> +    CallState(State state, CSSSelector* sel, Element* e, bool isSubSelector, bool encounteredLink, RenderStyle* elementStyle, RenderStyle* elementParentStyle)
> +            : m_state(state)

Not sure why this is indented 8 spaces.

> +    void push(CallState state)
> +    {
> +        m_stack.append(state);
> +    }

Cuts down on copying if this takes a const CallState&.

> +private:
> +    Vector<CallState> m_stack;

It may improve performance to use Vector<CallState, 20> here or some other number that's usually larger than the typical depth of the stack.

Is there a way to make this patch have a smaller diff, by doing enough of this to change the indentation level separately? All those lines of code marked different that are not different make the patch too hard to read.

Normally, we do not use "const bool".

I'm going to say review- for this patch, but it looks like a step in the right direction.

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