1665773 - Assertion failure: GetGlyphCount() == 0 (Glyph count already set), at src/gfx/thebes/gfxFont.h:918

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: GetGlyphCount() == 0 (Glyph count already set), at src/gfx/thebes/gfxFont.h:918

#0 0x7fb04f8103f7 in gfxShapedText::CompressedGlyph::SetGlyphCount(unsigned int) /builds/worker/workspace/obj-build/dist/include/gfxFont.h:918:7
#1 0x7fb0528d7091 in MergeCharactersInTextRun(gfxTextRun*, gfxTextRun*, bool const*, bool const*) src/layout/generic/nsTextRunTransformations.cpp:190:18
#2 0x7fb0528d8eb5 in nsCaseTransformTextRunFactory::RebuildTextRun(nsTransformedTextRun*, mozilla::gfx::DrawTarget*, gfxMissingFontRecorder*) src/layout/generic/nsTextRunTransformations.cpp:870:5
#3 0x7fb0528b25d4 in FinishSettingProperties src/layout/generic/nsTextRunTransformations.h:163:17
#4 0x7fb0528b25d4 in BuildTextRunsScanner::BreakSink::Finish(gfxMissingFontRecorder*) src/layout/generic/nsTextFrame.cpp:1115:29
#5 0x7fb0528b2402 in BuildTextRunsScanner::FlushLineBreaks(gfxTextRun*) src/layout/generic/nsTextFrame.cpp:1687:21
#6 0x7fb0528af63e in BuildTextRunsScanner::FlushFrames(bool, bool) src/layout/generic/nsTextFrame.cpp:1662:5
#7 0x7fb0528b5b92 in BuildTextRuns src/layout/generic/nsTextFrame.cpp:1581:11
#8 0x7fb0528b5b92 in nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, mozilla::gfx::DrawTarget*, nsIFrame*, nsLineList_iterator const*, unsigned int*) src/layout/generic/nsTextFrame.cpp:2998:7
#9 0x7fb0528bb847 in nsTextFrame::GetRenderedText(unsigned int, unsigned int, nsIFrame::TextOffsetType, nsIFrame::TrailingWhitespace) src/layout/generic/nsTextFrame.cpp:9801:20
#10 0x7fb05374639c in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:678:46
#11 0x7fb05264f7aa in nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:2054:12
#12 0x7fb0526572a1 in TickDriver src/layout/base/nsRefreshDriver.cpp:372:13
#13 0x7fb0526572a1 in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:351:7
#14 0x7fb05265718c in mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:366:5
#15 0x7fb05265c998 in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:818:5
#16 0x7fb05265c998 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:736:16
#17 0x7fb05265c291 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyParentProcessVsync() src/layout/base/nsRefreshDriver.cpp:638:7
#18 0x7fb05265550d in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:537:20
#19 0x7fb04deac9ff in mozilla::RunnableTask::Run() src/xpcom/threads/TaskController.cpp:242:16
#20 0x7fb04deaaa7a in mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:512:26
#21 0x7fb04dea9bd4 in mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal(mozilla::detail::BaseAutoLock<mozilla::Mutex&> const&) src/xpcom/threads/TaskController.cpp:371:15
#22 0x7fb04dea9d87 in mozilla::TaskController::ProcessPendingMTTask(bool) src/xpcom/threads/TaskController.cpp:168:36
#23 0x7fb04deb1736 in operator() src/xpcom/threads/TaskController.cpp:83:37
#24 0x7fb04deb1736 in mozilla::detail::RunnableFunction<mozilla::TaskController::InitializeInternal()::$_4>::Run() /builds/worker/workspace/obj-build/dist/include/nsThreadUtils.h:577:5
#25 0x7fb04dec4b3f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1234:14
#26 0x7fb04deca4ea in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:513:10
#27 0x7fb04e7c6de6 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:87:21
#28 0x7fb04e739ad3 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#29 0x7fb04e7399ed in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#30 0x7fb04e7399ed in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#31 0x7fb0523b5d48 in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:137:27
#32 0x7fb053b96713 in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:913:20
#33 0x7fb04e7c7ba9 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:237:9
#34 0x7fb04e739ad3 in MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:334:10
#35 0x7fb04e7399ed in RunHandler src/ipc/chromium/src/base/message_loop.cc:327:3
#36 0x7fb04e7399ed in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:309:3
#37 0x7fb053b962f8 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:744:34
#38 0x56247c5bb917 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:56:28
#39 0x56247c5bb917 in main src/browser/app/nsBrowserApp.cpp:303:18

Comment 1

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 2

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Comment 3

[Tyson Smith [:tsmith]]

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

Comment 4

[Jonathan Kew [:jfkthame]]

Ah, I see -- this is basically a followup to bug 1663230, where the patch wasn't quite right. Since the change there, CompressedGlyph::SetComplex shouldn't be setting the glyphCount field, as that's the responsibility of gfxShapedText::SetDetailedGlyphs.

This is harmless -- the field just ends up being reset to the same value twice -- but it's misusing the API and hence it asserts to tell us so.

It looks like every other caller of SetComplex currently just passes zero for the glyph count, and that's what we should be doing here as well. Which means we can remove that parameter altogether. Patch coming.

Comment 5

[Jonathan Kew [:jfkthame]]

And similarly, CompressedGlyph::SetMissing shouldn't be responsible for the glyphCount; it should only set the flags to indicate a missing-glyph, and leave SetDetailedGlyphs to manage the glyphCount field.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1665773 - Ensure CompressedGlyph::SetComplex() always clears the glyphCount to zero; let gfxShapedText::SetDetailedGlyphs() own the setting of this field. r=heycam

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3466a6111998
Ensure CompressedGlyph::SetComplex() always clears the glyphCount to zero; let gfxShapedText::SetDetailedGlyphs() own the setting of this field. r=heycam

Comment 8

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3466a6111998

Comment 9

[Jonathan Kew [:jfkthame]]

Tyson, I tried to use your testcase here as a crashtest, but it didn't seem to work for me on tryserver; even when included in the /accessible/tests/crashtests, as it seemed to depend on having a11y support enabled, it still didn't hit the assertion in my try runs (without the patch, obviously).

If you happen to have any other examples that also hit this assertion, it'd be good to have tests for it in CI, but at the moment I'm not sure how to achieve that.

Comment 10

[Tyson Smith [:tsmith]]

Sorry I don't have any other tests.