Bug 213265 - [WebGL] Lose the context if IOSurface allocation fails
Summary: [WebGL] Lose the context if IOSurface allocation fails
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-06-16 15:35 PDT by Dean Jackson
Modified: 2020-06-16 17:58 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.67 KB, patch)
2020-06-16 15:38 PDT, Dean Jackson
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2020-06-16 15:35:15 PDT
If we can't create the backing store for WebGL, we should immediately lose the context.
Comment 1 Radar WebKit Bug Importer 2020-06-16 15:35:27 PDT
<rdar://problem/64424742>
Comment 2 Dean Jackson 2020-06-16 15:38:13 PDT
Created attachment 402050 [details]
Patch
Comment 3 Simon Fraser (smfr) 2020-06-16 15:41:32 PDT
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.
Comment 4 Dean Jackson 2020-06-16 16:28:56 PDT
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.
Comment 5 Dean Jackson 2020-06-16 17:58:05 PDT
Committed r263127: <https://trac.webkit.org/changeset/263127>