New issue
Advanced search Search tips

Issue 836126 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 836140



Sign in to add a comment

Squad: always compute style as part of style recalc for true pseudo elements

Project Member Reported by futhark@chromium.org, Apr 24 2018

Issue description

We currently have CreateAndAttachPseudoElementIfNeeded() which computes pseudo element style during layout tree attachment. This is about getting rid of this for ::before, ::after, and ::backdrop. The last user of this method is ::first-letter which might be harder to do.

 
Blocking: 836140
Project Member

Comment 2 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06211ba1d45ec48b09f0c6ae4aabd84a5ccbf4f3

commit 06211ba1d45ec48b09f0c6ae4aabd84a5ccbf4f3
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu May 31 11:43:50 2018

[Squad] Update UseFallbackContent() before descendant recalc.

UpdateFromElement() called during style recalc may trigger
RenderFallbackContent() which will update the value of
UseFallbackContent() if the loading finished synchronously.

UseFallbackContent() is used to decide if we need to recalc style for
object fallback. Starting resource loading happened in DidRecalcStyle()
which is called after descendants are recalculated, which is too late.

All elements should now get ComputedStyle as part of style recalc.

Add a DCHECK that only pseudo elements may compute style during layout
tree building.

Bug:  843520 ,  836126 
Change-Id: Iba0d8af1f79e5c33b20f6fdba1b9613ae5aadefd
Reviewed-on: https://chromium-review.googlesource.com/1078437
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563195}
[modify] https://crrev.com/06211ba1d45ec48b09f0c6ae4aabd84a5ccbf4f3/third_party/blink/renderer/core/dom/layout_tree_builder.cc
[modify] https://crrev.com/06211ba1d45ec48b09f0c6ae4aabd84a5ccbf4f3/third_party/blink/renderer/core/html/html_plugin_element.cc
[modify] https://crrev.com/06211ba1d45ec48b09f0c6ae4aabd84a5ccbf4f3/third_party/blink/renderer/core/html/html_plugin_element.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d21d004fa5157a1c4409b19e2f808001d026304

commit 2d21d004fa5157a1c4409b19e2f808001d026304
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Jun 08 17:29:53 2018

[Squad] Make pseudo element creation not rely on layout tree.

- Base layout parent computed style on traversing the DOM instead of the
  layout tree.
- Moved top layer check for ::backdrop to CanGeneratePseudoElement().
- Moved CanHaveGeneratedChildren() to layout tree building. This means
  we may generate pseudo elements which may not generate boxes for their
  pseudo elements, but the boxes will still not be created.

First-letter is still relying on layout objects. Will address that
later.

Bug:  836126 
Change-Id: I4f283a09db597bc44d1b32d1007b4aa65e83f2e0
Reviewed-on: https://chromium-review.googlesource.com/1092692
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565668}
[modify] https://crrev.com/2d21d004fa5157a1c4409b19e2f808001d026304/third_party/blink/renderer/core/css/resolver/style_resolver.cc
[modify] https://crrev.com/2d21d004fa5157a1c4409b19e2f808001d026304/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/2d21d004fa5157a1c4409b19e2f808001d026304/third_party/blink/renderer/core/dom/layout_tree_builder.cc
[modify] https://crrev.com/2d21d004fa5157a1c4409b19e2f808001d026304/third_party/blink/renderer/core/dom/pseudo_element.cc

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70e693f2345a123d2d05071b73aa20783abc8a73

commit 70e693f2345a123d2d05071b73aa20783abc8a73
Author: Rune Lillesveen <futhark@chromium.org>
Date: Tue Jun 12 12:03:47 2018

[Squad] Do not ask for cached pseudo element style.

The goal is to remove the need for a pseudo style cache for pseudo
elements which have a PseudoElement on which we store a ComputedStyle
and a LayoutObject.

This CL removes an extra call to PseudoStyle() which would access the
pseudo style cache to re-retrieve the ComputedStyle(). Instead use
GetNonAttachedStyle() which would be set by the RecalcStyle() call for
re-attachment, or to null if the PseudoElement is no longer needed.

Bug:  836126 
Change-Id: Ide807be656843fcbfad35fce7b2f3ed1c858a0f6
Reviewed-on: https://chromium-review.googlesource.com/1096761
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566384}
[modify] https://crrev.com/70e693f2345a123d2d05071b73aa20783abc8a73/third_party/blink/renderer/core/dom/element.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76a8e1fb1aa5e0a528521a1b390708f3114ff683

commit 76a8e1fb1aa5e0a528521a1b390708f3114ff683
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jun 21 14:01:35 2018

[Squad] Move parts of CanGeneratePseudoElement to ComputedStyle.

Bug:  836126 
Change-Id: Ifd1705c3d72af1c9250f97279a03f439ee08d478
Reviewed-on: https://chromium-review.googlesource.com/1109824
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569250}
[modify] https://crrev.com/76a8e1fb1aa5e0a528521a1b390708f3114ff683/third_party/blink/renderer/core/css/resolver/style_resolver.cc
[modify] https://crrev.com/76a8e1fb1aa5e0a528521a1b390708f3114ff683/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/76a8e1fb1aa5e0a528521a1b390708f3114ff683/third_party/blink/renderer/core/style/computed_style.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90057f850fb2da311459da4336efe0a728b7df69

commit 90057f850fb2da311459da4336efe0a728b7df69
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jun 21 15:11:42 2018

[Squad] Split out method for inline box style for pseudo elements.

We create an unobservable inline box for display:contents pseudo
elements storing the original style on the element. Split this code out
into a separate method as we will need it for constructing pseudo
elements as part of style recalc later.

