1460787 - Assertion failure: aReflowInput.ComputedISize() != nscoord((1 << 30) - 1) (Should have a precomputed inline-size!), at src/layout/forms/nsFieldSetFrame.cpp:369

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: aReflowInput.ComputedISize() != nscoord((1 << 30) - 1) (Should have a precomputed inline-size!), at src/layout/forms/nsFieldSetFrame.cpp:369

#0 nsFieldSetFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/forms/nsFieldSetFrame.cpp:367:3
#1 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
#2 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
#3 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
#4 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#5 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#6 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
#7 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
#8 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
#9 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#10 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#11 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#12 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:713:5
#13 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#14 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:555:3
#15 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:678:3
#16 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1055:3
#17 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
#18 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:335:7
#19 mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8981:11
#20 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9154:24
#21 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4366:11
#22 mozilla::PresShell::DoFlushPendingNotifications(mozilla::FlushType) src/layout/base/PresShell.cpp:4159:3
#23 nsDocumentViewer::LoadComplete(nsresult) src/layout/base/nsDocumentViewer.cpp:980:12
#24 nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult) src/docshell/base/nsDocShell.cpp:7243:21
#25 nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp:7036:7
#26 non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) src/docshell/base/nsDocShell.cpp
#27 nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult) src/uriloader/base/nsDocLoader.cpp:1315:3
#28 nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) src/uriloader/base/nsDocLoader.cpp:858:14
#29 nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:747:9
#30 nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:632:5
#31 non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp
#32 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:629:28
#33 nsIDocument::DoUnblockOnload() src/dom/base/nsDocument.cpp:8397:18
#34 nsDocument::UnblockOnload(bool) src/dom/base/nsDocument.cpp:8319:9
#35 nsIDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5299:3
#36 mozilla::detail::RunnableMethodImpl<nsIDocument*, void (nsIDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1216:13
#37 mozilla::SchedulerGroup::Runnable::Run() src/xpcom/threads/SchedulerGroup.cpp:337:32
#38 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14
#39 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#40 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#41 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#42 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#43 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
#44 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#45 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#46 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#47 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#48 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34
#49 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#50 main src/browser/app/nsBrowserApp.cpp:282:18
#51 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#52 _start (firefox+0x423444)

Comment 1

[Daniel Holbert [:dholbert]]

Sounds like we need to relax this fatal assertion, to be a warning perhaps.

The assertion is assuming that web content can't possibly produce a size equal to the sentinel value NS_INTRINSICSIZE, but that's just an extremely-large size which *is* indeed possible to reach with content that has arbitrarily large sizes.

(See also bug 765861; this is the sort of assertion that *could* be conditionally fatal if we had the sort of flag suggested there.)

Comment 2

[Daniel Holbert [:dholbert]]

I'm going to co-opt this as a "good first bug" for an intern starting next week.

Comment 3

[Daniel Holbert [:dholbert]]

We'll want to use NS_WARNING_ASSERTION(...) rather than MOZ_ASSERT(...) here. NS_WARNING_ASSERTION is non-fatal if it fails.

Comment 4

[Yusuf Sermet [:yusuf]]

Attachment: Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

Review commit: https://reviewboard.mozilla.org/r/246186/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246186/

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

https://reviewboard.mozilla.org/r/246186/#review252312

Thanks! r- for now, see comments (r+ with this fixed though)

::: layout/forms/crashtests/1460787-1.html:1
(Diff revision 1)
> +<script>
> +document.addEventListener("DOMContentLoaded", function(){

You'll need to add a line for this file to the crashtest.list that lives alongside it (in sorted order, which may or may not be at the very bottom of the file).

::: layout/forms/nsFieldSetFrame.cpp:368
(Diff revision 1)
> -  MOZ_ASSERT(aReflowInput.ComputedISize() != NS_INTRINSICSIZE,
> -             "Should have a precomputed inline-size!");
> +  NS_WARNING_ASSERTION(areflowinput.computedisize() != ns_intrinsicsize,
> +		"should have a precomputed inline-size!");

Two nits:
 - It looks like you lowercased some of the text here ("aReflowInput" went to "areflowinput") -- maybe your editor program did that by accident somehow?

 - Please indent the second line ("should...") so that it stays aligned just inside the open-paren on the previous line, as it was before.

Comment 6

[Yusuf Sermet [:yusuf]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246186/diff/1-2/

Comment 7

[Yusuf Sermet [:yusuf]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246186/diff/2-3/

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

https://reviewboard.mozilla.org/r/246186/#review252342

Looks good!  Let's trigger a try run, and then get this landed.

Comment 9

[Yusuf Sermet [:yusuf]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246186/diff/3-4/

Comment 10

[Yusuf Sermet [:yusuf]]

Comment on attachment 8980041 [details]
Bug 1460787 - Change fatal assertion to warning for extremely large sized pages.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246186/diff/4-5/

Comment 11

[Daniel Holbert [:dholbert]]

In the first few Try runs, Yusuf noticed that this testcase does trigger 2-4 non-fatal assertions (NS_ASSERTION) as well -- sample log: https://treeherder.mozilla.org/logviewer.html#?job_id=180039001&repo=try

Those assertions are for similar issues to what I described in comment 1, so perhaps we should fix them, but I don't want to scope-creep this bug and I think they're not as fuzz-blocker-ish, so we've just worked around them with an "asserts()" annotation in crashtest.list for this bug's crashtest for the time being.

So: I'll go ahead and autoland this.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46001e61b8bf
Change fatal assertion to warning for extremely large sized pages. r=dholbert

Comment 13

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/46001e61b8bf