As discussed separately, it seems like the issue here is that ~TextRenderedRunIterator() assumes `mFrameIterator.Root()` is non-null; but we don't have that guarantee. In this case, Tyson let me know that the testcase hits bug 1467887's assertion in debug builds, which means `FrameIfAnonymousChildReflowed(` will be returning nullptr in opt builds; and so we'll be passing nullptr to initialize mFrameIterator (as its first arg) here: https://searchfox.org/firefox-main/rev/dc1c78e9c37aba6ed05a4ec47c4bfcb16f57b51d/layout/svg/SVGTextFrame.cpp#1756-1759 ```cpp explicit TextRenderedRunIterator(SVGTextFrame* aSVGTextFrame, RenderedRunFilter aFilter = eAllFrames, const nsIFrame* aSubtree = nullptr) : mFrameIterator(FrameIfAnonymousChildReflowed(aSVGTextFrame), aSubtree), ``` It handles a null mRootFrame nicely internally, e.g. here: https://searchfox.org/firefox-main/rev/dc1c78e9c37aba6ed05a4ec47c4bfcb16f57b51d/layout/svg/SVGTextFrame.cpp#1568-1571 ```cpp void Init() { if (!mRootFrame) { return; } ``` So it needs to handle null mRootFrame nicely at destruction-time too. I propose: (1) We null-check Root() before using it, in the TextRenderedRunIterator destructor. (2) Perhaps we we rename Root() and Current() and Next() to have a "Get" prefix, since they return pointers which need to be null-checked (cannot be assumed to be non-null), per our naming convention.
Bug 1993379 Comment 3 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
As discussed separately, it seems like the issue here is that ~TextRenderedRunIterator() assumes `mFrameIterator.Root()` is non-null; but we don't have that guarantee. In this case, Tyson let me know that the testcase hits bug 1467887's assertion in debug builds, which means `FrameIfAnonymousChildReflowed(` will be returning nullptr in opt builds; and so we'll be passing nullptr to initialize mFrameIterator (as its first arg) here: https://searchfox.org/firefox-main/rev/dc1c78e9c37aba6ed05a4ec47c4bfcb16f57b51d/layout/svg/SVGTextFrame.cpp#1756-1759 ```cpp explicit TextRenderedRunIterator(SVGTextFrame* aSVGTextFrame, RenderedRunFilter aFilter = eAllFrames, const nsIFrame* aSubtree = nullptr) : mFrameIterator(FrameIfAnonymousChildReflowed(aSVGTextFrame), aSubtree), ``` It handles a null mRootFrame nicely internally, e.g. here: https://searchfox.org/firefox-main/rev/dc1c78e9c37aba6ed05a4ec47c4bfcb16f57b51d/layout/svg/SVGTextFrame.cpp#1568-1571 ```cpp void Init() { if (!mRootFrame) { return; } ``` So it needs to handle null mRootFrame nicely at destruction-time too. I propose: (1) We null-check Root() before using it, in the TextRenderedRunIterator destructor. (2) Perhaps we we rename TextFrameIterator::Root() and Current() and Next() to have a "Get" prefix, since they return pointers which need to be null-checked (cannot be assumed to be non-null), per our naming convention.