Closed Bug 1486457 Opened 2 years ago Closed 2 years ago

Assertion failure: aState.mNoWrapFloats.IsEmpty(), at /builds/worker/workspace/build/src/layout/generic/nsBlockFrame.cpp:3965

Categories

(Core :: Layout: Ruby, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: jkratzer, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file testcase.html
Testcase found while fuzzing mozilla-central rev 190b827aaa2b.

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x0000000000000b40   rbx = 0x00007ffc109fc808
rsi = 0x00007f48dd0ba8b0   rdi = 0x00007f48dd0b9680
rbp = 0x00007ffc109fc2f0   rsp = 0x00007ffc109fc140
r8 = 0x00007f48dd0ba8b0    r9 = 0x00007f48de232740
r10 = 0x00000000ffffffc7   r11 = 0x0000000000000000
r12 = 0x00007f48c2fcb2b0   r13 = 0x00007ffc109fc1c8
r14 = 0x00007ffc109fc416   r15 = 0x000000007fffffff
rip = 0x00007f48cdb547ec
OS|Linux|0.0.0 Linux 4.15.0-32-generic #35-Ubuntu SMP Fri Aug 10 17:58:07 UTC 2018 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|0
0|0|libxul.so|nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsBlockFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|3965|0x18
0|1|libxul.so|nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsBlockFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2924|0x1a
0|2|libxul.so|nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsBlockFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2458|0x20
0|3|libxul.so|nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsBlockFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|1292|0xf
0|4|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|951|0x1a
0|5|libxul.so|nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsCanvasFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|804|0x4d
0|6|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|951|0x1a
0|7|libxul.so|nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|608|0x5
0|8|libxul.so|nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|731|0x14
0|9|libxul.so|nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsGfxScrollFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|1120|0x5
0|10|libxul.so|nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*)|hg:hg.mozilla.org/mozilla-central:layout/generic/nsContainerFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|995|0x19
0|11|libxul.so|mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&)|hg:hg.mozilla.org/mozilla-central:layout/generic/ViewportFrame.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|339|0x2b
0|12|libxul.so|mozilla::PresShell::DoReflow(nsIFrame*, bool)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|9022|0x25
0|13|libxul.so|mozilla::PresShell::ProcessReflowCommands(bool)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|9195|0xe
0|14|libxul.so|mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|4347|0x15
0|15|libxul.so|nsRefreshDriver::Tick(mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|1926|0x5
0|16|libxul.so|mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|324|0x8
0|17|libxul.so|mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|317|0xc
0|18|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|755|0xc
0|19|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|571|0xc
0|20|libxul.so|mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&)|hg:hg.mozilla.org/mozilla-central:layout/ipc/VsyncChild.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|78|0x9
0|21|libxul.so|mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&)|s3:gecko-generated-sources:0c7cf777c2ff93c34ff1546f677320cb1229427e6947e87c6fa76720f9b9c5b6a4a4d036521ed9a643f4fa5e10a57d8748e2532d47fce8282aa653340c0c00ff/ipc/ipdl/PVsyncChild.cpp:|167|0xc
0|22|libxul.so|mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2239|0x6
0|23|libxul.so|mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2166|0xb
0|24|libxul.so|mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2012|0xb
0|25|libxul.so|mozilla::ipc::MessageChannel::MessageTask::Run()|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|2045|0xc
0|26|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|1167|0x15
0|27|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|519|0x11
0|28|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|97|0xa
0|29|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|325|0x17
0|30|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|318|0x8
0|31|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|158|0xd
0|32|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|944|0x11
0|33|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|269|0x5
0|34|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|325|0x17
0|35|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|318|0x8
0|36|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|770|0x8
0|37|firefox|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|50|0x14
0|38|firefox|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|287|0x11
0|39|libc-2.27.so||||0x21b97
0|40|firefox|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:190b827aaa2b5e6fb9af7a0defb238ccc35f8b9e|164|0x5
Flags: in-testsuite?
Flags: needinfo?(emilio)
I don't understand enough of how ruby uses nsLineLayout to know if this proposed fix would be right. Sounds like it should. Xidorn, IIUC ruby stuff can't be float containers, so we should be able to try placing the floats after finishing the rtc layout like:

diff --git a/layout/generic/nsRubyBaseContainerFrame.cpp b/layout/generic/nsRubyBaseContainerFrame.cpp
index a71c71a8f3af..8581929d3851 100644
--- a/layout/generic/nsRubyBaseContainerFrame.cpp
+++ b/layout/generic/nsRubyBaseContainerFrame.cpp
@@ -431,16 +431,17 @@ nsRubyBaseContainerFrame::Reflow(nsPresContext* aPresContext,
     } else if (isize > rtcISize) {
       RubyUtils::SetReservedISize(textContainer, isize - rtcISize);
     }
 
     lineLayout->VerticalAlignLine();
     textContainer->SetISize(rtcISize);
     lineLayout->EndLineReflow();
   }
+  aReflowInput.mLineLayout->FlushNoWrapFloats();
 
   // Border and padding are suppressed on ruby base container,

Does that sound right? Alternatively we can try to place them after every inner line layout has finished reflowing... Not sure what's right here, do you know?
Flags: needinfo?(emilio) → needinfo?(xidorn+moz)
Component: Layout: Block and Inline → Layout: Ruby
Floats inside ruby don't have any meaningful use case so whatever simplest would be the best.

In this case, it isn't really clear to me whether that is the right thing to do... ReflowSpans would invoke line layout to reflow the ruby text frame, which is roughly just an inline frame, so I wonder shouldn't EndSpan handles this for ruby text frame already?
Flags: needinfo?(xidorn+moz)
My guess would be probably it is because of nested ruby where inner ruby text container also have a line breaking suppression and thus EndSpan doesn't flush that.

This is uncommon because there are very few callsites would start a root span (block, first-letter, ruby text container), and in the block case, at least, FlushNoWrapFloats would be invoked in PlaceLine. (This indicates that this assertion may happen for first-letter as well?)

So yeah, I think it's probably the right thing to do. Even if it's theoretically wrong, I'm not very worried about this at all.
Only one thing I'd like to confirm, would this affect floats inserted before ruby when the block frame is no-wrap? It sounds like in that case we would flush the floats prematurely. If there is no problem with that case, then I guess it is fine.
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> Only one thing I'd like to confirm, would this affect floats inserted before
> ruby when the block frame is no-wrap? It sounds like in that case we would
> flush the floats prematurely. If there is no problem with that case, then I
> guess it is fine.

Will double-check.
Flags: needinfo?(emilio)
We'll re-do the line anyway, and we'll forget about all the already-positioned
floats in the line DoReflowInlineFrames anyway.
Flags: needinfo?(emilio)
Assignee: nobody → emilio
Comment on attachment 9004591 [details]
Clear the float list if we weren't able to place a line.

David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 has approved the revision.
Attachment #9004591 - Flags: review+
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/7edf18c31ee0
Clear the float list if we weren't able to place a line. r=dbaron
https://hg.mozilla.org/mozilla-central/rev/7edf18c31ee0
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.