New issue
Advanced search Search tips

Issue 897141 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
mstensho@chromium.org
kojii@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

[LayoutNG] LayoutFlowThread appear as children of NGInlineNode

Project Member Reported by kojii@chromium.org, Oct 19 
From the Morten's review comment in:
https://chromium-review.googlesource.com/c/chromium/src/+/1196724#message-1c05f23c748bd39dd52c17bf808f75ba9b856887

This is about virtual/layout_ng_experimental/fast/multicol/paged-becomes-multicol-auto-height.html, right? We somehow manage to descend into a paged container (that has block children). We shouldn't even be here. You also have a check in NGBlockLayoutAlgorithm::TryReuseFragmentsFromCache() for HasBlockFragmentation(), but we even somehow manage to sneak past that one in this case.

I wonder if you should add NOTREACHED() instead and let the test crash.

Comment 1 by mstensho@chromium.org, Oct 19

Summary: [LayoutNG] LayoutFlowThread appear as children of NGInlineNode (was: [LayoutNG] LayoutFlowThread appear as children of NGInineNode)
One thing I forgot to mention:

The layout tree structure is the same in NG and legacy when it comes to multicol / paged overflow; i.e. there's a flow thread wrapper around the actual children. But the flow thread shouldn't be seen by NG.

If you need to work on the layout object tree on the NG side, the flow thread needs to be "invisible" when traversing. There are functions in layout/ng/legacy_layout_tree_walking.h to help with this. They are already used in NGBlockNode.

Is this relevant in this case?

Comment 2 by kojii@chromium.org, Oct 19 

Yeah, I'm not sure why. On normal layout, FirstChild() skips them properly, and that CollectInlines() does not see them. I admit that I haven't really looked into why it is different for PrepareReuse, this issue is to investigate it further in near future.
►

Sign in to add a comment