1576972 - Assertion failure: hasNext (Can't find frame in lines!), at /builds/worker/workspace/build/src/layout/base/nsBidiPresUtils.cpp:255

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Testcase found while fuzzing mozilla-central rev 98cbe150f012.

Assertion failure: hasNext (Can't find frame in lines!), at /builds/worker/workspace/build/src/layout/base/nsBidiPresUtils.cpp:255

rax = 0x0000560805ae51a0   rdx = 0x0000000000000000
rcx = 0x00007fbd7cf675fe   rbx = 0x00007ffc15c37f20
rsi = 0x00007fbd886668b0   rdi = 0x00007fbd88665680
rbp = 0x00007ffc15c37a90   rsp = 0x00007ffc15c37a70
r8 = 0x00007fbd886668b0    r9 = 0x00007fbd897e8780
r10 = 0x0000000000000000   r11 = 0x0000000000000000
r12 = 0x00007fbd51163a78   r13 = 0x00007fbd51163e90
r14 = 0x0000000000000000   r15 = 0x00007fbd51164070
rip = 0x00007fbd797e937e
OS|Linux|0.0.0 Linux 4.19.34-coreos #1 SMP Mon Apr 22 20:32:34 -00 2019 x86_64
CPU|amd64|family 6 model 62 stepping 4|32
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|BidiParagraphData::FastLineIterator::AdvanceToFrame(nsIFrame*)|hg:hg.mozilla.org/mozilla-central:layout/base/nsBidiPresUtils.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|249|0x29
0|1|libxul.so|BidiParagraphData::FastLineIterator::AdvanceToLinesAndFrame(BidiParagraphData::FrameInfo const&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsBidiPresUtils.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|283|0xb
0|2|libxul.so|nsBidiPresUtils::ResolveParagraph(BidiParagraphData*)|hg:hg.mozilla.org/mozilla-central:layout/base/nsBidiPresUtils.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1131|0x1a
0|3|libxul.so|nsBidiPresUtils::Resolve(nsBlockFrame*)|hg:hg.mozilla.org/mozilla-central:layout/base/nsBidiPresUtils.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|865|0x8
0|4|libxul.so|nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsBlockFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1241|0x10
0|5|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|896|0x1d
0|6|libxul.so|nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsCanvasFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|730|0x1d
0|7|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|896|0x1d
0|8|libxul.so|nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|646|0x5
0|9|libxul.so|nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|759|0xe
0|10|libxul.so|nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1160|0x5
0|11|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|936|0x19
0|12|libxul.so|mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/ViewportFrame.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|310|0x2b
0|13|libxul.so|mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|9257|0x21
0|14|libxul.so|mozilla::PresShell::ProcessReflowCommands(bool)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|9427|0x11
0|15|libxul.so|mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|4170|0x15
0|16|libxul.so|mozilla::PresShell::DoFlushPendingNotifications(mozilla::FlushType)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|3947|0x7
0|17|libxul.so|nsDocumentViewer::LoadComplete(nsresult)|hg:hg.mozilla.org/mozilla-central:layout/base/nsDocumentViewer.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1031|0x13
0|18|libxul.so|nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult)|hg:hg.mozilla.org/mozilla-central:docshell/base/nsDocShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|6518|0x14
0|19|libxul.so|nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult)|hg:hg.mozilla.org/mozilla-central:docshell/base/nsDocShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|6318|0x18
0|20|libxul.so|nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1336|0x64
0|21|libxul.so|nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|895|0x2a
0|22|libxul.so|nsDocLoader::DocLoaderIsEmpty(bool)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|729|0x15
0|23|libxul.so|nsDocLoader::OnStopRequest(nsIRequest*, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|617|0x16
0|24|libxul.so|mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult)|hg:hg.mozilla.org/mozilla-central:netwerk/base/nsLoadGroup.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|568|0x17
0|25|libxul.so|mozilla::dom::Document::DoUnblockOnload()|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|10674|0x20
0|26|libxul.so|mozilla::dom::Document::UnblockOnload(bool)|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|10606|0x5
0|27|libxul.so|mozilla::dom::Document::DispatchContentLoadedEvents()|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|7168|0xd
0|28|libxul.so|mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::*)(), true, (mozilla::RunnableKind)0>::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.h:98cbe150f012619bf9e8d44413fa33374e92dbc9|1176|0x13
0|29|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|1225|0x15
0|30|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|486|0x11
0|31|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|88|0xa
0|32|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98cbe150f012619bf9e8d44413fa33374e92dbc9|315|0x17
0|33|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98cbe150f012619bf9e8d44413fa33374e92dbc9|290|0x8
0|34|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|137|0xd
0|35|libxul.so|nsAppStartup::Run()|hg:hg.mozilla.org/mozilla-central:toolkit/components/startup/nsAppStartup.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|276|0xe
0|36|libxul.so|XREMain::XRE_mainRun()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|4573|0x11
0|37|libxul.so|XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|4711|0x8
0|38|libxul.so|XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|4792|0x5
0|39|firefox-bin|do_main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|213|0x22
0|40|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:98cbe150f012619bf9e8d44413fa33374e92dbc9|295|0xf
0|41|libc-2.27.so||||0x21b97
0|42|firefox-bin|MOZ_ReportCrash|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:98cbe150f012619bf9e8d44413fa33374e92dbc9|184|0x5

