Last Comment Bug 705333 - Use IDL for nsJSCID::{CreateInstance,GetService}
: Use IDL for nsJSCID::{CreateInstance,GetService}
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla11
Assigned To: :Ms2ger (⌚ UTC+1/+2)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 705324
  Show dependency treegraph
Reported: 2011-11-25 10:10 PST by :Ms2ger (⌚ UTC+1/+2)
Modified: 2011-12-04 14:12 PST (History)
2 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (8.29 KB, patch)
2011-11-25 10:10 PST, :Ms2ger (⌚ UTC+1/+2)
bobbyholley: review+
Details | Diff | Splinter Review

Description User image :Ms2ger (⌚ UTC+1/+2) 2011-11-25 10:10:32 PST
Created attachment 576965 [details] [diff] [review]
Patch v1
Comment 1 User image Bobby Holley (:bholley) (busy with Stylo) 2011-12-01 12:28:49 PST
Comment on attachment 576965 [details] [diff] [review]
Patch v1

I'm really sorry for totally failing on my promise of speedy reviews. :-( Thanks for doing this stuff!

>-    nsISupports createInstance();
>-    nsISupports getService();
>+    [implicit_jscontext,optional_argc] jsval createInstance([optional] in jsval iid);
>+    [implicit_jscontext,optional_argc] jsval getService([optional] in jsval iid);

It's good that you're using jsvals here, because I just wrote a patch that makes XPConnect throw on null IIDs: bug 705875. ;-)

>-    rv = xpc->WrapNativeToJSVal(cx, obj, inst, nsnull, iid, true, vp, nsnull);
>-    if (NS_FAILED(rv) || JSVAL_IS_PRIMITIVE(*vp))
>+    rv = nsXPConnect::GetXPConnect()->WrapNativeToJSVal(aCx, obj, inst, nsnull, iid, true, aRetval, nsnull);
>+    if (NS_FAILED(rv) || JSVAL_IS_PRIMITIVE(*aRetval))

Can nsXPConnect::GetXPConnect() ever return null? We check for it before, and with this patch we don't. We should either keep checking or be very sure that it's safe. Same thing in getInterface().

r=bholley once that's figured out.
Comment 2 User image Bobby Holley (:bholley) (busy with Stylo) 2011-12-01 17:49:31 PST
So, I realized that as much as I like the aFoo naming convention, it isn't consistent with js-style, and XPConnect is supposed to be js-style. So r+ contingent on changing that as well. :-(
Comment 3 User image :Ms2ger (⌚ UTC+1/+2) 2011-12-04 14:12:01 PST
nsXPConnect::GetXPConnect() certainly can't be null at that point; we would already have returned when the helper failed. (I doubt it can be null otherwise; maybe on shutdown?)

Note You need to log in before you can comment on or make changes to this bug.