1490507 - nsIXPCWrappedJSObjectGetter is unused

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement, P3)

Description

[Andrew McCreight [:mccr8]]

The interface nsIXPCWrappedJSObjectGetter appears to be completely unused, but there's a gigantic comment before it, in nsIXPConnect.idl, that sounds like it is possibly still relevant, although it talks about an interface  nsIXPCSecurityManager which does not exist any more. The comment itself does not seem to have been touched since 2001. There's a reference to this comment ("See the comment preceding nsIXPCWrappedJSObjectGetter in nsIXPConnect.idl.") before GetDoubleWrappedJSObject in XPCWrappedNativeJSOps.cpp.

Anyways, I would like to get rid of this interface, but I'm not sure if I should just move the comment to GetDoubleWrappedJSObject or what.

Comment 1

[Andrew McCreight [:mccr8]]

The big comment also contains the typo "implemementation".

Comment 2

[Andrew McCreight [:mccr8]]

Removing nsIXPCWrappedJSObjectGetter or making it not scriptable would save tens of bytes in binary size, by eliminating XPT data.

Comment 3

[Andrew McCreight [:mccr8]]

I'll just make it not scriptable in bug 1490503.

Comment 4

[Bobby Holley (:bholley)]

If the IID isn't referenced anywhere, I think the interface can just go away. A lot of the old security manager is gone now.

Comment 5

[Andrew McCreight [:mccr8]]

Yeah. The question is, what should I do with the comment, which seems about 75% relevant and 25% obsolete.

Comment 6

[Bobby Holley (:bholley)]

(In reply to Andrew McCreight [:mccr8] from comment #5)
> Yeah. The question is, what should I do with the comment, which seems about
> 75% relevant and 25% obsolete.

I'd clean it up and move it here: https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/js/xpconnect/src/XPCWrappedNativeJSOps.cpp#149

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Bug 1490507, part 1 - Remove nsIXPCWrappedJSObjectGetter and move the comment

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Bug 1490507, part 2 - Fix up the comment about double wrapping

Depends on 5690

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 9008491 [details]
Bug 1490507, part 1 - Remove nsIXPCWrappedJSObjectGetter and move the comment

Bobby Holley (:bholley) has approved the revision.

Comment 10

[Bobby Holley (:bholley)]

Comment on attachment 9008492 [details]
Bug 1490507, part 2 - Fix up the comment about double wrapping

Bobby Holley (:bholley) has approved the revision.

Comment 11

[Andrew McCreight [:mccr8]]

Boris, Bobby wanted me to ask if we expose XPCWJS to content at all these days. There's a bit of code at the end of XPC_WN_DoubleWrappedGetter that does a IsSystemCaller() check that I mentioned in my comment. Bobby wanted me to note it if these shouldn't be in content any more.

Comment 12

[Boris Zbarsky [:bzbarsky]]

See bug 1478359 comment 5 and bug 1478359 comment 6: XBL implements="" and its replacement via CustomElementRegistry::CallGetCustomInterface use XPCWrappedJS.  To the extent that we do this for elements not in chrome, we will have XPCWrappedJS for a non-chrome thing.

Now that said, the real question is whether we expose double-wrapped XPCWrappedJS to content, right?  The  answer there is bug 1207321 comment 2: Only if you do enablePrivilege or are a remote-XUL domain.  And we should really get rid of those...  Once we do that, I think this check can probably go away, since we won't have XPCWrapped_Native_ exposed to content.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0cbd91ad500b
part 1 - Remove nsIXPCWrappedJSObjectGetter and move the comment r=bholley
https://hg.mozilla.org/integration/autoland/rev/644d4b1846b0
part 2 - Fix up the comment about double wrapping r=bholley

Comment 14

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/0cbd91ad500b
https://hg.mozilla.org/mozilla-central/rev/644d4b1846b0