1729578 - Assertion failure: mFirstContinuation || !mPrevContinuation (mFirstContinuation unexpectedly null!), at src/layout/generic/nsTextFrame.cpp:4497

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Found while fuzzing m-c 20210828-98019dadfffe (--enable-debug --enable-fuzzing)

Assertion failure: mFirstContinuation || !mPrevContinuation (mFirstContinuation unexpectedly null!), at src/layout/generic/nsTextFrame.cpp:4497

#0 0x7f36ddfbb9f5 in nsContinuingTextFrame::FirstContinuation() const src/layout/generic/nsTextFrame.cpp:4496:5
#1 0x7f36ddfa7164 in nsContinuingTextFrame::SetPrevInFlow(nsIFrame*) src/layout/generic/nsTextFrame.cpp:4475:47
#2 0x7f36ddf90814 in nsSplittableFrame::RemoveFromFlow(nsIFrame*) src/layout/generic/nsSplittableFrame.cpp:172:25
#3 0x7f36ddfa7637 in nsContinuingTextFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsTextFrame.cpp:4598:3
#4 0x7f36ddf6f8eb in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsLineBox.cpp:387:14
#5 0x7f36dde4b75b in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsBlockFrame.cpp:450:3
#6 0x7f36ddea7709 in nsFrameList::DestroyFramesFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsFrameList.cpp:49:12
#7 0x7f36dde4bf49 in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsContainerFrame.cpp:226:11
#8 0x7f36ddf6f8eb in nsLineBox::DeleteLineList(nsPresContext*, nsLineList&, nsIFrame*, nsFrameList*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsLineBox.cpp:387:14
#9 0x7f36dde4b75b in nsBlockFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsBlockFrame.cpp:450:3
#10 0x7f36dde4bcc4 in nsContainerFrame::SafelyDestroyFrameListProp(nsIFrame*, mozilla::layout::PostFrameDestroyData&, mozilla::PresShell*, mozilla::FramePropertyDescriptor<nsFrameList> const*) src/layout/generic/nsContainerFrame.cpp:207:14
#11 0x7f36dde4c2f2 in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsContainerFrame.cpp:280:7
#12 0x7f36dde4bcc4 in nsContainerFrame::SafelyDestroyFrameListProp(nsIFrame*, mozilla::layout::PostFrameDestroyData&, mozilla::PresShell*, mozilla::FramePropertyDescriptor<nsFrameList> const*) src/layout/generic/nsContainerFrame.cpp:207:14
#13 0x7f36dde4c2f2 in nsContainerFrame::DestroyFrom(nsIFrame*, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsContainerFrame.cpp:280:7
#14 0x7f36dde69ec9 in Destroy src/layout/generic/nsIFrame.h:738:5
#15 0x7f36dde69ec9 in nsContainerFrame::DeleteNextInFlowChild(nsIFrame*, bool) src/layout/generic/nsContainerFrame.cpp:1496:16
#16 0x7f36dde6aa88 in nsBlockFrame::DeleteNextInFlowChild(nsIFrame*, bool) src/layout/generic/nsBlockFrame.cpp:6538:23
#17 0x7f36dde69de4 in nsContainerFrame::DeleteNextInFlowChild(nsIFrame*, bool) src/layout/generic/nsContainerFrame.cpp:1480:30
#18 0x7f36dde6aa88 in nsBlockFrame::DeleteNextInFlowChild(nsIFrame*, bool) src/layout/generic/nsBlockFrame.cpp:6538:23
#19 0x7f36dde69585 in nsBlockFrame::DoRemoveFrameInternal(nsIFrame*, unsigned int, mozilla::layout::PostFrameDestroyData&) src/layout/generic/nsBlockFrame.cpp:6319:45
#20 0x7f36dde67b04 in DoRemoveFrame src/layout/generic/nsBlockFrame.h:543:5
#21 0x7f36dde67b04 in nsBlockFrame::RemoveFrame(mozilla::layout::FrameChildListID, nsIFrame*) src/layout/generic/nsBlockFrame.cpp:5639:5
#22 0x7f36dddb5947 in nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, nsCSSFrameConstructor::RemoveFlags) src/layout/base/nsCSSFrameConstructor.cpp:7679:5
#23 0x7f36ddd5523b in mozilla::PresShell::ContentRemoved(nsIContent*, nsIContent*) src/layout/base/PresShell.cpp:4429:22
#24 0x7f36dae74ebe in operator() src/dom/base/MutationObservers.cpp:198:3
#25 0x7f36dae74ebe in Notify<IsRemoval::Yes, ShouldAssert::Yes, (lambda at src/dom/base/MutationObservers.cpp:198:3), (lambda at src/dom/base/MutationObservers.cpp:198:3)> src/dom/base/MutationObservers.cpp:93:5
#26 0x7f36dae74ebe in mozilla::dom::MutationObservers::NotifyContentRemoved(nsINode*, nsIContent*, nsIContent*) src/dom/base/MutationObservers.cpp:199:3
#27 0x7f36daf74790 in nsINode::RemoveChildNode(nsIContent*, bool) src/dom/base/nsINode.cpp:2108:5
#28 0x7f36daf75a40 in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) src/dom/base/nsINode.cpp:2470:18
#29 0x7f36db455b69 in InsertBefore src/dom/base/nsINode.h:1982:12
#30 0x7f36db455b69 in AppendChild src/dom/base/nsINode.h:1989:12
#31 0x7f36db455b69 in mozilla::dom::Node_Binding::appendChild(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) /builds/worker/workspace/obj-build/dom/bindings/NodeBinding.cpp:996:60
#32 0x7f36dc2a0df8 in bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) src/dom/bindings/BindingUtils.cpp:3300:13
#33 0x7f36dfa1e0d0 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) src/js/src/vm/Interpreter.cpp:401:13
#34 0x7f36dfa1d7ce in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:488:12
#35 0x7f36dfa1f1ce in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:548:10
#36 0x7f36dfa147c0 in CallFromStack src/js/src/vm/Interpreter.cpp:552:10
#37 0x7f36dfa147c0 in Interpret(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:3255:16
#38 0x7f36dfa0b3e5 in js::RunScript(JSContext*, js::RunState&) src/js/src/vm/Interpreter.cpp:370:13
#39 0x7f36dfa1d6c7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) src/js/src/vm/Interpreter.cpp:520:13
#40 0x7f36dfa1f1ce in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) src/js/src/vm/Interpreter.cpp:548:10
#41 0x7f36dfa1f3d1 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) src/js/src/vm/Interpreter.cpp:565:8
#42 0x7f36dfbff363 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) src/js/src/vm/CallAndConstruct.cpp:117:10
#43 0x7f36dbede419 in mozilla::dom::EventHandlerNonNull::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/EventHandlerBinding.cpp:283:37
#44 0x7f36dc6cc0b5 in void mozilla::dom::EventHandlerNonNull::Call<nsCOMPtr<mozilla::dom::EventTarget> >(nsCOMPtr<mozilla::dom::EventTarget> const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/EventHandlerBinding.h:365:12
#45 0x7f36dc6cb199 in mozilla::JSEventHandler::HandleEvent(mozilla::dom::Event*) src/dom/events/JSEventHandler.cpp:201:12
#46 0x7f36dc6ac76b in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*) src/dom/events/EventListenerManager.cpp:1121:22
#47 0x7f36dc6ad482 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool) src/dom/events/EventListenerManager.cpp:1312:17
#48 0x7f36dc6a22c5 in HandleEvent src/dom/events/EventListenerManager.h:394:5
#49 0x7f36dc6a22c5 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:348:17
#50 0x7f36dc6a17df in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) src/dom/events/EventDispatcher.cpp:550:16
#51 0x7f36dc6a4404 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) src/dom/events/EventDispatcher.cpp:1082:11
#52 0x7f36dddc6a93 in nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:1087:7
#53 0x7f36df25be84 in nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:6284:20
#54 0x7f36df25b97f in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:5674:7
#55 0x7f36df25c81f in non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
#56 0x7f36da4cdf9c in nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1376:3
#57 0x7f36da4cd51a in nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:974:14
#58 0x7f36da4cb897 in nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) src/uriloader/base/nsDocLoader.cpp:793:9
#59 0x7f36da4ccacf in nsDocLoader::OnStopRequest(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:676:5
#60 0x7f36df27c9c8 in nsDocShell::OnStopRequest(nsIRequest*, nsresult) src/docshell/base/nsDocShell.cpp:13468:23
#61 0x7f36d92ca1fa in mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) src/netwerk/base/nsLoadGroup.cpp:614:22
#62 0x7f36d92cb783 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:518:10
#63 0x7f36dade981d in mozilla::dom::Document::DoUnblockOnload() src/dom/base/Document.cpp:11450:18
#64 0x7f36dadc6270 in mozilla::dom::Document::UnblockOnload(bool) src/dom/base/Document.cpp:11380:9
#65 0x7f36dadd8956 in mozilla::dom::Document::DispatchContentLoadedEvents() src/dom/base/Document.cpp:7901:3
#66 0x7f36dae4a316 in applyImpl<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1147:12
#67 0x7f36dae4a316 in apply<mozilla::dom::Document, void (mozilla::dom::Document::*)()> /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1153:12
#68 0x7f36dae4a316 in mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:1200:13
#69 0x7f36d90fa162 in mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:144:20
#70 0x7f36d91280ae in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:502:16
#71 0x7f36d910337f in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:805:26
#72 0x7f36d9101fe8 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:641:15
#73 0x7f36d9102263 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:425:36
#74 0x7f36d912b6a6 in operator() src/xpcom/threads/TaskController.cpp:135:37
#75 0x7f36d912b6a6 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_0>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:531:5
#76 0x7f36d9116b5f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1148:16
#77 0x7f36d911d8aa in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:466:10
#78 0x7f36d9b6c576 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:85:21
#79 0x7f36d9a8cb57 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#80 0x7f36d9a8ca62 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#81 0x7f36d9a8ca62 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#82 0x7f36dda1ed18 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#83 0x7f36df8a1713 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:917:20
#84 0x7f36d9b6d46a in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:235:9
#85 0x7f36d9a8cb57 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:331:10
#86 0x7f36d9a8ca62 in RunHandler src/ipc/chromium/src/base/message_loop.cc:324:3
#87 0x7f36d9a8ca62 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:306:3
#88 0x7f36df8a0d4e in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:749:34
#89 0x55d4a8e25ab6 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#90 0x55d4a8e25ab6 in main src/browser/app/nsBrowserApp.cpp:327:18
#91 0x7f36f07e40b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
#92 0x55d4a8e028bc in _start (/home/user/workspace/browsers/m-c-20210907094849-fuzzing-debug/firefox-bin+0x158bc)

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/FUzFBvXnms2x2r7skKZ7sw/index.html

