Bug 105660

Created attachment 180561 [details] Patch

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

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

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

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

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

Comment on attachment 180561 [details] Patch Failed to add the files. :)

Created attachment 180567 [details] Now with actual files.

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.

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.

m_selectorFilter and m_selectorChecker would be way more informative.

(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.

Created attachment 180636 [details] Patch for landing

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

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

Comment on attachment 180636 [details] Patch for landing Clearing flags on attachment: 180636 Committed r138432: <http://trac.webkit.org/changeset/138432>

All reviewed patches have been landed. Closing bug.

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