Implement ::shadow pseudo element and replace /shadow/ with ::shadow.

Implement ::shadow pseudo element and replace /shadow/ with ::shadow.

If an element has at least one shadow tree, the ::shadow pseudo-element matches the shadow roots themselves.

c.f. Shadow DOM styling spec:
http://drafts.csswg.org/css-scoping/#shadow-pseudoelement

The new spec doesn't support /shadow/ combinator. Instead, it supports ::shadow.

BUG=355954
TEST=fast/dom/shadow/content-pseudo-element-with-hat.html,fast/dom/shadow/querySelector-for-multiple-shadowroots.html,fast/dom/shadow/querySelector-for-useragent-shadowroot.html,fast/dom/shadow/querySelector-with-shadow-all-and-shadow-deep.html,fast/dom/shadow/style-with-hat.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170131

[tasak, 2014-03-25 10:58:55]

Would you review this CL?

[dglazkov, 2014-03-25 17:10:53]

lgtm, though the code quality isn't there yet.

https://codereview.chromium.org/210713002/diff/60001/LayoutTests/fast/css/inv...
File
LayoutTests/fast/css/invalidation/targeted-class-shadow-combinator-expected.txt
(right):

https://codereview.chromium.org/210713002/diff/60001/LayoutTests/fast/css/inv...
LayoutTests/fast/css/invalidation/targeted-class-shadow-combinator-expected.txt:1:
Check that targeted class invalidation works for ::shadow and /shadow-deep/
combinators.
I feel really bad about the syntax churn. We'll have to split up and rename all
these tests eventually. Not today though :)

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSParse...
File Source/core/css/CSSParserValues.h (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSParse...
Source/core/css/CSSParserValues.h:241: bool needsCrossingTreeScopeBoundary()
const { return isCustomPseudoElement() || pseudoType() == CSSSelector::PseudoCue
|| pseudoType() == CSSSelector::PseudoShadow; }
This function could lose "needs" and "boundary" and just be "crossesTreeScopes"

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
File Source/core/css/CSSSelector.h (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
Source/core/css/CSSSelector.h:131: ShadowPseudo, // Special case of shadow DOM
pseudo elements / shadow pseudo element
This needs to be renamed to CustomPseudoElement or something, now that we have
an actual shadow pseudo :)

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
File Source/core/css/CSSSelectorList.cpp (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
Source/core/css/CSSSelectorList.cpp:185: class
SelectorHasCombinatorCrossingTreeBoundary {
This class name (and function name below) still talks about combinators, but one
of the checks is now for pseudoelement.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
File Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
Source/core/css/SelectorChecker.cpp:296: if
(!selectorMatchesShadowRoot(nextContext.selector))
This is effectively copy-paste code from CSSSelector::Descendant, right?

Why don't we have this up at the top of SelectorChecker::checkOne, where we
check other types of pseudos?

[rune, 2014-03-25 20:57:56]

On 2014/03/25 17:10:53, dglazkov wrote:
>
https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
> File Source/core/css/CSSSelector.h (right):
> 
>
https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
> Source/core/css/CSSSelector.h:131: ShadowPseudo, // Special case of shadow DOM
> pseudo elements / shadow pseudo element
> This needs to be renamed to CustomPseudoElement or something, now that we have
> an actual shadow pseudo :)

+1, but I think the use of ShadowPseudo as a combinator for ::shadow is wrong
anyway.

[rune, 2014-03-25 20:58:21]

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
File Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
Source/core/css/SelectorChecker.cpp:159: return SelectorFailsLocally;
You're just checking for isInShadowTree(). Won't that make selectors like
"::shadow ::shadow ::shadow" match inside the same shadow dom fragment given
that the shadow tree is deep enough?

Also, according to the spec, ::shadow should just match the shadow root. So
"::shadow > div" should only match divs that are direct children of the shadow
root.

[tasak, 2014-03-26 11:01:45]

Thank you for reviewing.

So is it ok to replace all Element* in SelectorChecker with ContainerNode*?

The reason why this patch needs shadow relation and why checkOne doesn't check
::shadow is that context.element cannot store any shadow roots because of its
type, i.e. Element*.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSParse...
File Source/core/css/CSSParserValues.h (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSParse...
Source/core/css/CSSParserValues.h:241: bool needsCrossingTreeScopeBoundary()
const { return isCustomPseudoElement() || pseudoType() == CSSSelector::PseudoCue
|| pseudoType() == CSSSelector::PseudoShadow; }
On 2014/03/25 17:10:53, dglazkov wrote:
> This function could lose "needs" and "boundary" and just be
"crossesTreeScopes"

Done.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
File Source/core/css/CSSSelector.h (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
Source/core/css/CSSSelector.h:131: ShadowPseudo, // Special case of shadow DOM
pseudo elements / shadow pseudo element
On 2014/03/25 17:10:53, dglazkov wrote:
> This needs to be renamed to CustomPseudoElement or something, now that we have
> an actual shadow pseudo :)

The actual shadow pseudo uses this relation, because, for example, div::shadow >
div" is converted to "div <==ShadowPseudo== ::shadow <==Child== div".

SelectorChecker uses "ShadowPseudo" to walk up from "::shadow"(shadow root) to
"div"(shadow host), because shadow roots are not elements.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
File Source/core/css/CSSSelectorList.cpp (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/CSSSelec...
Source/core/css/CSSSelectorList.cpp:185: class
SelectorHasCombinatorCrossingTreeBoundary {
On 2014/03/25 17:10:53, dglazkov wrote:
> This class name (and function name below) still talks about combinators, but
one
> of the checks is now for pseudoelement.

Done.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
File Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
Source/core/css/SelectorChecker.cpp:159: return SelectorFailsLocally;
On 2014/03/25 20:58:21, rune - CET wrote:
> You're just checking for isInShadowTree(). Won't that make selectors like
> "::shadow ::shadow ::shadow" match inside the same shadow dom fragment given
> that the shadow tree is deep enough?

If we have "::shadow ::shadow ::shadow span", CSSParser takes the rule as if
"*::shadow *::shadow *::shadow span". (This is the same behavior as custom
pseudo elements, e.g. ::cue.)

So the selector matches only spans in a shadow tree of a shadow tree of a
shadow.

Anyway, I will add a layout test for this.

> Also, according to the spec, ::shadow should just match the shadow root. So
> "::shadow > div" should only match divs that are direct children of the shadow
> root.

Yeah. So I modified the code for Child relation in SelectorChecker.cpp, because
I would like not to enlarge the size of CSSSelector.

So I cannot add any more relations (e.g. ChildWithShadowPseudoElement or
something) / any more bits (e.g. affectedByPseudoShadowElement or something) to
CSSSelector.

And I have another reason. context.selector's type is "Element*", not
"ContainerNode*" or "Node*". We cannot store "ShadowRoot" in context.element.

https://codereview.chromium.org/210713002/diff/60001/Source/core/css/Selector...
Source/core/css/SelectorChecker.cpp:296: if
(!selectorMatchesShadowRoot(nextContext.selector))
On 2014/03/25 17:10:53, dglazkov wrote:
> This is effectively copy-paste code from CSSSelector::Descendant, right?

I see. I created a new method for this.

> Why don't we have this up at the top of SelectorChecker::checkOne, where we
> check other types of pseudos?

Because we cannot store any shadow root in context.element. Shadow roots are not
elements.

To do so, it is ok to replace all "Element*" with "ContainerNode*"?

I think, we don't have enough time to change context.element to take
"ContainerNode*" (i.e. replace context.element with context.node?). I'm afraid
of causing too many regressions.

[dglazkov, 2014-03-26 16:40:24]

lgtm. Let's not forget to keep iterating on this code.

[tasak, 2014-03-27 03:08:53]

Thank you for reviewing.

On 2014/03/26 16:40:24, dglazkov wrote:
> lgtm. Let's not forget to keep iterating on this code.

Yeah. I will start from fixing layout test filenames.

[tasak, 2014-03-27 03:08:57]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-03-27 03:09:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tasak@google.com/210713002/330001

[commit-bot: I haz the power, 2014-03-27 04:21:27]

Change committed as 170131