getContext("2d") should throw if allocating the backing store fails

RESOLVED FIXED in mozilla2.0b12

Status

()

P1
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

Trunk
mozilla2.0b12
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Silently returning a non-functional 2d context seems uncalled for.
Created attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.
Attachment #508682 - Flags: review?(vladimir)
Whiteboard: [need review]
Comment on attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.

sold.
Attachment #508682 - Flags: review?(vladimir) → review+
Whiteboard: [need review] → [need approval]
Comment on attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.

This just makes us throw on OOM when setting up a 2d context, instead of silently returning a broken context.  Should help avoid memory-exhaustion crashes if people create lots of 2d contexts.
Attachment #508682 - Flags: approval2.0?
Comment on attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.

Wouldn't it be better for us to return a null context in this situation?
Seems like it would be better to throw at the failure point with a clear message, rather than whenever the caller tries to access the 2d context for the first time, no?

Or did you mean throw _and_ return null?  The return value should be ignored if we throw anyway.
I could see either throwing or returning null. The reason I suggest returning null is because that's what WebGL does if there are problems allocating the surfaces, AIUI.
(I don't feel terribly strongly about it either way, though!)
So you're proposing I just use some error code other than NS_ERROR_OUT_OF_MEMORY?
Ah, no.  So as far as I can see, WebGL doesn't do what comment 6 says...  It throws NS_ERROR_FAILURE on surface allocation failures, and this gets thrown to web script.
Comment on attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.

I don't know where I got the idea that WebGL returns a null context on failure. Reading the relevant code, WebGLContext::SetDimensions, that clearly isn't true.
Attachment #508682 - Flags: approval2.0? → approval2.0+
Whiteboard: [need approval] → [need landing]
Pushed http://hg.mozilla.org/mozilla-central/rev/ee0596d83d99
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [need landing]
Target Milestone: --- → mozilla2.0b12
You need to log in before you can comment on or make changes to this bug.