Element::EnsureComputedStyle() cannot be called with dirty ancestors |
||||
Issue descriptionEnsureComputedStyle is meant to be used for creating ComputedStyle in display:none subtrees. In particular if someone calls getComputedStyle() on such elements. There are a bunch of places where we call EnsureComputedStyle without updating style and layout tree first. The consequence is that the style recalc code is confused when setting ComputedStyle on dirty elements without clearing the dirtiness ending up with different sorts of invalidation problems.
,
Oct 17
,
Oct 17
Confirmed that fixing the two blockers will make the trybots pass with a DCHECK for clean style/layout-tree in EnsureComputedStyle(). The rest of the EnsureComputedStyle() not immediately preceded by a style and layout tree update are seemingly called in safe places.
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ba22458557026ee54a1dda6a63829a299936967c commit ba22458557026ee54a1dda6a63829a299936967c Author: Rune Lillesveen <futhark@chromium.org> Date: Wed Oct 17 10:37:16 2018 Replace EnsureComputedStyle with ComputedStyleRef in input_type_view. Bug: 895894 Change-Id: I6111fc26cb21ddc3a6db5702cb382fce1e623632 Reviewed-on: https://chromium-review.googlesource.com/c/1286413 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#600340} [modify] https://crrev.com/ba22458557026ee54a1dda6a63829a299936967c/third_party/blink/renderer/core/html/forms/input_type_view.cc
,
Oct 17
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6f3782535bc1b863e88b409213f9bbbb6a1cf2f commit d6f3782535bc1b863e88b409213f9bbbb6a1cf2f Author: Rune Lillesveen <futhark@chromium.org> Date: Wed Oct 17 16:47:01 2018 Don't force ComputedStyle for dragged image. EnsureComputedStyle() is for forcing a ComputedStyle on display:none elements and should only be called when style is clean. Use GetComputedStyle() instead. I'm not even sure if GetComputedStyle() can ever be null here. Bug: 895894 Change-Id: I4e5bc12a52464ec74a9d9d5e85d4f9900ce9ef4f Reviewed-on: https://chromium-review.googlesource.com/c/1286137 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#600421} [modify] https://crrev.com/d6f3782535bc1b863e88b409213f9bbbb6a1cf2f/third_party/blink/renderer/core/page/drag_controller.cc
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/235eeefaaea0df7d9d70ca465158501beda6eae2 commit 235eeefaaea0df7d9d70ca465158501beda6eae2 Author: Rune Lillesveen <futhark@chromium.org> Date: Wed Oct 17 23:38:32 2018 Don't EnsureComputedStyle in LayoutTextControlSingleLine. We do have a LayoutObject for the spinner which means we have a ComputedStyle already. Also added some documentation to EnsureComputedStyle for <area>. Bug: 895894 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ie0ee5c75fae0f699721a30f3072eec526c50083e Reviewed-on: https://chromium-review.googlesource.com/c/1286412 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#600600} [modify] https://crrev.com/235eeefaaea0df7d9d70ca465158501beda6eae2/third_party/blink/renderer/core/layout/layout_text_control_single_line.cc [modify] https://crrev.com/235eeefaaea0df7d9d70ca465158501beda6eae2/third_party/blink/renderer/core/paint/image_painter.cc
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf0755e164aefc79cebfe2d37ce46e8d06f0f2d9 commit cf0755e164aefc79cebfe2d37ce46e8d06f0f2d9 Author: Rune Lillesveen <futhark@chromium.org> Date: Thu Oct 25 13:29:26 2018 DCHECK that tree is style clean for EnsureComputedStyle(). EnsureComputedStyle is used for forcing a ComputedStyle for elements inside display:none subtrees. We had some instances where the tree was not otherwise style-clean which messed up dirtiness handling. Those instances are now fixed and we can introduce a DCHECK to make sure this is enforced. We move style propagation to viewport until after everything is clean in Document::UpdateStyle for allow for the DCHECK when we are ensuring style for the root element and body when propagating. The reason we need to ensure the style there is because some properties propagate to the viewport even though they are display:none, like overflow. Moving the call outside of the documentElement() check means we'll have to check if documentElement() is null inside PropagateStyleToViewport. Bug: 895894 Change-Id: I95ebcf573ec3b4badd65c6df9d459d4a62c46ad8 Reviewed-on: https://chromium-review.googlesource.com/c/1285054 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Anders Ruud <andruud@chromium.org> Cr-Commit-Position: refs/heads/master@{#602685} [modify] https://crrev.com/cf0755e164aefc79cebfe2d37ce46e8d06f0f2d9/third_party/blink/renderer/core/dom/document.cc [modify] https://crrev.com/cf0755e164aefc79cebfe2d37ce46e8d06f0f2d9/third_party/blink/renderer/core/dom/element.cc [modify] https://crrev.com/cf0755e164aefc79cebfe2d37ce46e8d06f0f2d9/third_party/blink/renderer/core/layout/layout_box_model_object.cc
,
Oct 27
|
||||
►
Sign in to add a comment |
||||
Comment 1 by futhark@google.com
, Oct 16