Bug 213265 – [WebGL] Lose the context if IOSurface allocation fails

• Component: WebGL
• Status: RESOLVED FIXED

[Dean Jackson]

If we can't create the backing store for WebGL, we should immediately lose the context.

[Radar WebKit Bug Importer]

<rdar://problem/64424742>

[Dean Jackson]

Created attachment 402050 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 402050 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402050&action=review

> Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:221
> +    if (!allocateIOSurfaceBackingStore(IntSize(width, height))) {

Weird that we have size above then break it out into width/height then put it back to size here.

> Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:224
> +        return true;

What does the true mean? Shouldn't this be false? Or should it be an enum?

> Source/WebCore/platform/graphics/opengl/GraphicsContextGLOpenGLBase.cpp:195
> +    if (!allocateIOSurfaceBackingStore(IntSize(width, height))) {
> +        RELEASE_LOG(WebGL, "Fatal: Unable to allocate backing store of size %d x %d", width, height);
> +        forceContextLost();
> +        return true;

Ditto.

[Dean Jackson]

Comment on attachment 402050 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=402050&action=review

>> Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:221
>> +    if (!allocateIOSurfaceBackingStore(IntSize(width, height))) {
> 
> Weird that we have size above then break it out into width/height then put it back to size here.

Yeah.

>> Source/WebCore/platform/graphics/angle/GraphicsContextGLANGLE.cpp:224
>> +        return true;
> 
> What does the true mean? Shouldn't this be false? Or should it be an enum?

This should be in a separate cleanup. It indicates whether or not to restore the FBO bindings, so unrelated to allocation.

Given that we're losing the context here, it doesn't matter what we return.

[Dean Jackson]

Committed r263127: <https://trac.webkit.org/changeset/263127>