We should remove WebGLBuffer from list[1] to prevent the delete order problem. [1] https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLBuffer.cpp#66

Remove the WebGLBuffer from the list in mContext.

Comment on attachment 8764810 [details] [diff] [review] remove WebGLBuffer from list Review of attachment 8764810 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLBuffer.cpp @@ +62,5 @@ > mContext->MakeContextCurrent(); > mContext->gl->fDeleteBuffers(1, &mGLName); > mByteLength = 0; > mCache = nullptr; > + LinkedListElement<WebGLBuffer>::removeFrom(mContext->mBuffers); removeFrom() just has a slightly different assert from remove(), then just calls remove(), so this should have no functional change.

(In reply to Jeff Gilbert [:jgilbert] from comment #2) > Comment on attachment 8764810 [details] [diff] [review] > remove WebGLBuffer from list > > Review of attachment 8764810 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/canvas/WebGLBuffer.cpp > @@ +62,5 @@ > > mContext->MakeContextCurrent(); > > mContext->gl->fDeleteBuffers(1, &mGLName); > > mByteLength = 0; > > mCache = nullptr; > > + LinkedListElement<WebGLBuffer>::removeFrom(mContext->mBuffers); > > removeFrom() just has a slightly different assert from remove(), then just > calls remove(), so this should have no functional change. Right, I think we should take it as refactoring. It would be more clear to use removeFrom and all other places in WebGL use removeFrom now[1]. I'll change the title of this bug. [1] https://dxr.mozilla.org/mozilla-central/search?q=removeFrom+path%3Acanvas&redirect=false

(In reply to Ethan Lin[:ethlin] from comment #3) > (In reply to Jeff Gilbert [:jgilbert] from comment #2) > > Comment on attachment 8764810 [details] [diff] [review] > > remove WebGLBuffer from list > > > > Review of attachment 8764810 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > > ::: dom/canvas/WebGLBuffer.cpp > > @@ +62,5 @@ > > > mContext->MakeContextCurrent(); > > > mContext->gl->fDeleteBuffers(1, &mGLName); > > > mByteLength = 0; > > > mCache = nullptr; > > > + LinkedListElement<WebGLBuffer>::removeFrom(mContext->mBuffers); > > > > removeFrom() just has a slightly different assert from remove(), then just > > calls remove(), so this should have no functional change. > > Right, I think we should take it as refactoring. It would be more clear to > use removeFrom and all other places in WebGL use removeFrom now[1]. I'll > change the title of this bug. > > [1] > https://dxr.mozilla.org/mozilla-central/ > search?q=removeFrom+path%3Acanvas&redirect=false :jgilbert, do you think it's okay to do the refactoring? I think at least it's using the same way as other WebGL objects to do the remove, though the change is small.

(In reply to Ethan Lin[:ethlin] from comment #4) > (In reply to Ethan Lin[:ethlin] from comment #3) > > (In reply to Jeff Gilbert [:jgilbert] from comment #2) > > > Comment on attachment 8764810 [details] [diff] [review] > > > remove WebGLBuffer from list > > > > > > Review of attachment 8764810 [details] [diff] [review]: > > > ----------------------------------------------------------------- > > > > > > ::: dom/canvas/WebGLBuffer.cpp > > > @@ +62,5 @@ > > > > mContext->MakeContextCurrent(); > > > > mContext->gl->fDeleteBuffers(1, &mGLName); > > > > mByteLength = 0; > > > > mCache = nullptr; > > > > + LinkedListElement<WebGLBuffer>::removeFrom(mContext->mBuffers); > > > > > > removeFrom() just has a slightly different assert from remove(), then just > > > calls remove(), so this should have no functional change. > > > > Right, I think we should take it as refactoring. It would be more clear to > > use removeFrom and all other places in WebGL use removeFrom now[1]. I'll > > change the title of this bug. > > > > [1] > > https://dxr.mozilla.org/mozilla-central/ > > search?q=removeFrom+path%3Acanvas&redirect=false > :jgilbert, do you think it's okay to do the refactoring? I think at least > it's using the same way as other WebGL objects to do the remove, though the > change is small. I don't think we should bother, since there is only one LinkedList these could be added to, so the assert should not be useful to us. I think it's a little more paranoid than we need.