xpconnect extensions to support SOAP

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
XPConnect
VERIFIED FIXED
17 years ago
11 years ago

People

(Reporter: John Bandhauer, Assigned: John Bandhauer)

Tracking

Trunk
mozilla0.9.9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

17 years ago
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.
(Assignee)

Comment 1

17 years ago
Created attachment 68102 [details] [diff] [review]
proposed fix. Could use more testing.
(Assignee)

Comment 2

17 years ago
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.
Status: NEW → ASSIGNED

Comment 3

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

Comment 4

17 years ago
Created attachment 69612 [details] [diff] [review]
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.
Attachment #68102 - Attachment is obsolete: true

Comment 5

17 years ago
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.
Attachment #69612 - Flags: review+
(Assignee)

Comment 6

17 years ago
> 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.
Target Milestone: --- → mozilla0.9.9

Comment 7

17 years ago
Comment on attachment 69612 [details] [diff] [review]
updated patch

sr=vidur
Attachment #69612 - Flags: superreview+
(Assignee)

Comment 8

17 years ago
patches checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 9

17 years ago
Marking Verified - 
Status: RESOLVED → VERIFIED

Updated

11 years ago
Blocks: 387648
You need to log in before you can comment on or make changes to this bug.