Re: jsdIDebuggerService crashing FF 2.0

James Ross wrote:
> John J. Barton wrote:
>> So I guess that by un-wrapping the result from the frame.eval() I 
>> caused my caller grief because it was expecting to un-wrap.  Since I 
>> passed back an un-wrapped value, the caller crashed. (Kudos to venkman 
>> once again, the ultimate in example code).
>>
>> Or at least that is what I think now...
> 
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/jsd/idl/jsdIDebuggerService.idl&rev=1.33&mark=674-675#664 
> 
> 
> I hate to say RTFM but, well, it does say jsdIValue there. ;)
> 
> (Personally, I'd have thought XPConnect should shit itself long before 
> this got to JSD code, but I've also learnt to never trust XPConnect to 
> do its job right.)
> 

James,

XPConnect did its job exactly right here. The *real* bug here is in 
jsd_xpc.cpp. That bug ought to be fixed.

But first. Before spreading the notion that XPConnect is broken, you 
should try to understand what job it is doing. Part of the idea of 
XPConnect is that it allows JS coders to minimally implement xpcom 
interfaces. This means that it automatically builds a wrapper around a 
passed in JSObject without requiring that the object implement all 
methods of the (scriptable) xpcom interface. JavaScript is dynamic. 
The methods an object has at one point in time might not be the 
methods it has at another point in time. XPConnect allows the JS coder 
to 'claim' that a given JSObject implements a given scriptable xpcom 
interface. The coder makes that 'claim' by simply using the object in 
a context that requires an interface. It is that simple by design.

If at method call time a given method is found to not exist on the 
JSObject (or its inheritance chain) then XPConnect's stub for the 
interface method returns an error code (while properly handling all 
in, out, and in/out params). That is, XPConnect makes it so that the 
wrapper does in fact fully implement the xpcom interface as declared 
in xpidl.

There was some discussion early on about allowing for a way to declare 
in xpidl that a given interface was *callable* via script, but not 
*implementable* via script. Though that might have been useful in this 
case, we decided then that this was an unnecessary and potentially 
harmful complication.

I'm sorry that your expectations are incorrect. But, XPConnect worked 
exactly as designed here. The JS coder provided a JSObject in a place 
where an object implementing the jsdIValue interface was required. 
XPConnect then built a wrapper compliant with that interface.

The bug is in jsds_ExecutionHookProc's use of the returned value. See:

http://lxr.mozilla.org/seamonkey/source/js/jsd/jsd_xpc.cpp#688

It calls GetJSDValue() on the object but ignores the nsresult. As it 
happens, GetJSDValue maps to a [noscript] attribute on the interface, 
so XPConnect returns an error code to the caller.

But, that jsd_xpc code (incorrectly) ignores the nsresult and assumes 
that it got a useful answer from the method call. So, it goes ahead 
and calls JSD_GetValueWrappedJSVal with a garbage parameter. That is 
the bug. And it ought to be fixed.

As it happens, in a debug build of JSD, JSD_GetValueWrappedJSVal would 
do various validations of the passed in params. Though, in this case, 
the param passed in is not even pre-initialized.

Anyway, I suggest that you file a JSD bug and fix the real problem.

John.