Bug 59459 - fast/canvas/webgl/bad-arguments-test.html et al crashing since r84893
Summary: fast/canvas/webgl/bad-arguments-test.html et al crashing since r84893
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Adam Barth
Depends on: 59487
Blocks: 59659
  Show dependency treegraph
Reported: 2011-04-26 08:04 PDT by Stephen White
Modified: 2011-04-27 17:49 PDT (History)
9 users (show)

See Also:

Patch (1.97 KB, patch)
2011-04-26 08:34 PDT, Stephen White
kbr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Adam Barth 2011-04-26 08:26:26 PDT
I'm pretty sure the change is correct.  There are likely bugs elsewhere causing the problem.  However, we can revert and investigate.
Comment 2 Stephen White 2011-04-26 08:34:00 PDT
Created attachment 91111 [details]
Comment 3 Kenneth Russell 2011-04-26 08:35:00 PDT
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.
Comment 4 Kenneth Russell 2011-04-26 08:35:25 PDT
Comment on attachment 91111 [details]

Yes, let's revert this change until we understand what's going on.
Comment 5 Stephen White 2011-04-26 08:37:08 PDT
Committed r84916: <http://trac.webkit.org/changeset/84916>