Closed Bug 1715098 Opened 3 years ago Closed 3 years ago

Assertion failure: !prev->GetPrevContinuation() (Property should always be set on prev continuation if not the first continuation)

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
92 Branch
Tracking Flags:
Tracking Status
firefox-esr78 --- unaffected
firefox90 --- wontfix
firefox91 --- wontfix
firefox92 --- verified

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [bugmon:bisected,confirmed], [wptsync upstream])

Attachments

(3 files)

testcase.html
206 bytes, text/html
Details
testcase2.html
340 bytes, text/html
Details
Bug 1715098 - Skip overflow containers in nsSplittableFrame::CalcAndCacheConsumedBSize. r=TYLin,#layout-reviewers
48 bytes, text/x-phabricator-request
Details | Review
Attached file testcase.html —

Found while fuzzing m-c 20210408-83a21ab93aff (--enable-debug --enable-fuzzing)

Assertion failure: !prev->GetPrevContinuation() (Property should always be set on prev continuation if not the first continuation), at src/layout/generic/nsSplittableFrame.cpp:206

#0 0x7ff451935681 in nsSplittableFrame::CalcAndCacheConsumedBSize() src/layout/generic/nsSplittableFrame.cpp:204:5
#1 0x7ff4517fc0c8 in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1256:27
#2 0x7ff451834ba0 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:1001:14
#3 0x7ff451824daf in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool) src/layout/generic/nsColumnSetFrame.cpp:692:7
#4 0x7ff451826f47 in ReflowColumns src/layout/generic/nsColumnSetFrame.cpp:403:37
#5 0x7ff451826f47 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1234:37
#6 0x7ff45180c03c in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:288:11
#7 0x7ff451807a69 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3855:11
#8 0x7ff4518056b6 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3196:5
#9 0x7ff4518002e4 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2733:7
#10 0x7ff4517fc9ea in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1375:3
#11 0x7ff45180c03c in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:288:11
#12 0x7ff451807a69 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3855:11
#13 0x7ff4518056b6 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3196:5
#14 0x7ff4518002e4 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2733:7
#15 0x7ff4517fc9ea in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1375:3
#16 0x7ff451834ba0 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:1001:14
#17 0x7ff451824daf in nsColumnSetFrame::ReflowChildren(mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&, nsColumnSetFrame::ReflowConfig const&, bool) src/layout/generic/nsColumnSetFrame.cpp:692:7
#18 0x7ff451826726 in ReflowColumns src/layout/generic/nsColumnSetFrame.cpp:403:37
#19 0x7ff451826726 in nsColumnSetFrame::FindBestBalanceBSize(mozilla::ReflowInput const&, nsPresContext*, nsColumnSetFrame::ReflowConfig&, nsColumnSetFrame::ColumnBalanceData, mozilla::ReflowOutput&, bool, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1124:9
#20 0x7ff451827024 in nsColumnSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsColumnSetFrame.cpp:1241:5
#21 0x7ff45180c03c in nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:288:11
#22 0x7ff451807a69 in nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3855:11
#23 0x7ff4518056b6 in nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3196:5
#24 0x7ff4518002e4 in nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2733:7
#25 0x7ff4517fc9ea in nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1375:3
#26 0x7ff451834ba0 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:1001:14
#27 0x7ff451822c19 in nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:817:7
#28 0x7ff451834ba0 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:1001:14
#29 0x7ff45186fa73 in nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*) src/layout/generic/nsGfxScrollFrame.cpp:758:3
#30 0x7ff451870409 in nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:881:3
#31 0x7ff4518749e6 in nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1300:3
#32 0x7ff451834ff8 in nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:1041:14
#33 0x7ff4517f37f7 in mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:372:7
#34 0x7ff4516fe5a6 in mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) src/layout/base/PresShell.cpp:9597:11
#35 0x7ff4517085ae in mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9768:24
#36 0x7ff451707ab9 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4245:11
#37 0x7ff4516d0169 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1408:5
#38 0x7ff4516d0169 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2216:20
#39 0x7ff4516d7f11 in TickDriver src/layout/base/nsRefreshDriver.cpp:346:13
#40 0x7ff4516d7f11 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:324:7
#41 0x7ff4516d7df3 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:340:5
#42 0x7ff4516d7408 in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:773:5
#43 0x7ff4516d7408 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:702:16
#44 0x7ff4516d6cee in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyParentProcessVsync() src/layout/base/nsRefreshDriver.cpp:615:7
#45 0x7ff4516d6769 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) src/layout/base/nsRefreshDriver.cpp:536:9
#46 0x7ff450eebe46 in mozilla::dom::VsyncChild::RecvNotify(mozilla::VsyncEvent const&, float const&) src/dom/ipc/VsyncChild.cpp:68:15
#47 0x7ff44dc74cc0 in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:178:54
#48 0x7ff44da65d6c in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:6008:32
#49 0x7ff44d719c9e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2154:25
#50 0x7ff44d71617d in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2078:9
#51 0x7ff44d717626 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1926:3
#52 0x7ff44d71836b in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1957:13
#53 0x7ff44cdf3123 in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:470:16
#54 0x7ff44cdcd9f3 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:754:26
#55 0x7ff44cdcc944 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:609:15
#56 0x7ff44cdccad3 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:393:36
#57 0x7ff44cdf6bc6 in operator() src/xpcom/threads/TaskController.cpp:133:37
#58 0x7ff44cdf6bc6 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:534:5
#59 0x7ff44cde01c0 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1159:16
#60 0x7ff44cde6e6a in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:548:10
#61 0x7ff44d71f5d6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
#62 0x7ff44d68a323 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:335:10
#63 0x7ff44d68a23d in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
#64 0x7ff44d68a23d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
#65 0x7ff451415808 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#66 0x7ff452c8c2d3 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:906:20
#67 0x7ff44d7204bc in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:237:9
#68 0x7ff44d68a323 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:335:10
#69 0x7ff44d68a23d in RunHandler src/ipc/chromium/src/base/message_loop.cc:328:3
#70 0x7ff44d68a23d in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:310:3
#71 0x7ff452c8beaf in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:738:34
#72 0x55ca5312f0d6 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#73 0x55ca5312f0d6 in main src/browser/app/nsBrowserApp.cpp:309:18
#74 0x7ff461ccd0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
#75 0x55ca5310ce7c in _start (/home/worker/builds/m-c-20210408095111-fuzzing-debug/firefox-bin+0x14e7c)
Flags: in-testsuite?

