Parser support for >>> (shadow-piercing descendant) combinator.

Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
Committed: https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952
Cr-Commit-Position: refs/heads/master@{#433133}

[kochi, 2016-11-14 07:17:27]

Description was changed from

==========
Parser support for >>> (deep descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed for static
profile.

BUG=633007
==========

to

==========
Parser support for >>> (deep descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed for static
profile. The parsing is guarded by a runtime enabled flag.

BUG=633007
==========

[kochi, 2016-11-14 07:43:26]

The CQ bit was checked by kochi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 07:43:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kochi, 2016-11-14 08:38:35]

kochi@chromium.org changed reviewers:
+ timloh@chromium.org

[kochi, 2016-11-14 08:39:02]

Description was changed from

==========
Parser support for >>> (deep descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed for static
profile. The parsing is guarded by a runtime enabled flag.

BUG=633007
==========

to

==========
Parser support for >>> (deep descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed for static
profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

[kochi, 2016-11-14 08:41:57]

PTAL

Tim, is it better to recognize '>>>' as a single token?

[commit-bot: I haz the power, 2016-11-14 09:14:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-14 09:14:01]

Dry run: This issue passed the CQ dry run.

[rune, 2016-11-14 10:17:19]

On 2016/11/14 08:41:57, kochi wrote:
> PTAL
> 
> Tim, is it better to recognize '>>>' as a single token?

I don't think so as '>' is a delim in CSS Syntax level 3. Like we allow ":/* 
*/hover" to be recognized as a hover pseudo class, we should probably allow
">>/*  */>" to be recognized as a deep descendant combinator.

[kochi, 2016-11-14 14:08:59]

On 2016/11/14 10:17:19, rune wrote:
> On 2016/11/14 08:41:57, kochi wrote:
> > PTAL
> > 
> > Tim, is it better to recognize '>>>' as a single token?
> 
> I don't think so as '>' is a delim in CSS Syntax level 3. Like we allow ":/* 
> */hover" to be recognized as a hover pseudo class, we should probably allow
> ">>/*  */>" to be recognized as a deep descendant combinator.

Rune, thanks for the comment! I'll work on it tomorrow.

[hayato, 2016-11-15 02:29:11]

I prefer '>>>' as a single token. '>/**/>>' should not be considered as a '>>>',
IMO.

Regarding spec, it looks unclear about how comments affect tokenizing.
https://drafts.csswg.org/css-syntax/

We might want to update the definition of <delim-token> so that ">", ">>" and
">>>" should be considered as a single token other than '<delim-token>'.

[Timothy Loh, 2016-11-15 02:45:38]

On 2016/11/15 02:29:11, hayato wrote:
> I prefer '>>>' as a single token. '>/**/>>' should not be considered as a
'>>>',
> IMO.
> 
> Regarding spec, it looks unclear about how comments affect tokenizing.
> https://drafts.csswg.org/css-syntax/
> 
> We might want to update the definition of <delim-token> so that ">", ">>" and
> ">>>" should be considered as a single token other than '<delim-token>'.

I think it's clear enough from the specs that >/**/>/**/> should be allowed, but
it seems reasonable enough to have tokens for these as we already have tokens
for $= and so on. Maybe file a spec issue. Do other browsers implement this yet?

[kochi, 2016-11-15 04:03:43]

On 2016/11/15 02:45:38, Timothy Loh wrote:
> On 2016/11/15 02:29:11, hayato wrote:
> > I prefer '>>>' as a single token. '>/**/>>' should not be considered as a
> '>>>',
> > IMO.
> > 
> > Regarding spec, it looks unclear about how comments affect tokenizing.
> > https://drafts.csswg.org/css-syntax/
> > 
> > We might want to update the definition of <delim-token> so that ">", ">>"
and
> > ">>>" should be considered as a single token other than '<delim-token>'.
> 
> I think it's clear enough from the specs that >/**/>/**/> should be allowed,
but
> it seems reasonable enough to have tokens for these as we already have tokens
> for $= and so on. Maybe file a spec issue. Do other browsers implement this
yet?

Filed an issue on csswg-drafts:
https://github.com/w3c/csswg-drafts/issues/712

It looks Safari10 implements ">>" (descendant combinator)
http://caniuse.com/#search=%3E%3E

In css-syntax spec, <delim-token> consists of all one-character delimiters,
so ">>" and ">>>" would have some different token names.

[kochi, 2016-11-15 05:44:09]

Description was changed from

==========
Parser support for >>> (deep descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed for static
profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

to

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

[kochi, 2016-11-15 06:03:24]

Description was changed from

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (aka /deep/ for Shadow DOM v0) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

to

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

[kochi, 2016-11-15 08:09:10]

Patchset #4 (id:60001) has been deleted

[kochi, 2016-11-15 08:11:01]

The CQ bit was checked by kochi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-15 08:11:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kochi, 2016-11-15 08:14:15]

As far as I tested, Safari TP supports '>>' descendant
combinator, and it allows '>/**/>'.
Therefore, let's try parsing '>>>' as 3 delimiter('>')s
as currently spec'ed, until the css-syntax issue #712
https://github.com/w3c/csswg-drafts/issues/712
resolves. 

Added a unit test for testing '>/**/>>' etc.

Tim, could you take another look?

[rune, 2016-11-15 08:19:00]

rune@opera.com changed reviewers:
+ rune@opera.com

[rune, 2016-11-15 08:19:01]

I agree it makes sense to add a new token to the spec. My comment was about
complying with the current spec.

[kochi, 2016-11-15 08:26:28]

On 2016/11/15 08:19:01, rune wrote:
> I agree it makes sense to add a new token to the spec. My comment was about
> complying with the current spec.

Thanks for the clarification.

Do you agree we shouldn't be blocked on spec change or do you
prefer landing this change until the issue#712 is resolved?

I don't know how long it would take to resolve the css issue,
and as this is the feature behind the flag, I think it is okay
to go ahead with complying to the current spec.

[rune, 2016-11-15 09:09:00]

rune@opera.com changed reviewers:
+ tabatkins@chromium.org

[rune, 2016-11-15 09:09:01]

Adding Tab.

[commit-bot: I haz the power, 2016-11-15 10:11:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-15 10:11:57]

Dry run: This issue passed the CQ dry run.

[kochi, 2016-11-17 05:10:23]

Rune, let us continue the current way (allow '>>/**/>')
so this won't be blocked on the csswg-drafts issue 712,
but I'll pursue it separately.

[rune, 2016-11-17 09:21:43]

lgtm

https://codereview.chromium.org/2500813003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
(right):

https://codereview.chromium.org/2500813003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp:298: {"div
>/**/>/**/> span", "div >>> span"}};
Could you also add a test for ">>" being invalid in the static profile?

[kochi, 2016-11-17 10:35:38]

Thanks for the review!

https://codereview.chromium.org/2500813003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
(right):

https://codereview.chromium.org/2500813003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp:298: {"div
>/**/>/**/> span", "div >>> span"}};
On 2016/11/17 09:21:43, rune wrote:
> Could you also add a test for ">>" being invalid in the static profile?

Done.

[kochi, 2016-11-17 10:36:16]

Tim, PTAL?

[Timothy Loh, 2016-11-17 14:44:15]

lgtm

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp (right):

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:613: if
(delimiter == '~')
Reads a bit weirdly, maybe drop this if.

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
(right):

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp:295: const
char* testCases[] = {"div >>>> span", "div >> span"};
Some additional tests:

div > >> span
div >> > span
div > > > span

[kochi, 2016-11-18 02:28:45]

Thanks for the review!

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp (right):

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp:613: if
(delimiter == '~')
On 2016/11/17 14:44:15, Timothy Loh wrote:
> Reads a bit weirdly, maybe drop this if.

Done.

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
(right):

https://codereview.chromium.org/2500813003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp:295: const
char* testCases[] = {"div >>>> span", "div >> span"};
On 2016/11/17 14:44:15, Timothy Loh wrote:
> Some additional tests:
> 
> div > >> span
> div >> > span
> div > > > span

Added.

[kochi, 2016-11-18 02:29:09]

The CQ bit was checked by kochi@chromium.org

[kochi, 2016-11-18 02:29:09]

The patchset sent to the CQ was uploaded after l-g-t-m from rune@opera.com,
timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/2500813003/#ps120001
(title: "Address timloh's comments.")

[commit-bot: I haz the power, 2016-11-18 02:30:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kochi, 2016-11-18 03:31:48]

kochi@chromium.org changed reviewers:
+ tkent@chromium.org

[kochi, 2016-11-18 03:32:11]

tkent-san, could you review for Source/platform?

[tkent, 2016-11-18 03:33:38]

lgtm

[commit-bot: I haz the power, 2016-11-18 04:19:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 04:19:27]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kochi, 2016-11-18 04:54:49]

The CQ bit was checked by kochi@chromium.org

[commit-bot: I haz the power, 2016-11-18 04:55:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 06:35:56]

Description was changed from

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

to

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

[commit-bot: I haz the power, 2016-11-18 06:35:57]

Committed patchset #6 (id:120001)

[commit-bot: I haz the power, 2016-11-18 06:41:28]

Description was changed from

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007
==========

to

==========
Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG=633007

Committed: https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952
Cr-Commit-Position: refs/heads/master@{#433133}
==========

[commit-bot: I haz the power, 2016-11-18 06:41:30]

Patchset 6 (id:??) landed as
https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952
Cr-Commit-Position: refs/heads/master@{#433133}