Comment 2

[Bugmon [:jkratzer for issues]]

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

Start: a5704004514340c73b01dce446c0690abd215621 (20210827152456)
End: 15fdb071da984afa7541a63cfb00d615d1a09c05 (20210827183025)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a5704004514340c73b01dce446c0690abd215621&tochange=15fdb071da984afa7541a63cfb00d615d1a09c05

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1729578 - Adjust overly-strict assertion in nsContinuingTextFrame::FirstContinuation(). r=#layout-reviewers

It turns out this assertion (added as part of bug 1725555) is not necessarily valid.
When we're updating the frame tree in response to DOM modifications (in the testcase
here, we're under nsCSSFrameConstructor::ContentRemoved), we may call DeleteFrom on
the primary textFrame first, which results in calling SetPrevInFlow(nullptr) on its
nextContinuation; this in turn clears the mFirstContinuation back-pointers in all
the successors. But if we then call DeleteFrom on one of the continuations later
in the chain, those nulled-out pointers trigger this assertion. In fact, it's OK
for them to be null in this case, so the assertion is over-zealous and we should
drop it.

I believe the only time it's OK for this to happen is if the chain of nsContinuingTextFrames
has been "detached" from the primary frame during frame-tree updating, as has happened in
this example.

We can still sanity-check the mFirstContinuation pointer here by asserting that it
matches the FirstContinuation() reported by our prev-continuation, or is null if there
is no prev-continuation. This is a bit expensive, though, so it's a only a debug-mode
assertion (for the fuzzers to try and hit).

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88fee7a54781
Adjust overly-strict assertion in nsContinuingTextFrame::FirstContinuation(). r=emilio

