Closed Bug 1167189 Opened 9 years ago Closed 9 years ago

Replace NS_RUNTIMEABORT("OOM") with something better

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: away, Assigned: away)

References

Details

Attachments

(4 files)

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).
Keywords: leave-open
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+
Attachment #8610806 - Flags: review?(dbaron)
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).
Attached patch Cleanup in dom/Splinter Review
Attachment #8610809 - Flags: review?(bobbyholley)
Attachment #8610810 - Flags: review?(bobbyholley)
Attachment #8610809 - Flags: review?(bobbyholley) → review+
Attachment #8610810 - Flags: review?(bobbyholley) → review+
Keywords: leave-open
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+
(Setting these flags for the sake of the sheriff scripts)
You need to log in before you can comment on or make changes to this bug.