21643 - xpconnect root wrapper scheme is funky

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P3)

Description

[Chris Waterson]

...in a corner case where a "root" wrapper and a "real" wrapper need to be
constructed at the same time.

Comment 1

[Chris Waterson]

Attachment: sloppy fix

Comment 2

[Chris Waterson]

N.B. that the first two parts of the patch is irrelevant, and is simply meant to
more clearly balance the way that the root wrapper is released in the dtor. The
second part of the patch is the sauce.

Comment 3

[Scott MacGregor]

I *think* I found another leak here too. Although I'm not positive. In
xpconvert::JSData2Native, we make a call to GetNative which looks to me like it
ref counts the natvie object before it returns the native object. But there is
no appropriate release in this method.

I added a release in that code and our leak count went down by a little bit.
(Didn't solve the xul document leak though).

I'll attach another patch with this leak fix too. I'm hoping shaver or jband can
help verify that we do need to release the object returned by GetNative.

Comment 4

[John Bandhauer]

I'll look at waterson's patch.
I can see for sure that the case that mscott refers to in xpcconvert is not a
leak. This is calling nsXPCWrappedNative::GetNative with no param which is the
inlined method with no addref:
http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcprivate.h#822

Comment 5

[Chris Waterson]

jband: I'd like to land this fix, or something like it, when the tree opens.
what say you?

Comment 6

[John Bandhauer]

go for it

Comment 7

[Chris Waterson]

hacky fix checked in. jband, leaving this assigned to you as you were gonna
rewrite this all RSN, anyway.

Comment 8

[John Bandhauer]

changing bug title from...
  [MLK] XPConnect leaks native "root" wrapper
...to...
  xpconnect root wrapper scheme is funky

And, I removed the 'mlk' keyword 'cuz it doesn't leak anymore.

I'll revisit this later when this scheme is rewritten for flattening, etc.

Comment 9

[Gervase Markham [:gerv]]

This bug has not been touched for more than nine months. In most cases, that 
means it has "slipped through the net". Please could the owner take a moment to 
add a comment to the bug with current status, and/or close it.

Thank you :-)

Gerv

Comment 10

[John Bandhauer]

flattening changes in xpcdom branch will fix this.

Comment 11

[John Bandhauer]

fixed by XPCDOM_20010329_BRANCH landing

Comment 12

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/821ce1573461
[fenix] For https://github.com/mozilla-mobile/fenix/issues/21643: Pre-land strings for inactive tabs CFR. Update section title for normal, non-search group tabs. (https://github.com/mozilla-mobile/fenix/pull/21645)