RESOLVED FIXED 105660
Split fast-rejection filter logic off SelectorChecker. 
https://bugs.webkit.org/show_bug.cgi?id=105660
Summary Split fast-rejection filter logic off SelectorChecker.
Dimitri Glazkov (Google)
Reported 2012-12-21 14:46:29 PST
Split fast-rejection filter logic off SelectorChecker.
Attachments
Patch (21.76 KB, patch)
2012-12-21 14:50 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Now with actual files. (32.49 KB, patch)
2012-12-21 15:55 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Patch for landing (32.29 KB, patch)
2012-12-23 19:59 PST, Dimitri Glazkov (Google) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dimitri Glazkov (Google)
Comment 1 2012-12-21 14:50:00 PST
Created attachment 180561 [details] Patch
Build Bot
Comment 2 2012-12-21 14:56:14 PST
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 3 2012-12-21 14:59:33 PST
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 4 2012-12-21 15:03:33 PST
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 5 2012-12-21 15:05:43 PST
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 6 2012-12-21 15:06:00 PST
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 7 2012-12-21 15:09:48 PST
Comment on attachment 180561 [details] Patch Failed to add the files. :)
Dimitri Glazkov (Google)
Comment 8 2012-12-21 15:55:54 PST
Created attachment 180567 [details] Now with actual files.
Eric Seidel (no email)
Comment 9 2012-12-21 15:59:38 PST
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 10 2012-12-21 16:27:47 PST
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
Comment 11 2012-12-21 16:29:01 PST
m_selectorFilter and m_selectorChecker would be way more informative.
Dimitri Glazkov (Google)
Comment 12 2012-12-23 19:58:21 PST
(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)
Comment 13 2012-12-23 19:59:50 PST
Created attachment 180636 [details] Patch for landing
WebKit Review Bot
Comment 14 2012-12-23 20:39:06 PST
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)
Comment 15 2012-12-23 20:50:01 PST
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 16 2012-12-23 21:06:23 PST
Comment on attachment 180636 [details] Patch for landing Clearing flags on attachment: 180636 Committed r138432: <http://trac.webkit.org/changeset/138432>
WebKit Review Bot
Comment 17 2012-12-23 21:06:30 PST
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 18 2012-12-23 22:23:46 PST
I've filed bug 105705 about the less-than-stellar reporting from the EWS.
Note You need to log in before you can comment on or make changes to this bug.