Bug 59459 – fast/canvas/webgl/bad-arguments-test.html et al crashing since r84893

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Stephen White]

Many WebGL tests are crashing; seems to have started at http://trac.webkit.org/changeset/84893/

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20chromium.org&tests=fast%2Fcanvas%2Fwebgl%2Fbad-arguments-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fbuffer-bind-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fbuffer-data-array-buffer.html%2Cfast%2Fcanvas%2Fwebgl%2Fcanvas-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fconstants.html%2Cfast%2Fcanvas%2Fwebgl%2Fcontext-attributes-alpha-depth-stencil-antialias.html%2Cfast%2Fcanvas%2Fwebgl%2Fcontext-destroyed-crash.html%2Cfast%2Fcanvas%2Fwebgl%2Fcontext-lost-restored.html%2Cfast%2Fcanvas%2Fwebgl%2Fcontext-lost.html%2Cfast%2Fcanvas%2Fwebgl%2Fcopy-tex-image-and-sub-image-2d.html%2Cfast%2Fcanvas%2Fwebgl%2Fcss-webkit-canvas-repaint.html%2Cfast%2Fcanvas%2Fwebgl%2Fcss-webkit-canvas.html%2Cfast%2Fcanvas%2Fwebgl%2Fdraw-arrays-out-of-bounds.html%2Cfast%2Fcanvas%2Fwebgl%2Fdraw-elements-out-of-bounds.html%2Cfast%2Fcanvas%2Fwebgl%2Ferror-reporting.html%2Cfast%2Fcanvas%2Fwebgl%2Fframebuffer-object-attachment.html%2Cfast%2Fcanvas%2Fwebgl%2Fframebuffer-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fget-active-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fgl-bind-attrib-location-test.html%2Cfast%2Fcanvas%2Fwebgl%2Fgl-enable-enum-test.html

[Adam Barth]

I'm pretty sure the change is correct.  There are likely bugs elsewhere causing the problem.  However, we can revert and investigate.

[Stephen White]

Created attachment 91111 [details]
Patch

[Kenneth Russell]

Something is clearly wrong with the OwnPtr cleanup in http://trac.webkit.org/changeset/84893/ . The only thing I can think of is that the assignment of OwnPtr<T> to OwnPtr<T> is broken. Looking at the OwnPtr and PassOwnPtr headers I do not see how this assignment is handled by these classes. There exists:

OwnPtr& operator=(const PassOwnPtr<T>&);

but I don't see a cast operator from OwnPtr<T> to PassOwnPtr<T>, nor a copy constructor on the PassOwnPtr class which takes OwnPtr. Therefore I think this assignment is performing a member-by-member assignment, and when the right hand side goes out of scope, the pointed-to object is deleted.

I think the code requires a .release() on the pointer on the right-hand side.

If this analysis is correct, then all of the strict OwnPtr cleanups made yesterday are broken and should be reverted. Additionally, the assignment operator "OwnPtr& operator=(const OwnPtr<T>&);" should explicitly be made private in OwnPtr.

[Kenneth Russell]

Comment on attachment 91111 [details]
Patch

Yes, let's revert this change until we understand what's going on.

[Stephen White]

Committed r84916: <http://trac.webkit.org/changeset/84916>