See hyatt's plan for use of this in XBL: news://news.mozilla.org/39EEBCCE.8090800%40netscape.com An email exchange... John Bandhauer wrote: > > David Hyatt wrote: > > > > Suppose we have an element that impls interfaces A and B. The XBL > > implements C. Let's say someone asks for C, so I do a WrapJS on it and > > make a wrapper and hand that back. > > > > My question then is this. If C inherits from the other interface B that > > my native object already implements, will the wrapper return itself when > > B is asked for because I already did a WrapJS for C? In other words, > > when you call WrapJS on an object, does it add only C to its list of > > known interfaces, or will it also add all interfaces C inherits from? > > > > I'm worrying about a problem where you could have a B ptr handed back by > > the native XUL element, you QI to C, which gives you back a ptr to the > > wrapper, and you then QI back to B and still get the wrapper. > > > > Tell me I'm ok! :) > > > > dave > > (hyatt@netscape > > Yeah that is a problem. The wrapper has a schecme to walk its own > inheritence tree looking for matches. I noticed last night that > this was a bit broken and I worked on fixing it. Current code is > at: > > http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcwrappedjsclass.c pp#303 > > I have a plan. xpcwrappedJS will get a new method called > aggregatedQueryInterface. > > the wrapper's QI will figure out up front if it is part of an > aggregation with you. If so it will delegate to you right off, > else it does its usual thing. If you decide in your QI to > delegate to it then you call aggregatedQI rather than QI and good > stuff happens. So, your QI always gets first shot and no call > cycles. > > I thought more about the whole scheme and I think we're going to > be OK. I'll add a nsIXPConnect::wrapJSAggregatedToNative > (or somesuch) and I'll mark the wrapper with a flag (rather than > go look at the JSObject's class flags each time I use it). The > wrapping process will be changed as we decided. The autowrap > process will not need to change at all because when autowrap > calls your QI on the native you can build a wrapper on the > JSObject and return it as appropriate. I think it will all just > work. > > There is the issue that these wrappers will not be doublewrapped > when passed back into JS. So, some xpconnect magic on the JS side > won't work; e.g. 'foo instanceof Components.interfaces.nsIFoo'. > That is the curse of the DOM object in xpconnect.
I have the xpconnect end mostly coded. I'm thinking about some things I may not have done 'right'... I set it up so that the wrapper does not need an owning reference on the native object. It gets that from the JSObject private when it needs to call it. I'm not sure I like that. The owning reference might be bad, but as it is the wrapper has to go through a bunch of stuff to get the native when it needs it. The wrapper chain is rooting the JSObject anyway. Would it be bad to have it also have an owning reference to the native object? This gives us yet another cycle in the 'live' case. But, when the native wants to die it can break the cycle by doing what it will already need to do: releasing its reference to the root wrapper. I'm pretty much convinced that the owning reference in the wrapper on the native is a good thing. I'm inclined to code that. Thoughts? Also, what I have so far assumes that you are never going to call nsIXPConnect::wrapJSAggregatedToNative for a JSObject that might also have been previously wrapped with a 'plain' wrapper. This should be OK since xpconnect autowrap stuff won't actually build a wrapper around a JSObject with a native private (i.e. the special flag set in the JSClass).
Created attachment 17578 [details] [diff] [review] second draft of fix (though of a stupid bug while writing post!)
I attached a draft patch. This is a first draft for hyatt to experiment with. I'm sure it needs cleanup. Make an aggregating wrapper with nsIXPConnect:wrapJSAggregatedToNative and pass in the nsISupports* of the native 'outer'. That outer will be called via delagation for all QI calls on the wrapper. The exception is that the native can call QI on the returned wrapper to ask for the nsIXPConnectWrappedJS interface without the wrapper delegating to the native (this allows the native to create the wrapper and *then* get the interface on the wrapper that it needs to use for aggregatedQueryInterface). The native can delegate QI calls to the wrapper's aggregatedQueryInterface method. If the wrapper's aggregatedQI method is asked for the nsISupports iface then it will return a pointer to the wrapper root. The native might find this useful. The native should not pass that on as the identify object for the aggregation; i.e. the native's normal QI handing should filter out requests for nsISupports (and other interfaces the native does natively) before delegating to the wrapper's aggregatedQueryInterface. This patch has the root wrapper of a wrapper chain holding an owning reference on the native 'outer'. Any attempt to autowrap a JSObject that is already part of one of these aggregations will follow the old path of detecting that the JSObject has an nsISupports private and just return the result of QI on that private. Since the native's QI is capable of determining which ifaces are supposed to result in wrapper creation then it can decide to call wrapJSAggregatedToNative or return NO_INTERFACE or whatever. We wind in a little deeper, but it works I think. Let me know how this works.
The third draft is the same as the second except it moves the new method on nsIXPConnect to the end of the interface rather than the middle so that the world does not need to be recompiled. I actually ran mozilla this time :)
Ok, cool! I'll play with this and let you know how it works.
I applied the patch (just now getting back to working on this) and it doesn't compile. xpcwrappedjs.cpp C:\gecko\mozilla\js\src\xpconnect\src\xpcwrappedjs.cpp(348) : error C2039: 'IsIID' : is not a member of 'nsDerivedSafe<class nsIInterfaceInfo>'
Never mind. I'm an idiot. It does compile.
a=hyatt. I'm ready for this, so get an r= when you can, and land this puppy. :)
fix checked in.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.