| Summary: | Use filterRootId in SelectorQuery even if CSS JIT is not enabled | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | allan.jensen, benjamin, commit-queue, esprehn+autocc, kangil.han, thorton | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 142826 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Yusuke Suzuki
2015-03-14 18:01:49 PDT
Created attachment 248661 [details]
Patch
Created attachment 248662 [details]
Patch
Comment on attachment 248662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248662&action=review I guess that could be useful when the JIT is disabled. > Source/WebCore/ChangeLog:16 > + - fast/selectors/filter-root-node-with-selector-contains-adjacents.html > + - fast/selectors/querySelector-id-filtering.html > + - fast/selectors/querySelector-scope-filtered-root.html If that's not already covered, it would be good to add tests for: -Failures on selectorForIdLookup()'s rootNode.inDocument() (Element.matches() on a detached element). -Failures on selectorForIdLookup()'s rootNode.document().inQuirksMode(). -Tests for CompilableSingleWithRootFilter and CompilableSingle. with cases that compiles with the CSS JIT, and cases that fail to compile. > Source/WebCore/dom/SelectorQuery.cpp:535 > + > + Two blank lines here. Comment on attachment 248662 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248662&action=review Thank you for your review! >> Source/WebCore/ChangeLog:16 >> + - fast/selectors/querySelector-scope-filtered-root.html > > If that's not already covered, it would be good to add tests for: > -Failures on selectorForIdLookup()'s rootNode.inDocument() (Element.matches() on a detached element). > -Failures on selectorForIdLookup()'s rootNode.document().inQuirksMode(). > -Tests for CompilableSingleWithRootFilter and CompilableSingle. > with cases that compiles with the CSS JIT, and cases that fail to compile. Great! I'll add them. >> Source/WebCore/dom/SelectorQuery.cpp:535 >> + > > Two blank lines here. Oops, thank you! http://trac.webkit.org/changeset/181699 Landed with tests. Accidentally webkit-patch crashed, I've closed this issue manually. This broke the build with some compilers, because it doesn't make sense to FALLTHROUGH after an ASSERT. Tim made a temporary fix in http://trac.webkit.org/changeset/181706, please fix this properly. (In reply to comment #6) > This broke the build with some compilers, because it doesn't make sense to > FALLTHROUGH after an ASSERT. > > Tim made a temporary fix in http://trac.webkit.org/changeset/181706, please > fix this properly. Thank you! I'll fix it soon. OK, I've found that, when !ENABLE(CSS_SELECTOR_JIT) and --debug, both FALLTHROUGH and ASSERT_NOT_REACHED are materialized and raise this compile error. Here, ASSERT_NOT_REACHED is correct because this CompiledSingle and CompiledSingleWithRootFilter is not set when !ENABLE(CSS_SELECTOR_JIT). So I'll fix it by dropping this FALLTHROUGH in !ENABLE(CSS_SELECTOR_JIT) build. Created attachment 249009 [details]
Patch
Committed r181725: <http://trac.webkit.org/changeset/181725> |