Bug 53604 – A deleted object should never been bound again

• Component: WebGL
• Status: RESOLVED FIXED

[Zhenyao Mo]

Currently if a deleted object is called in bind* or useProgram, it will succeed.

Instead, we should always replace the deleted object with null before caching them in the WebGLRenderingContext.

Otherwise, certain functions might crash, for example, generateMipmaps().

[Zhenyao Mo]

Created attachment 80959 [details]
Patch

[Zhenyao Mo]

Note that I will copy the changes in the test back to khronos once this is reviewed.

Also, this patch solves the crash in generateMipmap().

[Kenneth Russell]

Comment on attachment 80959 [details]
Patch

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

Looks good overall. Couple of minor nits.

> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:549
> +bool WebGLRenderingContext::checkToBeBoundObject(WebGLObject* object, bool* deleted)

Use bool& for deleted to match WebKit style and avoid the need for the ASSERT. Also, I think "checkObjectToBeBound" would be more grammatically correct.

[Zhenyao Mo]

(In reply to comment #3)
> (From update of attachment 80959 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80959&action=review
> 
> Looks good overall. Couple of minor nits.
> 
> > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:549
> > +bool WebGLRenderingContext::checkToBeBoundObject(WebGLObject* object, bool* deleted)
> 
> Use bool& for deleted to match WebKit style and avoid the need for the ASSERT. Also, I think "checkObjectToBeBound" would be more grammatically correct.

Will address these issues for the landing patch.  Thanks!

[Zhenyao Mo]

Committed r77417: <http://trac.webkit.org/changeset/77417>

[WebKit Review Bot]

http://trac.webkit.org/changeset/77417 might have broken Qt Windows 32-bit Release