RESOLVED FIXED 53604
A deleted object should never been bound again
https://bugs.webkit.org/show_bug.cgi?id=53604
Summary A deleted object should never been bound again
Zhenyao Mo
Reported 2011-02-02 10:29:57 PST
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().
Attachments
Patch (19.83 KB, patch)
2011-02-02 13:25 PST, Zhenyao Mo
kbr: review+
Zhenyao Mo
Comment 1 2011-02-02 13:25:23 PST
Zhenyao Mo
Comment 2 2011-02-02 13:26:13 PST
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 3 2011-02-02 13:44:40 PST
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
Comment 4 2011-02-02 13:59:23 PST
(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
Comment 5 2011-02-02 14:14:11 PST
WebKit Review Bot
Comment 6 2011-02-02 14:44:11 PST
http://trac.webkit.org/changeset/77417 might have broken Qt Windows 32-bit Release
Note You need to log in before you can comment on or make changes to this bug.