New issue
Advanced search Search tips

Issue 920600 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 914784



Sign in to add a comment

Ensured ComputedStyle in combination with recalc roots can cause unnecessary style recalc.

Project Member Reported by futhark@chromium.org, Jan 10

Issue description

If style recalc starts traversal from the document root, we will clear ComputedStyle in display:none subtrees as we walk down the tree to reach dirty nodes.

After we introduced style recalc roots, we may end up in a situation where we start at an element which is dirty, does not have a ComputedStyle stored, is in a display:none subtree, and the parent which is also in a display:none subtree has a non-null ComputedStyle because getComputedStyle() cached it.

In that case (see attachment), we will not clear the parent ComputedStyle because it does not take part in the traversal, and we will return a non-null for ParentComputedStyle() and start generating computed styles for its descendants (unless they are explicitly display:none).

 
comp.html
314 bytes View Download
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 14

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

commit 55321ed58a44b095732e4c59ad889923ab3f676e
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Jan 14 11:01:08 2019

Avoid style recalc in display:none subtrees.

ParentComputedStyle() should be null in display:none subtrees to avoid
computing style unnecessarily when we start at a recalc root inside a
display:none subtree where the parent of the recalc root has a
ComputedStyle because it's forced by Window.getComputedStyle().

First, we introduce a flag to mark ComputedStyle as being forced inside
a display:none subtree. This is strictly not necessary for the current
code since we can check where it is stored, but for the
unify-computed-style storage issue (914784), we will start storing
ComputedStyle in the same place for both rendered and display:none
elements, so we'll need it soon.

Let ParentComputedStyle() return null when this flag is set to make sure
display:none subtrees stay free of ComputedStyles unless enforced.

Bug:  920600 , 914784
Change-Id: Iea07af009c0237d4ac6ba155af774d2e3dece354
Reviewed-on: https://chromium-review.googlesource.com/c/1404171
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622414}
[modify] https://crrev.com/55321ed58a44b095732e4c59ad889923ab3f676e/third_party/blink/renderer/core/css/style_engine_test.cc
[modify] https://crrev.com/55321ed58a44b095732e4c59ad889923ab3f676e/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/55321ed58a44b095732e4c59ad889923ab3f676e/third_party/blink/renderer/core/dom/node_computed_style.h
[modify] https://crrev.com/55321ed58a44b095732e4c59ad889923ab3f676e/third_party/blink/renderer/core/style/computed_style_extra_fields.json5

Status: Fixed (was: Started)

Sign in to add a comment