Closed Bug 865477 Opened 8 years ago Closed 8 years ago
Block Frame::Reflow should use Maybe<> for stack-allocation, instead of ns Auto Ptr<> & heap allocation, for lazily-constructed ns HTMLReflow State
Right now, nsBlockFrame::Reflow has a variable 'mutableReflowState' which is heap-allocated because we don't initially know whether we'll need to construct it or not: > 977 nsAutoPtr<nsHTMLReflowState> mutableReflowState; http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp?rev=abc416eac56f#977 This is the *only* place in our code where we have a nsAutoPtr<nsHTMLReflowState>, FWIW. Maybe<> is much better-suited for this situation. Its header-comment says it's a "Small utility for lazily constructing objects without using dynamic storage". We also already use it for this same purpose in one other spot -- in nsLineLayout::ReflowFrame: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsLineLayout.cpp?rev=32503b49921e#769 Plus, reflow states are inherently stack-based objects (they're generally declared as local-variables, used for the duration of a Reflow call, and then discarded), so it's much nicer to have them just live on the stack. So: Let's definitely switch this to Maybe<>.
Summary: nsBlockFrame::Reflow should use Maybe<> instead of heap for possibly-allocated nsHTMLReflowState → nsBlockFrame::Reflow should use Maybe<> instead of heap for lazily-constructed nsHTMLReflowState
I haven't tested this vigorously yet, but it compiles and I think it should do the trick.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #741544 - Flags: review?(matspal)
Try run (Linux64-only, debug & opt, reftests & mochitests): https://tbpl.mozilla.org/?tree=Try&rev=93312673af54
Summary: nsBlockFrame::Reflow should use Maybe<> instead of heap for lazily-constructed nsHTMLReflowState → nsBlockFrame::Reflow should use Maybe<> for stack-allocation, instead of nsAutoPtr<> & heap allocation, for lazily-constructed nsHTMLReflowState
OS: Linux → All
Hardware: x86_64 → All
Actually, I'll tag bz for review to make sure he's in the loop on this change, since he added this variable in http://hg.mozilla.org/mozilla-central/rev/6f0219923a28#l1.36 for bug 626395.
Depends on: 626395
Attachment #741544 - Flags: review?(matspal) → review?(bzbarsky)
[Try run's green, woot!]
Comment on attachment 741544 [details] [diff] [review] fix v1 r=me
Attachment #741544 - Flags: review?(bzbarsky) → review+
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.