942528 - Make XPCWrappedNative into a more normal CCed class

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Andrew McCreight [:mccr8]]

Attachment: xpcwn.patch

PCWN and XPCWJS are the only two remaining CCed C++ things that don't use the purple buffer (and thus snow white).  I'm abusing the purple buffer for ICC, so right now I have to reinvent these mechanisms, poorly, for XPCWN and XPCWJS.  But maybe I can just make these classes into normal CCed classes, and then I wouldn't have to do anything more, while at the same time reducing overall weirdness.  I have some patches that sort of work, but not entirely.

Comment 1

[Andrew McCreight [:mccr8]]

The XPCWN stuff is fairly green.  I hit some crashiness that might be fixable by immediately removing the XPCWN from the global table when it hits a refcount of 0.  There's also a problem that looks like my patch is tripping some kind of lock detection when we run about:memory.

https://tbpl.mozilla.org/?tree=Try&rev=456ae933a061

XPCWJS is stable enough to run the browser for a bit, but it fails at least one test ( content/base/test/chrome/test_bug549682.xul ) which looks like it involves a weakly-held XPCWJS going away too quickly, which I could believe.  There may be other problems, too.  I don't understand why XPCWJS has to addref itself twice for weak references.  That code is from 2001, so who knows if it is really needed.  Though if you just remove it, bad things happen, I discovered.

Comment 2

[Andrew McCreight [:mccr8]]

If I'm reading this right, weak references to WPCJS are actually strong, due to the extra addref that only gets dropped when there are no weak references.

Comment 3

[Andrew McCreight [:mccr8]]

I have a patch that works for XPCWN, though it does
  NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(XPCWrappedNative, Destroy())
which is kind of cheating...
  https://tbpl.mozilla.org/?tree=Try&rev=546c6a77723a

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Make XPCWN more of a normal cycle collected class. r=smaug

Comment 5

[Andrew McCreight [:mccr8]]

Try run looks green: https://tbpl.mozilla.org/?tree=Try&rev=23fbf78872d2

I'm going to split XPCWJS into bug 944492.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8340086 [details] [diff] [review]
Make XPCWN more of a normal cycle collected class. r=smaug

>+NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(XPCWrappedNative, Destroy())
Could you add a comment here why we want _LAST_RELEASE + Destroy()

Comment 7

[Andrew McCreight [:mccr8]]

Comment added.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7330220c26d

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f7330220c26d