1548985 - Assertion failure: !intrinsicSize.width (GetIntrinsicSize should have reported no intrinsic width), at src/layout/svg/nsSVGOuterSVGFrame.cpp:320

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: !intrinsicSize.width (GetIntrinsicSize should have reported no intrinsic width), at src/layout/svg/nsSVGOuterSVGFrame.cpp:320

#0 nsSVGOuterSVGFrame::ComputeSize(gfxContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) src/layout/svg/nsSVGOuterSVGFrame.cpp:331:7
#1 mozilla::ReflowInput::InitConstraints(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, nsMargin const*, nsMargin const*, mozilla::LayoutFrameType) src/layout/generic/ReflowInput.cpp:2484:34
#2 mozilla::ReflowInput::Init(nsPresContext*, mozilla::Maybe<mozilla::LogicalSize> const&, nsMargin const*, nsMargin const*) src/layout/generic/ReflowInput.cpp:379:3
#3 mozilla::ReflowInput::ReflowInput(nsPresContext*, mozilla::ReflowInput const&, nsIFrame*, mozilla::LogicalSize const&, mozilla::Maybe<mozilla::LogicalSize> const&, unsigned int) src/layout/generic/ReflowInput.cpp:226:5
#4 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:708:17
#5 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:893:14
#6 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*) src/layout/generic/nsGfxScrollFrame.cpp:562:3
#7 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:675:3
#8 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1076:3
#9 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:932:14
#10 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:307:7
#11 mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) src/layout/base/PresShell.cpp:9172:11
#12 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9342:24
#13 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4185:11
#14 nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1949:20
#15 mozilla::InactiveRefreshDriverTimer::TickOne() src/layout/base/nsRefreshDriver.cpp:943:7
#16 mozilla::InactiveRefreshDriverTimer::TimerTickOne(nsITimer*, void*) src/layout/base/nsRefreshDriver.cpp:952:12
#17 nsTimerImpl::Fire(int) src/xpcom/threads/nsTimerImpl.cpp:561:7
#18 nsTimerEvent::Run() src/xpcom/threads/TimerThread.cpp:260:11
#19 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1180:14
#20 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
#21 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:110:5
#22 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:315:10
#23 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
#24 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#25 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:919:20
#26 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:238:9
#27 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:315:10
#28 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
#29 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:757:34
#30 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#31 main src/browser/app/nsBrowserApp.cpp:263:18

Comment 1

[Daniel Holbert [:dholbert]]

The testcase uses contain:size and the assertion is about intrinsic size, so this is likely a regression from bug 1544121.

Comment 2

[Daniel Holbert [:dholbert]]

Also, the testcase crashes my Nightly by tripping a MOZ_RELEASE_ASSERT:
bp-cfe43e0c-774b-45a4-a382-51b080190504

Comment 3

[Daniel Holbert [:dholbert]]

I'll take a look at this on Monday if I don't get to it sooner.

Comment 4

[Daniel Holbert [:dholbert]]

(or possibly Tuesday)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (warning, crashes Firefox Nightly)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: Bug 1548985: Don't let 'contain:size' suppress the intrinsic size of document-root SVG elements. r?TYLin

Comment 7

[Daniel Holbert [:dholbert]]

The assertion-failure is inside of this clause:

  if (!mContent->GetParent()) {
    // We're the root of the outermost browsing context, so we need to scale
    // cbSize by the full-zoom so that SVGs with percentage width/height zoom:
[...]
    // We also need to honour the width and height attributes' default values
    // of 100% when we're the root of a browsing context.  (GetIntrinsicSize()
    // doesn't report these since there's no such thing as a percentage
    // intrinsic size.  Also note that explicit percentage values are mapped
    // into style, so the following isn't for them.)
[...]
    if (width.IsPercentage()) {
      MOZ_ASSERT(!intrinsicSize.width,
                 "GetIntrinsicSize should have reported no intrinsic width");
      float val = width.GetAnimValInSpecifiedUnits() / 100.0f;
      intrinsicSize.width.emplace(std::max(val, 0.0f) *
                                  cbSize.Width(aWritingMode));
    }

As noted in the second code-comment's parenthetical there, this code is assuming that a percent width or height attribute (the default value) will produce "Nothing()" in our GetIntrinsicSize() output, in that dimension. And if this is a SVG Document (i.e. if our SVG Element is the root element), then we replace that Nothing value with an actual intrinsic size that is based on the viewport size.

The assertion was failing in this case because contain:size unconditionally gives this element an intrinsic size with Some(0) instead of Nothing(). (Note that intrinsicSize.width is "truthy" when it is Some(0), so fails the MOZ_ASSERT(!intrinsicSize.width,...) check here. And opt builds trip over a MOZ_RELEASE_ASSERT in the .emplace(...) call, because you can only .emplace() an empty Maybe<> object.)

Really, it's silly of us to pretend the intrinsic size is 0,0 in this case where the SVG is the outermost element, because
(a) it's not really "replaced" (it contains the whole document - it's not behaving as an element in another external document)
(b) there's no outer context that we could "contain"/hide our internal sizing from, so it'd be odd to have special contain:size sizing behavior on this outermost node.

So, let's just avoid applying this contain:size 0,0-intrinsic-size behavior to outer SVG elements if they're the root of the document.

(This is only something we have to worry about for SVG -- i.e. I don't think we need the same behavior for other content -- because <svg> is the only element which can either be a replaced element or can be the root element in a document.)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/129dafe816e8
Don't let 'contain:size' suppress the intrinsic size of document-root SVG elements. r=TYLin

Comment 10

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/129dafe816e8