Comment 5

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

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

Comment 6

[Narcis Beleuzu [:NarcisB]]

Backed out for crashtest failures on 1507961-1.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/ae68c3ee95d687c7a978ef3e74bb17793daeac61
Log link: https://treeherder.mozilla.org/logviewer?job_id=351480915&repo=autoland&lineNumber=9347

Please also check the geckoview-junit-e10s failure -> https://treeherder.mozilla.org/logviewer?job_id=351482709&repo=autoland&lineNumber=12919

Comment 7

[Jonathan Kew [:jfkthame]]

Ugh, I guess the new assertion is potentially slow enough to cause timeouts on large testcases where it gets tested a huge number of times.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0e0c84f0c6e7
Adjust overly-strict assertion in nsContinuingTextFrame::FirstContinuation(). r=emilio

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0e0c84f0c6e7

Comment 10

[Bugmon [:jkratzer for issues]]

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

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 9240970 [details]
Bug 1729578 - Adjust overly-strict assertion in nsContinuingTextFrame::FirstContinuation(). r=#layout-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Possible content-process crash. Although this was found by the fuzzers, it could affect real-world sites (rarely, I hope).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: Load the attached testcase -> crash.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The crash is because we're hitting an overly-strict diagnostic-assert; the patch just makes that assertion debug-only, and targets it more precisely, so that it won't fire spuriously and affect release-build users.
• String changes made/needed:

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9240970 [details]
Bug 1729578 - Adjust overly-strict assertion in nsContinuingTextFrame::FirstContinuation(). r=#layout-reviewers

Low risk crash fix in early betas, approved for 93 beta 6, thanks.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/184a0c849433