Closed Bug 1343948 Opened 4 years ago Closed 5 months ago

ASSERTION: conflicting overflow containers lists: '!(overflowContainers && GetPrevInFlow() && static_cast<nsContainerFrame*>(GetPrevInFlow()) ->GetPropTableFrames(ExcessOverflowContainersProperty()))'

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla79
Tracking Status
firefox54 --- wontfix
firefox79 --- fixed

People

(Reporter: mats, Assigned: TYLin)

References

Details

Attachments

(1 file)

Follow-up from bug 1343606.
Priority: -- → P3

This is one of the root cause behind the recent flex item fragmentation regressions like bug 1640028.

The reason is that we unconditionally merge overflow incomplete children to a flex / grid ExcessOverflowContainersProperty list in PushIncompleteChildren [1]. We expect the excess list to be drained by the next-in-flow. However, if the flex /grid container already has a next-in-flow that is an overflow container having an existing OverflowContainersProperty list, the assertion [2] triggers when we drain the excess overflow container list when reflowing the next-in-flow.

There's an invariant described in [3], saying that only one overflow containers list can exist, either a frame's own OverflowContainersProperty or its prev-in-flow's ExcessOverflowContainersProperty, not both. I think we should fix PushIncompleteChildren to respect that.

[1] https://searchfox.org/mozilla-central/rev/9aa7bebfd169bc2ead00ef596498a406e56bbb85/layout/generic/nsContainerFrame.cpp#1597
[2] https://searchfox.org/mozilla-central/rev/9aa7bebfd169bc2ead00ef596498a406e56bbb85/layout/generic/nsContainerFrame.cpp#1967-1970
[3] https://searchfox.org/mozilla-central/rev/9aa7bebfd169bc2ead00ef596498a406e56bbb85/layout/generic/nsContainerFrame.h#317-319

This is to prevent the assertion at the beginning of
DrainExcessOverflowContainersList().

The invariant is described in the comment revised in this patch. That
is, "only one overflow containers list exists for a given frame: either
its own OverflowContainersProperty or its prev-in-flow's
ExcessOverflowContainersProperty, not both."

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7925ab748617
Merge overflow container children to next-in-flow's OverflowContainersProperty() if the property already exists. r=mats

By looking at the logcat-emulator-5554.log (in the "Jobs Details" panel) on Android, my patch only reduce the assertion count by 1. The other three assertions are ASSERTION: overflow containers must be zero-block-size: 'finalSize.BSize(wm) == 0'.

06-02 00:34:17.791  2814  2829 I Gecko   : REFTEST TEST-LOAD | http://10.0.2.2:8854/tests/layout/base/crashtests/1343606.html | 1844 / 3809 (48%)
06-02 00:34:17.791  2814  2829 I Gecko   : 
06-02 00:34:17.791  2814  2829 I Gecko   : {"action":"log","time":1591054457791,"thread":null,"pid":null,"source":"reftest","level":"DEBUG","message":"START http://10.0.2.2:8854/tests/layout/base/crashtests/1343606.html"}
06-02 00:34:17.870  2845  2862 I Gecko   : [Child 2845, Main Thread] ###!!! ASSERTION: overflow containers must be zero-block-size: 'finalSize.BSize(wm) == 0', file /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp, line 1947
06-02 00:34:17.886  2814  2829 I Gecko   : 
06-02 00:34:17.886  2814  2829 I Gecko   : {"action":"log","time":1591054457886,"thread":null,"pid":null,"source":"reftest","level":"DEBUG","message":"[CONTENT] OnDocumentLoad triggering AfterOnLoadScripts"}
06-02 00:34:17.887  2845  2862 I Gecko   : [Child 2845, Main Thread] ###!!! ASSERTION: overflow containers must be zero-block-size: 'finalSize.BSize(wm) == 0', file /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp, line 1947
06-02 00:34:17.894  2845  2862 I Gecko   : [Child 2845, Main Thread] ###!!! ASSERTION: overflow containers must be zero-block-size: 'finalSize.BSize(wm) == 0', file /builds/worker/checkouts/gecko/layout/generic/nsBlockFrame.cpp, line 1947

I'll adjust the expectation for Android and reland my patch.

Assignee: mats → aethanyc
Status: NEW → ASSIGNED
Type: enhancement → defect
Flags: needinfo?(aethanyc)
Priority: P3 → P1
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/adac15659f36
Merge overflow container children to next-in-flow's OverflowContainersProperty() if the property already exists. r=mats
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla79
You need to log in before you can comment on or make changes to this bug.