Comment 1

[Daniel Holbert [:dholbert]]

(I think you may have forgotten to attach the testcase)

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Comment 3

[Jason Kratzer [:jkratzer]]

Testcase bisects to the following build range:

Start: 69ac304560c98a733d44a0245fe9782dc6a465e2 (20190723034754)
End: 5b35e2ff7c15cc2f4d1eb419a18d899294560243 (20190723155748)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=69ac304560c98a733d44a0245fe9782dc6a465e2&tochange=5b35e2ff7c15cc2f4d1eb419a18d899294560243

Comment 4

[Daniel Holbert [:dholbert]]

Thanks. Almost certainly a regression from bug 1300293, which touched the assertion that's failing here (moved it and promoted it from NS_ASSERTION to MOZ_ASSERT), in https://hg.mozilla.org/mozilla-central/rev/fc179320bf36cb2b398d4f49be817ece9562b100

Jason, would you mind testing to see whether you get a non-fatal version of this assertion in a build from before your bisected range (###!!! ASSERTION: ... with the same message, rather than a fatal abort)?

It'd be good to know whether that range is where the assertion started failing vs. whether that range is simply where an already-failing assertion became fatal (via the above-linked commit).

Comment 5

[David Baron :dbaron:]

This fixes it. I'll turn it into an actual patch with a test tomorrow (I hope).

The basic problem was that when it tried to advance to lastRealFrame it was trying to go backwards, since this case was failing to update lastRealFrame. (Moving the setting of lastRealFrame earlier would introduce more complexity, I think, because of the splitting cases that change it.)

diff --git a/layout/base/nsBidiPresUtils.cpp b/layout/base/nsBidiPresUtils.cpp
index ab940e156be6..c44915c868f6 100644
--- a/layout/base/nsBidiPresUtils.cpp
+++ b/layout/base/nsBidiPresUtils.cpp
@@ -1018,7 +1018,9 @@ nsresult nsBidiPresUtils::ResolveParagraph(BidiParagraphData* aBpd) {
           // Set the base level and embedding level of the current run even
           // on an empty frame. Otherwise frame reordering will not be correct.
           frame->AdjustOffsetsForBidi(0, 0);
-          // Nothing more to do for an empty frame.
+          // Nothing more to do for an empty frame, except update
+          // lastRealFrame like we do below.
+          lastRealFrame = frameInfo;
           continue;
         }
         nsLineList::iterator currentLine = aBpd->mCurrentResolveLine.GetLine();

Comment 6

[David Baron :dbaron:]

(Although I'm wondering if there's actually more that needs doing...)

Comment 7

[Jason Kratzer [:jkratzer]]

(In reply to Daniel Holbert [:dholbert] from comment #4)

Thanks. Almost certainly a regression from bug 1300293, which touched the assertion that's failing here (moved it and promoted it from NS_ASSERTION to MOZ_ASSERT), in https://hg.mozilla.org/mozilla-central/rev/fc179320bf36cb2b398d4f49be817ece9562b100

Jason, would you mind testing to see whether you get a non-fatal version of this assertion in a build from before your bisected range (###!!! ASSERTION: ... with the same message, rather than a fatal abort)?

It'd be good to know whether that range is where the assertion started failing vs. whether that range is simply where an already-failing assertion became fatal (via the above-linked commit).

I just tested using m-c rev 64fc6a9a9fb2 (20190722) and do not see the non-fatal assertion message.

Comment 8

[David Baron :dbaron:]

Attachment: Bug 1576972 - Keep lastRealFrame up-to-date even when we skip empty text frames.

Comment 9

[David Baron :dbaron:]

I'd note that I tested the crashtest both with and without the patch; I got the assertion without, and pass with.

Comment 10

[David Baron :dbaron:]

And here's a try run.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/696c78a89f82
Keep lastRealFrame up-to-date even when we skip empty text frames. r=jfkthame

Comment 12

[David Baron :dbaron:]

Also, I believe the reason this was actually a regression from bug 1300293 is that 57c43ac551e8 introduced the call to AdvanceToLinesAndFrame that was where we tried to advance backwards because lastRealFrame wasn't kept up-to-date correctly.

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/696c78a89f82

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/696c78a89f82