Split fast-rejection filter logic off SelectorChecker.

• Component: New Bugs
• Status: RESOLVED FIXED

[Dimitri Glazkov (Google)]

Split fast-rejection filter logic off SelectorChecker.

[Dimitri Glazkov (Google)]

Created attachment 180561 [details]
Patch

[Build Bot]

Comment on attachment 180561 [details]
Patch

Attachment 180561 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15460466

[EFL EWS Bot]

Comment on attachment 180561 [details]
Patch

Attachment 180561 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15449585

[Early Warning System Bot]

Comment on attachment 180561 [details]
Patch

Attachment 180561 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15456567

[kov's GTK+ EWS bot]

Comment on attachment 180561 [details]
Patch

Attachment 180561 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/15445627

[Early Warning System Bot]

Comment on attachment 180561 [details]
Patch

Attachment 180561 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15451569

[Eric Seidel (no email)]

Comment on attachment 180561 [details]
Patch

Failed to add the files. :)

[Dimitri Glazkov (Google)]

Created attachment 180567 [details]
Now with actual files.

[Eric Seidel (no email)]

Comment on attachment 180567 [details]
Now with actual files.

Its a bit odd that the stack is in teh filter object, but seems OK.  Definitely better that the current location.

[Antti Koivisto]

Comment on attachment 180567 [details]
Now with actual files.

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

Looks like a good refactoring.

> Source/WebCore/ChangeLog:7
> +        The awesome Bloom filter and parent stack logic don't need to be in SelectorChecker. They nicely abstract out
> +        into their own pretty thing, named thereby SelectorFilter.

It is not exactly "abstracting out". It thankfully doesn't get any more abstract.

> Source/WebCore/css/StyleResolver.h:496
>      SelectorChecker m_checker;
> +    SelectorFilter m_filter;

m_filter does seems like overly generic name. So does m_checker, obviously, but there is no need to propagate bad naming.

[Antti Koivisto]

m_selectorFilter and m_selectorChecker would be way more informative.

[Dimitri Glazkov (Google)]

(In reply to comment #10)
> (From update of attachment 180567 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=180567&action=review
> 
> Looks like a good refactoring.
> 
> > Source/WebCore/ChangeLog:7
> > +        The awesome Bloom filter and parent stack logic don't need to be in SelectorChecker. They nicely abstract out
> > +        into their own pretty thing, named thereby SelectorFilter.
> 
> It is not exactly "abstracting out". It thankfully doesn't get any more abstract.

Changed to "factor out".

> 
> > Source/WebCore/css/StyleResolver.h:496
> >      SelectorChecker m_checker;
> > +    SelectorFilter m_filter;
> 
> m_filter does seems like overly generic name. So does m_checker, obviously, but there is no need to propagate bad naming.

Renamed to m_selectorFilter, will change m_checker in a separate patch.

[Dimitri Glazkov (Google)]

Created attachment 180636 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 180636 [details]
Patch for landing

Rejecting attachment 180636 [details] from commit-queue.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', u'--status-host=queues.webkit.org', ..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
Kit/chromium/third_party/yasm/source/patched-yasm --revision 167605 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
55>At revision 167605.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/15495309

[Eric Seidel (no email)]

I think I've made the error reporting less-awesome.

But this was the error:
Merge conflict during commit: Conflict at '/trunk/Source/WebCore/ChangeLog' at /usr/lib/git-core/git-svn line 570

[WebKit Review Bot]

Comment on attachment 180636 [details]
Patch for landing

Clearing flags on attachment: 180636

Committed r138432: <http://trac.webkit.org/changeset/138432>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

I've filed bug 105705 about the less-than-stellar reporting from the EWS.