A Pernosco session is available here: https://pernos.co/debug/Krls2U5WC4pfQibgbYd-OA/index.html

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210607214637-d3303869191c.
The bug appears to have been introduced in the following build range:

Start: 2f08ec7e57c3b7968ce11a729bb563257173b70e (20201118160535)
End: 78ec198ed4f2698265a5f4c8d40cebcd52d34eb7 (20201118142551)
Pushlog: https://hg.mozilla.org/mozilla-unified/pushloghtml?fromchange=2f08ec7e57c3b7968ce11a729bb563257173b70e&tochange=78ec198ed4f2698265a5f4c8d40cebcd52d34eb7

Whiteboard: [bugmon:bisected,confirmed]

It looks like this comes from bug 1675376 - Emilio, can you take a look?

Blocks: 1675376
Severity: -- → S3
Flags: needinfo?(emilio)

Sure.

This assert firing is harmless since we proceed to compute the right thing anyways (just slower), so not super-urgent / could be downgraded if needed.

Anyhow I took a quick look. It seems we're trying to reflow continuations out of order somehow, and that code relies on that not happening to be correct.

Ting-Yu, is it expected that multicol reflow ends up reflowing the continuations out of order? That seems extremely weird to me. In the test-case the continuation that we try to reflow out of order is the <body> block...

Assignee: nobody → emilio
Flags: needinfo?(emilio) → needinfo?(aethanyc)

:emilio, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)
No longer blocks: 1675376
Flags: needinfo?(emilio)
Regressed by: 1675376
Has Regression Range: --- → yes
Attached file testcase2.html —

Ting-Yu, is it expected that multicol reflow ends up reflowing the continuations out of order? That seems extremely weird to me. In the test-case the continuation that we try to reflow out of order is the <body> block...

It's not reflow out of order, but some of the overflow children are not reflowed at all. This is a bit hard to explain, but I'll try ...

