Replace NS_RUNTIMEABORT("OOM") with something better

RESOLVED FIXED in Firefox 39

Status

()

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: dmajor, Assigned: dmajor)

Tracking

unspecified
mozilla41
Points:
---

Firefox Tracking Flags

(firefox39 fixed, firefox40 fixed, firefox41 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

4 years ago
I see various instances of NS_RUNTIMEABORT("OOM") when people are unable to report a size for NS_ABORT_OOM(size_t). That's unfortunate because those NS_RUNTIMEABORTs don't get counted as OOMs by crash-stats.

At the very least those should be NS_ABORT_OOM(0), or some little helper that does the same. At least then it would show up on crash searches as "OOM | unknown | ..."

As a bonus fix, the NS_RUNTIMEABORT in ImageLoader::AddImage can be removed altogether, since the allocation that it's testing is already infallible (and reports a size).
Assignee

Updated

4 years ago
Keywords: leave-open
Assignee

Comment 2

4 years ago
This gets rid of most of the offenders, by funneling them through NS_ABORT_OOM(0).

I'll handle the others case-by-case.
Assignee: nobody → dmajor
Attachment #8608948 - Flags: review?(jst)
Attachment #8608948 - Flags: review?(jst) → review+
Assignee

Comment 6

4 years ago
Attachment #8610806 - Flags: review?(dbaron)
Assignee

Comment 7

4 years ago
Comment on attachment 8610806 [details] [diff] [review]
Cleanup in layout/

AFAICT the callees are all infallible (including the allocation in nsNullPrincipal::Create, but that one can also fail for other reasons, so I clarified the message).
Assignee

Comment 8

4 years ago
Attachment #8610809 - Flags: review?(bobbyholley)
Assignee

Comment 9

4 years ago
Attachment #8610810 - Flags: review?(bobbyholley)
Attachment #8610809 - Flags: review?(bobbyholley) → review+
Attachment #8610810 - Flags: review?(bobbyholley) → review+
Attachment #8610806 - Flags: review?(dbaron) → review+
Assignee

Updated

4 years ago
Keywords: leave-open
Assignee

Comment 12

4 years ago
Comment on attachment 8610809 [details] [diff] [review]
Cleanup in dom/

Requesting uplift to get better diagnostics for bug 1106264 comment 12.

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: poor crash information
[Describe test coverage new/current, TreeHerder]: landed on m-c a while back
[Risks and why]: trivial fix that only affects OOM codepaths
[String/UUID change made/needed]: none
Attachment #8610809 - Flags: approval-mozilla-beta?
Attachment #8610809 - Flags: approval-mozilla-aurora?
Comment on attachment 8610809 [details] [diff] [review]
Cleanup in dom/

Approved for uplift to aurora and beta - to help with crash diagnostics.
Attachment #8610809 - Flags: approval-mozilla-beta?
Attachment #8610809 - Flags: approval-mozilla-beta+
Attachment #8610809 - Flags: approval-mozilla-aurora?
Attachment #8610809 - Flags: approval-mozilla-aurora+
Assignee

Comment 14

4 years ago
(Setting these flags for the sake of the sheriff scripts)
Duplicate of this bug: 1048012
You need to log in before you can comment on or make changes to this bug.