Open Bug 620279 Opened 9 years ago Updated 2 years ago

Assume infallible NS_NewListControlFrame in nsCSSFrameConstructor::ConstructSelectFrame

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

ASSIGNED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

3037 nsCSSFrameConstructor::ConstructSelectFrame(nsFrameConstructorState& aState,

this can fail:
3154       nsIFrame* listFrame = NS_NewListControlFrame(mPresShell, styleContext);

handled here:
3155       if (listFrame) {

passed carelessly here:
3168       InitializeSelectFrame(aState, listFrame, scrolledFrame, content,

3185 nsCSSFrameConstructor::InitializeSelectFrame(nsFrameConstructorState& aState,
3186                                              nsIFrame*                scrollFrame,

crashes here:
3205   scrollFrame->Init(aContent, geometricParent, nsnull);
Attached patch proposal (obsolete) — Splinter Review
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #498680 - Flags: review?(dbaron)
Attachment #498680 - Flags: approval2.0?
Comment on attachment 498680 [details] [diff] [review]
proposal

Failing to destroy listFrame properly could turn this from the safe null-dereference crash that it is now into something worse.  If we really want to handle OOM here, I think we need proper calls to destruction of partially-created frames.  I'm not sure what the state of the art is there, though, since it's changed a bit recently.

That said, I think we should just make stuff like this infallible.
Attachment #498680 - Flags: review?(dbaron)
Attachment #498680 - Flags: review-
Attachment #498680 - Flags: approval2.0?
Assignee: timeless → nobody
> this can fail:

If it can, that's a bug.  That should be an infallible allocation, imo.
Severity: critical → enhancement
Keywords: crash
Summary: [@ nsCSSFrameConstructor::InitializeSelectFrame | nsCSSFrameConstructor::ConstructSelectFrame] when NS_NewListControlFrame fails → Assume infallible NS_NewListControlFrame in nsCSSFrameConstructor::ConstructSelectFrame
Attached patch proposalSplinter Review
Assignee: nobody → timeless
Attachment #498680 - Attachment is obsolete: true
Attachment #502444 - Flags: review?(dbaron)
Comment on attachment 502444 [details] [diff] [review]
proposal

This is still a ways from being infallible; the pres arena is not infallible, though it probably should be.
Attachment #502444 - Flags: review?(dbaron) → review-
You need to log in before you can comment on or make changes to this bug.