Summary: | Split fast-rejection filter logic off SelectorChecker. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dimitri Glazkov (Google) <dglazkov> | ||||||||
Component: | New Bugs | Assignee: | Dimitri Glazkov (Google) <dglazkov> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | allan.jensen, cmarcelo, dpranke, eric, gtk-ews, gyuyoung.kim, koivisto, macpherson, menard, ojan.autocc, rakuco, webkit-ews, webkit.review.bot, xan.lopez | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 89879 | ||||||||||
Attachments: |
|
Description
Dimitri Glazkov (Google)
2012-12-21 14:46:29 PST
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. |