1881157 - Assertion failure: !GetPrevInFlow() (TableBCProperty should only be set on the first-in-flow!), at /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2392

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20240219-a8a8cdb0966b (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: !GetPrevInFlow() (TableBCProperty should only be set on the first-in-flow!), at /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2392

#0 0x7f98ec55ebe8 in nsTableFrame::GetOrCreateTableBCData() /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2391:3
#1 0x7f98ec551218 in nsTableFrame::SetFullBCDamageArea() /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:3733:24
#2 0x7f98ec55d44e in nsTableFrame::DidSetComputedStyle(mozilla::ComputedStyle*) /builds/worker/checkouts/gecko/layout/tables/nsTableFrame.cpp:2010:5
#3 0x7f98ec28a39f in SetComputedStyle /builds/worker/checkouts/gecko/layout/generic/nsIFrame.h:851:7
#4 0x7f98ec28a39f in mozilla::RestyleManager::ProcessPostTraversal(mozilla::dom::Element*, mozilla::ServoRestyleState&, mozilla::ServoPostTraversalFlags) /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:2978:10
#5 0x7f98ec28c802 in mozilla::RestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:3237:28
#6 0x7f98ec25f0d5 in mozilla::RestyleManager::ProcessPendingRestyles() /builds/worker/checkouts/gecko/layout/base/RestyleManager.cpp:3350:3
#7 0x7f98ec25e217 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /builds/worker/checkouts/gecko/layout/base/PresShell.cpp:4344:39
#8 0x7f98ec2226d9 in FlushPendingNotifications /builds/worker/workspace/obj-build/dist/include/mozilla/PresShell.h:1474:5
#9 0x7f98ec2226d9 in nsRefreshDriver::TickObserverArray(unsigned int, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2500:20
#10 0x7f98ec21ef08 in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsRefreshDriver::IsExtraTick) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:2734:28
#11 0x7f98ec228a41 in TickDriver /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:367:13
#12 0x7f98ec228a41 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver>>&) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:345:7
#13 0x7f98ec228940 in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:361:5
#14 0x7f98ec2287dd in mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:951:5
#15 0x7f98ec227a7c in mozilla::VsyncRefreshDriverTimer::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:861:5
#16 0x7f98ec226ce9 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread() /builds/worker/checkouts/gecko/layout/base/nsRefreshDriver.cpp:592:14
#17 0x7f98eb540c4b in mozilla::dom::VsyncMainChild::RecvNotify(mozilla::VsyncEvent const&, float const&) /builds/worker/checkouts/gecko/dom/ipc/VsyncMainChild.cpp:66:15
#18 0x7f98eb830e4d in mozilla::dom::PVsyncChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PVsyncChild.cpp:237:78
#19 0x7f98e7650d81 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PBackgroundChild.cpp:5555:32
#20 0x7f98e75e4c4f in mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1813:25
#21 0x7f98e75e19a2 in mozilla::ipc::MessageChannel::DispatchMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::UniquePtr<IPC::Message, mozilla::DefaultDelete<IPC::Message>>) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1732:9
#22 0x7f98e75e2622 in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::ActorLifecycleProxy*, mozilla::ipc::MessageChannel::MessageTask&) /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1525:3
#23 0x7f98e75e376f in mozilla::ipc::MessageChannel::MessageTask::Run() /builds/worker/checkouts/gecko/ipc/glue/MessageChannel.cpp:1623:14
#24 0x7f98e68e8bd7 in mozilla::RunnableTask::Run() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:578:16
#25 0x7f98e68de346 in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:905:26
#26 0x7f98e68dcb27 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:728:15
#27 0x7f98e68dcfa5 in mozilla::TaskController::ProcessPendingMTTask(bool) /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:514:36
#28 0x7f98e68ecb76 in operator() /builds/worker/checkouts/gecko/xpcom/threads/TaskController.cpp:232:37
#29 0x7f98e68ecb76 in mozilla::detail::RunnableFunction<mozilla::TaskController::TaskController()::$_0>::Run() /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.h:548:5
#30 0x7f98e6901ee2 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1199:16
#31 0x7f98e690902d in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#32 0x7f98e75eabb5 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:85:21
#33 0x7f98e7500f41 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#34 0x7f98e7500f41 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#35 0x7f98ebe559a8 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#36 0x7f98ebf15788 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:470:33
#37 0x7f98edd42bdb in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:721:20
#38 0x7f98e75eba96 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#39 0x7f98e7500f41 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#40 0x7f98e7500f41 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#41 0x7f98edd42442 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:656:34
#42 0x556190a5b3b6 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#43 0x556190a5b3b6 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
#44 0x7f98fc229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#45 0x7f98fc229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#46 0x556190a310e8 in _start (/home/user/workspace/browsers/m-c-20240220094730-fuzzing-debug/firefox-bin+0x590e8) (BuildId: a8beba661b4dd560fdbaaf5736dad067341ac891)

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240221043059-558067032002.
The bug appears to have been introduced in the following build range:

Start: ec7d4cb306bc811361cffc0253b35ff2385ae376 (20231027211343)
End: d3050f2e90b6c76ee08f521390b599760f95b8b5 (20231027185330)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ec7d4cb306bc811361cffc0253b35ff2385ae376&tochange=d3050f2e90b6c76ee08f521390b599760f95b8b5

Comment 2

[Dianna Smith [:diannaS]]

:tylin could you take a look?

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1881157 - Restore the old behavior that allows TableBCDataProperty to be set on table continuations. r?#layout

Bug 1861562 Part 6 [1] made a bold assumption that TableBCDataProperty should
only be set on first-in-flow, but apparently a table continuation can call
GetOrCreateTableBCData() to set the property in the testcase [2].

This patch restores the old behavior that allows TableBCDataProperty to be set
on table continuations.

[1] https://hg.mozilla.org/mozilla-central/rev/c9c310c769d2
[2] Note: we don't support fragmenting tables in multicol. Table continuations
are created in this testcase because the abspos <dialog> in the table
is fragmented.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

(In reply to Dianna Smith [:diannaS] from comment #2)

:tylin could you take a look?

Sure. Posted a patch to fix this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2960ea3e50ca
Restore the old behavior that allows TableBCDataProperty to be set on table continuations. r=dholbert

Comment 6

[Sandor Molnar[:smolnar]]

Backed out for causing crashtest assertion failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/d6fdfc208b9ae10050236b238236fc4fa57486f2

Push with failures

Failure log -> INFO - [Child 1830, Main Thread] ###!!! ASSERTION: Data loss - incomplete row needed more block-size than available, on top of page!: 'rowMetrics.BSize(wm) <= rowReflowInput.AvailableBSize(

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Re comment 6:

The assertion that 1881157.html triggered is

###!!! ASSERTION: Someone forgot a NextInFlowNeedsReflow flag: 'frameStatus.NextInFlowNeedsReflow()'

https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=R5aFG4UfRXyLFVKwuFFtNA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=03e6d6ff8bde56b645cfbc34dee452ff375ff3ca&searchStr=crashtest

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13644ef7ee35
Restore the old behavior that allows TableBCDataProperty to be set on table continuations. r=dholbert

Comment 9

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/13644ef7ee35

Comment 10

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240308044528-ac5529a784bd.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:TYLin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox124 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

This patch fixed a testcase that triggers assertions in debug build. No real user impact though. I think it is OK for it to ride the train.