Bug 142703 – Use filterRootId in SelectorQuery even if CSS JIT is not enabled

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Use filterRootId in SelectorQuery even if CSS JIT is not enabled

[Yusuke Suzuki]

Created attachment 248661 [details]
Patch

[Yusuke Suzuki]

Created attachment 248662 [details]
Patch

[Benjamin Poulain]

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.

[Yusuke Suzuki]

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!

[Yusuke Suzuki]

http://trac.webkit.org/changeset/181699 Landed with tests.
Accidentally webkit-patch crashed, I've closed this issue manually.

[Alexey Proskuryakov]

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.

[Yusuke Suzuki]

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

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

Created attachment 249009 [details]
Patch

[Yusuke Suzuki]

Committed r181725: <http://trac.webkit.org/changeset/181725>