Bug 137285

Summary: CSS JIT: add the initial implementation of :nth-child(An+B of selector)
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, kling, ysuzuki
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Benjamin Poulain 2014-09-30 21:05:04 PDT
CSS JIT: add the initial implementation of :nth-child(An+B of selector)
Comment 1 Benjamin Poulain 2014-09-30 21:53:37 PDT
Created attachment 238996 [details]
Patch
Comment 2 Yusuke Suzuki 2014-09-30 23:22:55 PDT
(In reply to comment #0)
> CSS JIT: add the initial implementation of :nth-child(An+B of selector)

I've landed the StackAllocator.h part in the separate patch[1] as reviewed by you[2].
So now, we can use `allocateUninitialized(int)` :)

[1]: http://trac.webkit.org/changeset/174142
[2]: https://bugs.webkit.org/show_bug.cgi?id=135293
Comment 3 Benjamin Poulain 2014-09-30 23:23:27 PDT
Comment on attachment 238996 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238996&action=review

> Source/WebCore/cssjit/SelectorCompiler.cpp:1509
> +    if (unsigned stackRequirementCount = backtrackingMemoryRequirements.stackCount) {
> +        for (unsigned i = 0; i < stackRequirementCount; ++i)
> +            m_backtrackingStack.append(m_stackAllocator.allocateUninitialized());
> +        if (!temporaryStackBase.isValid())
> +            temporaryStackBase = m_backtrackingStack.first();
>      }

This is terrible, I did not get a chance to fix it tonight.

I'll fix it before landing by using http://trac.webkit.org/changeset/174142
Comment 4 Benjamin Poulain 2014-09-30 23:24:20 PDT
(In reply to comment #2)
> (In reply to comment #0)
> > CSS JIT: add the initial implementation of :nth-child(An+B of selector)
> 
> I've landed the StackAllocator.h part in the separate patch[1] as reviewed by you[2].
> So now, we can use `allocateUninitialized(int)` :)
> 
> [1]: http://trac.webkit.org/changeset/174142
> [2]: https://bugs.webkit.org/show_bug.cgi?id=135293

Thanks! I was just adding the comment about that. :)
Comment 5 Benjamin Poulain 2014-10-01 12:29:48 PDT
Comment on attachment 238996 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238996&action=review

> Source/WebCore/cssjit/SelectorCompiler.cpp:1593
> +    // FIXME: ASSERT clean state.

Oops
Comment 6 Andreas Kling 2014-10-02 09:55:02 PDT
Comment on attachment 238996 [details]
Patch

r=me.
Comment 7 Benjamin Poulain 2014-10-02 15:12:15 PDT
Created attachment 239146 [details]
Patch
Comment 8 Benjamin Poulain 2014-10-02 16:41:11 PDT
Comment on attachment 239146 [details]
Patch

Clearing flags on attachment: 239146

Committed r174245: <http://trac.webkit.org/changeset/174245>
Comment 9 Benjamin Poulain 2014-10-02 16:41:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Darin Adler 2015-01-28 00:25:05 PST
I think this patch caused some leaks in the CSS parser because the new rules don’t all delete nth_selector_ending.
Comment 11 Benjamin Poulain 2015-01-28 01:37:57 PST
(In reply to comment #10)
> I think this patch caused some leaks in the CSS parser because the new rules
> don’t all delete nth_selector_ending.

I guess you meant to comment on https://bugs.webkit.org/show_bug.cgi?id=136845 ?

I think you are right. I'll check tomorrow.