284889 - OOM crash [@nsStyleContext::ApplyStyleFixups]

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel de Wildt]

GetUniqueStyleData could return nsnull is several cases (e.g. invalid
nsStyleStructID, OOM etc.). This is not handled in
nsStyleContext::ApplyStyleFixups which could result in a crash

Comment 1

[Boris Zbarsky [:bzbarsky]]

Hmmmm...  We really need this fixup -- if it doesn't happen, we'll crash in
various places in layout, iirc.  Maybe if getting the unique style data fails we
should just munge our non-unique data?

Comment 2

[Bastiaan Jacques]

Attachment: add null check

This patch adds a null check. I don't completely understand bz's comment, so
his suggested additional fix is lost upon me.

Comment 3

[Boris Zbarsky [:bzbarsky]]

My comment just says that with that patch we'll go on to crash elsewhere instead
of crashing here.

My suggestion is that if we fail to get a unique style data we should perhaps
modify |disp| (or rather a non-const version thereof) directly.  Possibly by
making GetUniqueStyleData never return null and return the non-unique data if it
can't allocate a new struct.

Comment 4

[Bastiaan Jacques]

Attachment: better fix

Comment 5

[Bastiaan Jacques]

Attachment: even better fix..

On second thought, we should probably return immediately.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 188120 [details] [diff] [review]
even better fix..

>+    NS_WARNING("Ran out of memory while trying to allocate memory for a unique nsStyleStruct! " \
>+               "Returning the non-unique data.");

No need for the backslash.  It may even be counterproductive.

Comment 7

[Bastiaan Jacques]

Attachment: patch for checkin

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 188120 [details] [diff] [review]
even better fix..

Requesting 1.8b4 approval for this OOM error handling fix.  Risk is quite low.

Comment 9

[Bastiaan Jacques]

Checked in by timeless (2005-07-19 14:03).