[LayoutNG] Should not read legacy size data during layout |
||
Issue descriptionLayoutNG writes size data back to legacy layout (LayoutBox::frame_rect_) when it's about to finish layout of an NGBlockNode. This means that if anyone tries to read it *during* layout, the result will typically be from the previous layout pass (or 0, if it's the first pass). Reading out size data from ancestors during layout is the normal way of doing things in the legacy engine, e.g. to resolve child sizes. If the containing block is an NG object, however, we need to prevent this. Failing to do that has been causing bugs, and is still causing a few. The main solution is to store a containing block size override on the legacy roots (e.g. tables) before entering legacy layout. The main challenge is to honor those overrides everywhere in the legacy code.
,
Nov 23
,
Nov 27
Here's a CL with a DCHECK that fails if someone tries to read out size data from an NG LayoutObject during layout. Fails a lot! Not sure if we want to fix all the issues and land this CL, or just focus on the issues that can be proven to be bugs. https://chromium-review.googlesource.com/c/chromium/src/+/1350927
,
Nov 28
Might be related:
void LayoutInline::AddOutlineRectsForContinuations(
.....
1765│ if (LayoutBoxModelObject* continuation = Continuation()) {
1766│ if (continuation->NeedsLayout()) {
1767│ // TODO(mstensho): Prevent this from happening. Before we can get th
1768│ // outlines of an object, we obviously need to have laid it out. We
1775├> DCHECK(false);
1776│ return;
I've added a DCHECK here, and I hit it while running without NG.
This will go away when we are all NG.
,
Nov 28
I don't consider that to be related. That's about objects being read *before* layout, but *during* layout. This is a problem even in pure legacy engine situations.
,
Nov 28
Here's one CL that fixes a DCHECK that would have failed with my CL in #c3 applied: https://chromium-review.googlesource.com/c/chromium/src/+/1348055 |
||
►
Sign in to add a comment |
||
Comment 1 by mstensho@chromium.org
, Nov 23