Bug 43735 – C++ DOM binding classes with private structs need operator= overload

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Kevin Ollivier]

C++ DOM binding classes which have a private struct that manages the WebCore pointer already have a copy constructor defined that passes the pointer properly, but they do not define operator= so outside of construction, assignment leads to a crash, since it does a simple copy of the private structure rather than creating a new one and assigning the pointer.

[Kevin Ollivier]

Created attachment 63912 [details]
Add operator= overloads to C++ DOM binding classes with private structs.

[Geoffrey Garen]

Are there cases where assignment is used like this? If so, you should be able to write a layout test.

If assignment is not used like this, I suggest just forbidding assignment, in the same way the Noncopyable class does.

[Geoffrey Garen]

Comment on attachment 63912 [details]
Add operator= overloads to C++ DOM binding classes with private structs.

r- because the new code in this patch does not include a test.

[Kevin Ollivier]

(In reply to comment #2)
> Are there cases where assignment is used like this? If so, you should be able to write a layout test.

Yes, I ran into this problem when generating Python wrappers for these bindings. The SWIG tool does something along these lines (code simplified to show the relevant bits):

WebDOMNode result;

{
    result = range->startContainer();
}
 
swigptr = new WebDOMNode(result);

The block assigning result has some extra error handling code, etc. which is why they isolate it in a code block.

As for the LayoutTest part, does WebDOM* have a test suite yet? If so I'll be happy to add a case for it.

> If assignment is not used like this, I suggest just forbidding assignment, in the same way the Noncopyable class does.

For a public API like the WebDOM* classes, I'm concerned that making the objects non-copyable would be pretty restrictive. (And, of course, it would really set me back on trying to get Python bindings working.)

[Geoffrey Garen]

Comment on attachment 63912 [details]
Add operator= overloads to C++ DOM binding classes with private structs.

Okeedokee.

I don't think WebDOM* has a test suite yet. That's a shame, but it's not the fault of this patch.

r=me

[Darin Adler]

Comment on attachment 63912 [details]
Add operator= overloads to C++ DOM binding classes with private structs.

You’ll need to update the test results checked in at the directory WebCore/bindings/cpp as well.

[Kevin Ollivier]

Created attachment 63974 [details]
Patch updated with bindings test outputs updated.

Thanks for catching the bindings test updates! I didn't realize these were in the tree. New patch up now with them added.

Note that there are a couple other changes (with the ENABLE(Conditional)) in one of the test headers as well, this is from a change in a previous commit (the C++ DOM bindings wasn't enclosing the function definitions with the conditional #if, only their implementations) and the results are correct.

[Adam Barth]

Comment on attachment 63974 [details]
Patch updated with bindings test outputs updated.

Forwarding ggaren's r+.

[WebKit Commit Bot]

Comment on attachment 63974 [details]
Patch updated with bindings test outputs updated.

Clearing flags on attachment: 63974

Committed r65147: <http://trac.webkit.org/changeset/65147>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.