[css-display] Support pseudo-elements on display: contents elements.

[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
Review-Url: https://codereview.chromium.org/2789283003
Cr-Commit-Position: refs/heads/master@{#461848}
Committed: https://chromium.googlesource.com/chromium/src/+/1be2a6c872255b63b319d272a74d8b6e66529295

[emilio, 2017-04-04 18:40:29]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 18:41:33]

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

[emilio, 2017-04-04 20:20:59]

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

[emilio, 2017-04-04 20:20:59]

ptal,

This is also split from the other pseudo-elements work, since I plan to
experiment some more with different approaches for display: contents ::before
and ::after.

Thanks :)

[rune, 2017-04-04 20:32:06]

On 2017/04/04 20:20:59, emilio wrote:
> ptal,
> 
> This is also split from the other pseudo-elements work, since I plan to
> experiment some more with different approaches for display: contents ::before
> and ::after.
> 
> Thanks :)

Please shorten the description title to for instance:

[css-display] Support pseudo elements on display:contents elements.

[rune, 2017-04-04 20:32:40]

lgtm, but please add a test if clearComputedStyle is an observable change.

https://codereview.chromium.org/2789283003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2789283003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Element.cpp:2024:
elementRareData()->clearComputedStyle();
I think I've commented on this before ...

Was the missing clearComputedStyle() observable as a bug? For instance, if you
went from display:contents to display:none, did that mean you'd get out-of-date
style for getComputedStyle() on the display:none? If so, it would be good to
have a test for that.

[emilio, 2017-04-04 20:52:53]

On 2017/04/04 20:32:40, rune wrote:
>
https://codereview.chromium.org/2789283003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/dom/Element.cpp:2024:
> elementRareData()->clearComputedStyle();
> I think I've commented on this before ...

Heh, yeah: https://codereview.chromium.org/2727853002/#msg25 :)

> Was the missing clearComputedStyle() observable as a bug? For instance, if you
> went from display:contents to display:none, did that mean you'd get
out-of-date
> style for getComputedStyle() on the display:none? If so, it would be good to
> have a test for that.

It was not, because we clear it on attach/detachLayoutTree, so there's no
problem when display changes.

Thanks for the review!

[emilio, 2017-04-04 20:53:47]

Description was changed from

==========
[css-display] Support (non display: contents) pseudo-elements of display:
contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
==========

to

==========
[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
==========

[emilio, 2017-04-04 20:54:19]

Description was changed from

==========
[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
==========

to

==========
[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
==========

[emilio, 2017-04-04 21:05:30]

The CQ bit was unchecked by ecobos@igalia.com

[emilio, 2017-04-04 21:05:41]

The CQ bit was checked by ecobos@igalia.com

[commit-bot: I haz the power, 2017-04-04 21:06:14]

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, 2017-04-04 21:40:34]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491339941933030, "parent_rev":
"9e73df9aebdf6e7374791bc2d076b93ef7961eaa", "commit_rev":
"1be2a6c872255b63b319d272a74d8b6e66529295"}

[commit-bot: I haz the power, 2017-04-04 21:41:31]

Description was changed from

==========
[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748
==========

to

==========
[css-display] Support pseudo-elements on display: contents elements.

This is split from https://codereview.chromium.org/2727853002/, since display:
contents ::before and ::after still need some extra work.

BUG=657748

Review-Url: https://codereview.chromium.org/2789283003
Cr-Commit-Position: refs/heads/master@{#461848}
Committed:
https://chromium.googlesource.com/chromium/src/+/1be2a6c872255b63b319d272a74d...
==========

[commit-bot: I haz the power, 2017-04-04 21:41:32]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1be2a6c872255b63b319d272a74d...