Bug:  836126 
Change-Id: I8610ea5cd92ebbefd0428f84ca2ff4e48169b232
Reviewed-on: https://chromium-review.googlesource.com/1109827
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569264}
[modify] https://crrev.com/90057f850fb2da311459da4336efe0a728b7df69/third_party/blink/renderer/core/dom/pseudo_element.cc
[modify] https://crrev.com/90057f850fb2da311459da4336efe0a728b7df69/third_party/blink/renderer/core/dom/pseudo_element.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8

commit 5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jun 21 15:53:17 2018

[Squad] Rename cached/uncached pseudo style methods.

Be explicit about when we try to access pseudo style caches instead of
uncached. We will remove cached access for computing style for
PseudoElement and only use caching for pseudo element style where we
don't generate any elements (like ::selection) and for getComputedStyle
queries on elements which do not generate pseudo elements. E.g.
getComputedStyle(elm, "::before") when elm has no matching ::before
rules.

Bug:  836126 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I2e4436ea78b31bc72f05ada66b2955369ee93f4a
Reviewed-on: https://chromium-review.googlesource.com/1109825
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569276}
[modify] https://crrev.com/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8/third_party/blink/renderer/core/dom/element.h
[modify] https://crrev.com/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8/third_party/blink/renderer/core/dom/pseudo_element.cc
[modify] https://crrev.com/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/5bc9720b1e3d2c438ac6ab7b70d9d27c608ffaa8/third_party/blink/renderer/core/paint/selection_painting_utils.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 21 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86f6eefc70ed2aa46969754e207198bd6917ee33

commit 86f6eefc70ed2aa46969754e207198bd6917ee33
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jun 21 17:58:30 2018

[Squad] Make StyleForPseudoElement not rely on layout tree.

Traverse flat tree instead of layout tree for ancestor style. Required
to update pseudo elements as part of style recalc.

Bug:  836126 
Change-Id: I509b89ad6907f7856022fb3b28151829b6317f49
Reviewed-on: https://chromium-review.googlesource.com/1109828
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569316}
[modify] https://crrev.com/86f6eefc70ed2aa46969754e207198bd6917ee33/third_party/blink/renderer/core/dom/element.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae149e74adff6f2d532c8f99988dc12c39b7ea41

commit ae149e74adff6f2d532c8f99988dc12c39b7ea41
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Jun 22 14:55:51 2018

[Squad] Don't create PseudoElementData for setting a nullptr.

SetPseudoElement created a PseudoElementData even if the input was a
nullptr. Early out for null input to avoid unnecessary allocation.

Bug:  836126 
Change-Id: If21c9ff8db2f20427b32f124ea0b129ebbebfe80
Reviewed-on: https://chromium-review.googlesource.com/1109962
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569625}
[modify] https://crrev.com/ae149e74adff6f2d532c8f99988dc12c39b7ea41/third_party/blink/renderer/core/dom/element_rare_data.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90351b8f23ad3c9195d54e741747a85d95755987

commit 90351b8f23ad3c9195d54e741747a85d95755987
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Jun 29 22:08:22 2018

[Squad] Generate ::before/::after/::backdrop in style recalc.

We used to create these pseudo elements and their computed styles in
Element::AttachLayoutTree when building the layout tree. Now we create
or dispose these elements from style recalc, that is,
UpdatePseudoElement. To make pseudo elements live through a style recalc
with a layout tree re-attach we no longer clear the pseudo elements
during DetachLayoutTree for performing_reattach=true. We do however need
to clear the pseudo elements which do not get a layout object for the
re-attach. That is done in AttachLayoutTree for the originating element
when the originating element does not generate a layout box.

We stop using the pseudo style cache on ComputedStyle for PseudoElements
and instead return the ComputedStyle when creating the pseudo element
and store it as non-attached style which can later be retrieved when
attaching the layout object.

An effect of this change is that we can detect transitions on pseudo
elements when ancestors display types changes and causes re-attachment.
That is  issue 836140 .

::first-letter may still be generated during layout tree building, and
the ::first-letter layout structure may still be updated during style
recalc.

Bug:  836126 ,  836140 
Change-Id: Iafad705b7a7b988d4c1598e8a126ce0d79c5873d
Reviewed-on: https://chromium-review.googlesource.com/1112244
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571665}
[add] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/WebKit/LayoutTests/external/wpt/css/css-transitions/pseudo-elements-002.html
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/css/resolver/style_resolver.cc
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/css/resolver/style_resolver.h
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/dom/element.h
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/dom/layout_tree_builder.cc
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/dom/pseudo_element.cc
[modify] https://crrev.com/90351b8f23ad3c9195d54e741747a85d95755987/third_party/blink/renderer/core/style/computed_style.cc

Cc: yosin@chromium.org
Status: Fixed (was: Started)
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e3b989006dfe87d977d1a781e1f8da0520c18dc

commit 1e3b989006dfe87d977d1a781e1f8da0520c18dc
Author: Rune Lillesveen <futhark@chromium.org>
Date: Wed Aug 01 12:18:33 2018

Don't use Bloom filter for layout tree rebuild.

Layout tree rebuild and re-attachment now only matches selectors for
::first-letter. It's not worth populating the Bloom filter with
ancestor attributes just for optimizing ::first-letter matching.

Bug:  843520 ,  836126 
Change-Id: I22c3a4d6518314ded66ef9b214db53c3ff16aac2
Reviewed-on: https://chromium-review.googlesource.com/1158230
Reviewed-by: Anders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579774}
[modify] https://crrev.com/1e3b989006dfe87d977d1a781e1f8da0520c18dc/third_party/blink/renderer/core/dom/element.cc

Sign in to add a comment