14486 - UMR in nsTextFrame::Reflow, causing disk hits on reflow

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Simon Fraser [no longer active]]

There is a UMR in nsTextFrame::Reflow, which is causing a number of problems.
It results in the passing of a garbage string to
 aReflowState.rendContext->GetWidth()

On Mac, this ends up causing lots of font loading, as we try to find a font
that can render the garbage unicode characters. This causes serious disk hits
whenever we load, resize, or edit a page.

Purify output looks like:

[W] UMR: Uninitialized memory read in nsTextFragment::nsTextFragment(void) {76
occurrences}
        Reading 4 bytes from 0x0960a760 (4 bytes at 0x0960a760 uninitialized)
        Address 0x0960a760 is 56 bytes into a 68 byte block at 0x0960a728
        Address 0x0960a760 points to a C++ new block in heap 0x03ce0000
        Thread ID: 0x97
        Error location
            nsTextFragment::nsTextFragment(void) [nsTextFragment.h:61]
            nsGenericDOMDataNode::nsGenericDOMDataNode(void)
[nsGenericDOMDataNode.cpp:60]
            nsTextNode::nsTextNode(void) [nsTextNode.cpp:90]
            NS_NewTextNode(nsIContent * *) [nsTextNode.cpp:82]
            SinkContext::FlushText(int *) [nsHTMLContentSink.cpp:1507]
            SinkContext::OpenContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:994]
            HTMLContentSink::OpenContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:2211]
            CNavDTD::OpenContainer(nsIParserNode const&,int) [CNavDTD.cpp:2530]
            CNavDTD::HandleDefaultStartToken(CToken *,nsHTMLTag,nsIParserNode&)
[CNavDTD.cpp:1056]
            CNavDTD::HandleStartToken(CToken *) [CNavDTD.cpp:1308]
        Allocation location
            new(UINT)      [new.cpp:23]
            NS_NewTextNode(nsIContent * *) [nsTextNode.cpp:82]
            SinkContext::FlushText(int *) [nsHTMLContentSink.cpp:1507]
            SinkContext::OpenContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:994]
            HTMLContentSink::OpenContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:2211]
            CNavDTD::OpenContainer(nsIParserNode const&,int) [CNavDTD.cpp:2530]
            CNavDTD::HandleDefaultStartToken(CToken *,nsHTMLTag,nsIParserNode&)
[CNavDTD.cpp:1056]
            CNavDTD::HandleStartToken(CToken *) [CNavDTD.cpp:1308]
            NavDispatchTokenHandler [CNavDTD.cpp:241]
            CTokenHandler::()(CToken *,nsIDTD *) [nsTokenHandler.cpp:80]
[W] UMR: Uninitialized memory read in nsTextFragment::nsTextFragment(void) {110
occurrences}
        Reading 4 bytes from 0x096483c8 (4 bytes at 0x096483c8 uninitialized)
        Address 0x096483c8 is 56 bytes into a 68 byte block at 0x09648390
        Address 0x096483c8 points to a C++ new block in heap 0x03ce0000
        Thread ID: 0x97
        Error location
            nsTextFragment::nsTextFragment(void) [nsTextFragment.h:61]
            nsGenericDOMDataNode::nsGenericDOMDataNode(void)
[nsGenericDOMDataNode.cpp:60]
            nsTextNode::nsTextNode(void) [nsTextNode.cpp:90]
            NS_NewTextNode(nsIContent * *) [nsTextNode.cpp:82]
            SinkContext::FlushText(int *) [nsHTMLContentSink.cpp:1507]
            SinkContext::CloseContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:1073]
            HTMLContentSink::CloseContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:2226]
            CNavDTD::CloseContainer(nsIParserNode const&,nsHTMLTag,int)
[CNavDTD.cpp:2594]
            CNavDTD::CloseContainersTo(int,nsHTMLTag,int) [CNavDTD.cpp:2627]
            CNavDTD::CloseContainersTo(nsHTMLTag,int) [CNavDTD.cpp:2649]
        Allocation location
            new(UINT)      [new.cpp:23]
            NS_NewTextNode(nsIContent * *) [nsTextNode.cpp:82]
            SinkContext::FlushText(int *) [nsHTMLContentSink.cpp:1507]
            SinkContext::CloseContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:1073]
            HTMLContentSink::CloseContainer(nsIParserNode const&)
[nsHTMLContentSink.cpp:2226]
            CNavDTD::CloseContainer(nsIParserNode const&,nsHTMLTag,int)
[CNavDTD.cpp:2594]
            CNavDTD::CloseContainersTo(int,nsHTMLTag,int) [CNavDTD.cpp:2627]
            CNavDTD::CloseContainersTo(nsHTMLTag,int) [CNavDTD.cpp:2649]
            CNavDTD::HandleEndToken(CToken *) [CNavDTD.cpp:1498]
            NavDispatchTokenHandler [CNavDTD.cpp:243]

Comment 1

[Simon Fraser [no longer active]]

The block where we end up with this garbage string is in the first call to
aReflowState.rendContext->GetWidth
after the comment
          // Look ahead in the text-run and compute the final word
          // width, taking into account any style changes and stopping
          // at the first breakable point.

Comment 2

[kipp]

I need a really narrow test case, otherwise I'll just have to guess while code
reading...thanks!

Comment 3

[Simon Fraser [no longer active]]

This occurs several times in startup, when resizing the editor window showing the
default page and probably when reflowing most other pages. I'd be happy to debug
it with out; I can reproduce it when applying bold in the editor.

Comment 4

[kipp]

You must have sent me the wrong stack trace. the UMR in ***nsTextFragment*** is
a purify bug - it can't handle bitfields. I have changes in my tree that will
cause purify to stop whining about the UMR's...

Can you stick on the correct stack trace?

Comment 5

[Simon Fraser [no longer active]]

Mmm. In that case I don't have a stack trace for you, but my comments above
regarding the call to aReflowState.rendContext->GetWidth(), which were derived
from looking at things in the debugger, are still accurate.

Comment 6

[kipp]

Ok, I looked at the code reference you mentioned and I think I've found the bug.
Once the tree opens I'll check it in and reassign the bug back to you to verify
the fix...

Comment 7

[kipp]

The UMR noise has been fixed. The GetWidth issue has also been fixed, but I need
it verified...Hopefully simon can do that; if not just close it...

Comment 8

[Simon Fraser [no longer active]]

Yup, that fixed it. Thanks.

Comment 9

[Chris Petersen]

Based on Simon's comments, verified fixed in the Sept 23rd.