573209 - nsDisplayListBuilder should not be NS_STACK_CLASS

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jesse Ruderman]

nsDisplayListBuilder is NS_STACK_CLASS
http://hg.mozilla.org/mozilla-central/annotate/5f3f36fe8d76/layout/base/nsDisplayList.h#l121

RangePaintInfo has a nsDisplayListBuilder member
http://hg.mozilla.org/mozilla-central/annotate/5f3f36fe8d76/layout/base/nsPresShell.cpp#l242

RangePaintInfo is heap-allocated
http://hg.mozilla.org/mozilla-central/annotate/5f3f36fe8d76/layout/base/nsPresShell.cpp#l5506

Dunno how bad this is.  Curious why static analysis didn't prevent this.  Noticed it while trying to track down a leak.

Comment 1

[Benjamin Smedberg]

Static analysis didn't prevent this because we haven't had a static analysis build running in many months. Ehren is working on resurrecting them as part of the regular debug builds.

nsDisplayListBuilder was NS_STACK_CLASS because at the time I checked it was only heap-allocated and that helped with XPCOMGC. Since XPCOMGC is dead, assuming that the automatic allocation is intentional we should just remove the NS_STACK_CLASS annotation.

Comment 2

[Jesse Ruderman]

I guess I'll also remove the private operator new that prevented direct heap instantiation (added by bz in bug 324969).

Comment 3

[Jesse Ruderman]

Attachment: patch

Remove NS_STACK_CLASS, remove throwing new, add trace-refcnt instrumentation to ensure we notice leaks.

Comment 4

[Jesse Ruderman]

http://hg.mozilla.org/mozilla-central/rev/30dc53f4f745

Comment 5

[Boris Zbarsky [:bzbarsky]]

Do we expect anyone to directly heap-allocate this?  If no, should we keep the private operator new?