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

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect, P1)

Description

[Boris Zbarsky [:bzbarsky]]

Silently returning a non-functional 2d context seems uncalled for.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't pretend everything is ok when allocating image surfaces failes.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 508682 [details] [diff] [review]
Don't pretend everything is ok when allocating image surfaces failes.

sold.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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.

Comment 4

[Joe Drew (not getting mail)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Joe Drew (not getting mail)]

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.

Comment 7

[Joe Drew (not getting mail)]

(I don't feel terribly strongly about it either way, though!)

Comment 8

[Boris Zbarsky [:bzbarsky]]

So you're proposing I just use some error code other than NS_ERROR_OUT_OF_MEMORY?

Comment 9

[Boris Zbarsky [:bzbarsky]]

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 10

[Joe Drew (not getting mail)]

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.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/ee0596d83d99