123770 - xpconnect extensions to support SOAP

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[John Bandhauer]

I'll attach a patch that has some minor xpconnect changes to support the SOAP
code. This includes two areas of change:

1) Extend the interface info lookup to use the
nsIInterfaceInfoSuperManager/additionalManagers scheme. This means that
xpconnect will be able to wrap objects whose interface types are dynamically
generated; i.e. not found in typelibs. 

To support this I change nsXPConnect to get and hold the
nsIInterfaceInfoSuperManager interface on the interface info manager service.
This interface inherits from the one we previously used - so it exposes the old
stuff plus some new stuff. This allows us to look at additionalInterfaces
related methods without doing an extra QI each time. And, I funnel the
(approriate) existing calls of GetInfoForIID and GetInfoForName into a method in
nsXPConnect that will traverse the additionalInterfaces list if present.

2) I did work to expose the object we use as 'Components.interfaces' as a
component that can be created and initialize for use with a given specific
nsIInterfaceInfoManager. This allows us to expose the SOAP generated interfaces
from a proxy that works with those interfaces. This should produce no noticable
change to Components.interfaces. But still allow us to leverage the same code to
implement MyProxy.interfaces. This change included changing nsJSIID to hold a
reference on the interface info it represents. This is actually a small space
savings in the normal case. But it also ensures the lifetime of a given
interface info when that matters in the dynamic case.

Comment 1

[John Bandhauer]

Attachment: proposed fix. Could use more testing.

Comment 2

[John Bandhauer]

I've been running with this and running our various tests. I also did another
once over of the diff and it looks good to me. Looking for reviews.

Comment 3

[David Bradley]

Tried to apply the patch to get some context, looks like
nsIScriptableInterfaces.idl is missing. This isn't holding me up right now.

Comment 4

[John Bandhauer]

Attachment: updated patch

Oops. This one has the added .idl file and also includes the Mac build changes.
Note that this diff is based from mozilla/js because it includes a diff in
mozilla/js/macbuild.

Comment 5

[David Bradley]

Comment on attachment 69612 [details] [diff] [review]
updated patch

r=dbradley

Couple of questions as much for my education as anything. Is it worth checking
the return value from GetIIDShared calls. Other code is split between checking
and not. Same for GetInfoForName and GetInfoForIID. I know the current
implementations all return NS_OK. 

Also should:
clazz = new nsXPCWrappedJSClass(ccx, aIID, info);

be checked for null? It wasn't checked in the old code either.

Comment 6

[John Bandhauer]

> checked for null?
Sure, avoid crash on mem alloc failure:

                clazz = new nsXPCWrappedJSClass(ccx, aIID, info);
                if(clazz && !clazz->mDescriptors)
                    NS_RELEASE(clazz);  // sets clazz to nsnull

I'm not too worried about the GetIIDShared calls. I think we are coupled enough
with the xpti code to trust that the [shared] call can't fail. The few checks
here are a bit of a waste, but no big deal I think. 

GetInfoForName and GetInfoForIID are a different story. These can both fail.
AFAICT (and I checked) the code always checks the value of the 'info' out param.
The code trusts that the out param would not be set if the method failed. This
is normal stuff for an object getter.

Thanks.

Comment 7

[vidur (gone)]

Comment on attachment 69612 [details] [diff] [review]
updated patch

sr=vidur

Comment 8

[John Bandhauer]

patches checked in.

Comment 9

[Phil Schwartau]

Marking Verified -