nsColumnSetFrame has a behavior that when balancing columns, it uses all the column count allowed, but still cannot fit all its content, it will put all the overflow content into a moz-column-content wrapper in its overflow list [1] and bail out. The overflow content is expected to be reflowed in its next balancing iteration (with a larger available bsize). Or if it is a inner multicol and already consumes all its available bsize, its parent multicol can create a new (outer) column, and eventually its continuation will pull its overflow content into the new (inner) column and reflow it. But things getting trickier when a multicol contains a column-span-wrapper dividing the ColumnSet in the frame tree.

[1] https://searchfox.org/mozilla-central/rev/b79212b4fc017f27ac2435a658d4e9b9798efa86/layout/generic/nsColumnSetFrame.cpp#837-840

Attached a reduced testcase with only two levels of multicol, and still reproduce the same assertion.

When the outer multicol chooses a large bsize for the inner multicol, everything is ok, and the frame tree looks like:

ColumnSetWrapper (outer)
  ...
    ColumnSetWrapper (inner)
      ColumnSet X
        Block (-moz-column-content A)
      Block (-moz-column-span-wrapper)
      ColumnSet Y
        Block (-moz-column-content B)

However, when the outer multicol choose a smaller bsize, the inner multicol's ColumnSet X cannot fit its content at all, and it puts its overflow contents in A'. Note A' won't be reflowed until the outer multicol's next balancing iteration.

A <-> A' <-> B are in the same continuation chain. Thus, when we reflow ColumnSet Y and its column content B, the assertion triggers because we expect A' to have the ConsumedBSizeProperty().

ColumnSetWrapper (outer)
  ...
    ColumnSetWrapper (inner)
      ColumnSet X
        Block (-moz-column-content A)
        OverflowList
          Block (-moz-column-content A') <-- Not reflowed
      Block (-moz-column-span-wrapper)
      ColumnSet Y
        Block (-moz-column-content B ) <- Asserts when reflowing this

I can make the ColumnSet that has a column span sibling (such as ColumnSet X) reflow the extra -moz-column-content if it doesn't have enough available bsize, i.e. it fills its all content as if it has column-fill:auto. In this way, we can reflow every children in -moz-column-content, and the last ColumnSet (such as Y) can still report incomplete status because all the previous ColumnSet have already used up all the available bsize.

This behavior change can help artificial testcase like this bug, and won't have any impact on real sites since multi-level multicol is really rare.

Emilio, unfortunately the answer is not simple, but if it makes sense to you, I'll write a patch to fix it. I feel it's a simple fix.

Flags: needinfo?(aethanyc) → needinfo?(emilio)

I see... I think the real fix is probably not accounting for the overflow content in the consumed bsize. Do you know what the right check to detect the overflow continuation is?

Flags: needinfo?(emilio) → needinfo?(aethanyc)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

I see... I think the real fix is probably not accounting for the overflow content in the consumed bsize. Do you know what the right check to detect the overflow continuation is?

Yeah, skipping overflow container when calculating the consumed bsize sounds like a good solution to me. By definition, overflow containers are only created after we are running out of the computed bsize, and they are supposed to have zero bsize. nsIFrame::IsTrueOverflowContainer() is the method to identify the overflow container.

Note: ColumSet has a quirk behavior that it keeps the overflow -moz-column-content frames in OverflowList (such as A' in comment 6), but they do have NS_FRAME_IS_OVERFLOW_CONTAINER bit, so I believe the solution should work.

Flags: needinfo?(aethanyc)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2dd0d0b26390
Skip overflow containers in nsSplittableFrame::CalcAndCacheConsumedBSize. r=TYLin,dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29687 for changes under testing/web-platform/tests
Whiteboard: [bugmon:bisected,confirmed] → [bugmon:bisected,confirmed], [wptsync upstream]

https://hg.mozilla.org/mozilla-central/rev/2dd0d0b26390

Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox92: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
Upstream PR merged by moz-wptsync-bot

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)
status-firefox91: affected → wontfix
Flags: needinfo?(emilio)

Bugmon Analysis
Verified bug as fixed on rev mozilla-central 20210716214302-38aa248ef576.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Status: RESOLVED → VERIFIED
status-firefox92: fixed → verified
Keywords: bugmon
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: