Currently if you ask xptcall to create a proxy for an interface, it will do so, even if the interface has [notxpcom] methods and cannot be implemented properly. NS_GetXPTCallSTub should just refuse to implement that.

Going to hold off on reviews until I have a try run: my browser starts, but I imagine this could cause havoc in weird places.

In addition to the XPCOM changes, when I was writing the test I discovered that XPConnect doesn't notice failure to create the xptcstub. This new version propagates that error correctly and now the test throws usefully. Appears to still work on try.

Comment on attachment 8492305 [details] [diff] [review] 1069518-xptcall-notxpcom Review of attachment 8492305 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the XPCOM bits with the added test below. ::: xpcom/tests/NotXPCOMTest.idl @@ +10,5 @@ > + void method1(); > +}; > + > +[scriptable, uuid(237d01a3-771e-4c6e-adf9-c97f9aab2950)] > +interface ScriptableWithNotXPCOM : nsISupports Can you add a test for an interface like: [scriptable, uuid(blah)] interface ScriptableButWithANotXPCOMParent : ScriptableWithNotXPCOM { void method3 (); }; ?

Comment on attachment 8492305 [details] [diff] [review] 1069518-xptcall-notxpcom Review of attachment 8492305 [details] [diff] [review]: ----------------------------------------------------------------- r=bholley on the XPConnect bits. ::: js/xpconnect/src/XPCWrappedJS.cpp @@ +346,5 @@ > JS::RootedObject rootJSObj(cx, clasp->GetRootJSObject(cx, jsObj)); > if (!rootJSObj) > return NS_ERROR_FAILURE; > > + nsresult rv; Given that this is only used as an out-param at the moment, I'd prefer it to be initialized with a known value. @@ +387,5 @@ > mClass(aClass), > mRoot(root ? root : MOZ_THIS_IN_INITIALIZER_LIST()), > mNext(nullptr) > { > + *rv = InitStub(GetClass()->GetIID()); Please add a comment saying that we need to press on here even in the case of failure to make sure that things are the right state when we get destroyed.

Updated to comments and to remove CRLF line endings.

When addon developer or XPCOM developer implements XPCOM by JS, it may not work by this fix. I think this should be marked by dev-doc-needed flag. If unnecessary, please clear this flag.

Indeed, this one broke the icaljs backend in Lightning. See bug 1081534. An update of documentation would be appreciated.

Also, is there a way to make logging for this type of error more obvious? All we get is this, without further explaination. Error: NS_ERROR_XPC_CI_RETURNED_FAILURE: Component returned failure code: 0x80570015 (NS_ERROR_XPC_CI_RETURNED_FAILURE) [nsIJSCID.createInstance]

(In reply to Philipp Kewisch [:Fallen] from comment #10) > Also, is there a way to make logging for this type of error more obvious? > All we get is this, without further explaination. > > Error: NS_ERROR_XPC_CI_RETURNED_FAILURE: Component returned failure code: > 0x80570015 (NS_ERROR_XPC_CI_RETURNED_FAILURE) [nsIJSCID.createInstance] What would you change about the logging that's already there: https://hg.mozilla.org/mozilla-central/rev/b41521572026#l3.30 ?

I don't think it should be restricted to debug builds.