Closed Bug 1548985 Opened 6 months ago Closed 6 months ago

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

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox67 --- unaffected
firefox68 --- fixed

People

(Reporter: tsmith, Assigned: dholbert)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: assertion, crash, testcase)

Attachments

(3 files)

Attached file 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
Flags: in-testsuite?

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

Flags: needinfo?(dholbert)
Regressed by: 1544121

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

Flags: needinfo?(dholbert)
Keywords: crash

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

Assignee: nobody → dholbert
Status: NEW → ASSIGNED

(or possibly Tuesday)

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.)

Duplicate of this bug: 1549877
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/129dafe816e8
Don't let 'contain:size' suppress the intrinsic size of document-root SVG elements. r=TYLin
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.