1601824 - InvalidArrayIndex_CRASH in [@ SVGTextFrame::DoTextPathLayout]

Status: RESOLVED FIXED

(Core :: SVG, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Hit MOZ_CRASH(ElementAt(aIndex = 1, aLength = 1)) at src/xpcom/ds/nsTArray.cpp:29

#0 MOZ_Crash(char const*, int, char const*) src/obj-firefox/dist/include/mozilla/Assertions.h:332:3
#1 InvalidArrayIndex_CRASH(unsigned long, unsigned long) src/xpcom/ds/nsTArray.cpp:27:3
#2 nsTArray_Impl<double, nsTArrayInfallibleAllocator>::ElementAt(unsigned long) src/obj-firefox/dist/include/nsTArray.h:1067:7
#3 SVGTextFrame::DoTextPathLayout() src/layout/svg/SVGTextFrame.cpp:4712:35
#4 SVGTextFrame::DoGlyphPositioning() src/layout/svg/SVGTextFrame.cpp:4932:3
#5 SVGTextFrame::ReflowSVG() src/layout/svg/SVGTextFrame.cpp:3300:3
#6 nsSVGDisplayContainerFrame::ReflowSVG() src/layout/svg/nsSVGContainerFrame.cpp:319:17
#7 nsSVGOuterSVGFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/svg/nsSVGOuterSVGFrame.cpp:459:14
#8 nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:878:13
#9 nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4376:15
#10 nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:4178:5
#11 nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:4063:9
#12 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3042:5
#13 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2582:7
#14 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1325:3
#15 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:293:11
#16 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3694:11
#17 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3039:5
#18 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2582:7
#19 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1325:3
#20 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:910:14
#21 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:738:5
#22 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:910:14
#23 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*) src/layout/generic/nsGfxScrollFrame.cpp:649:3
#24 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:763:3
#25 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1142:3
#26 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, nsIFrame::ReflowChildFlags, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:950:14
#27 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:299:7
#28 mozilla::PresShell::DoReflow(nsIFrame*, bool, mozilla::OverflowChangedTracker*) src/layout/base/PresShell.cpp:9179:11
#29 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9352:24
#30 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4111:11
#31 nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2051:20
#32 mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:351:7
#33 mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:368:5
#34 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:740:16
#35 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::VsyncEvent const&) src/layout/base/nsRefreshDriver.cpp:635:9
#36 mozilla::layout::VsyncChild::RecvNotify(mozilla::VsyncEvent const&) src/layout/ipc/VsyncChild.cpp:65:16
#37 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:187:54
#38 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:5876:32
#39 mozilla::ipc::MessageChannel::DispatchAsyncMessage(mozilla::ipc::ActorLifecycleProxy*, IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2208:25
#40 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2130:9
#41 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1972:3
#42 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:2003:13
#43 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1250:14
#44 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:486:10
#45 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:88:21
#46 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:315:10
#47 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
#48 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#49 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:943:20
#50 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:238:9
#51 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:315:10
#52 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:290:3
#53 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:778:34
#54 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#55 main src/browser/app/nsBrowserApp.cpp:303:18

Comment 1

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/1htl45UneapNncAqsacpeA/index.html

Comment 2

[Jonathan Kew [:jfkthame]]

This looks likely to be a regression from the recent SVG text changes (although I didn't try to confirm this).

Comment 3

[Cameron McCormack (:heycam)]

Thanks, yes looks like it. I will take a look soon.

Comment 4

[Cameron McCormack (:heycam)]

I think the issue is that the partialAdvances array gets an element for every character matching CharacterFilter::eOriginal, but we're looping from i + 1 (the second original character of the cluster / ligature group) up to k (the original character index after the cluster / ligature group), which includes undisplayed characters, such as those inside the <a systemLanguage=""> here, and which aren't reported by CharacterFilter::eOriginal.

We should pad out partialAdvances with extra values when we detect we had undisplayed characters, so that we can loop over the right number of parallel entries in partialAdvances and mPositions.

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1601824 - Measure partial ligature advance contributions correctly on text paths.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a061c1e11b76
Measure partial ligature advance contributions correctly on text paths. r=longsonr

Comment 8

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/a061c1e11b76

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.