1148070 - Make nsIClassInfo::getHelperForLanguage specific to JavaScript

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

nsIClassInfo has this method:
  nsISupports getHelperForLanguage(in uint32_t language);

The comment says "For instance, if asked for the helper for nsIProgrammingLanguage::JAVASCRIPT this might return an object that can be QI'd into the nsIXPCScriptable interface to assist XPConnect in supplying JavaScript specific behavior to callers of the instance object."

In fact, this is the only use I can see for this method, in XPCWrappedNative::GatherProtoScriptableCreateInfo().  I think we should change it to:
  nsIXPCScriptable getXPCHelper();

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Change nsIClassInfo::getHelperForLanguage() to getXPCHelper().

This is a DOM class info method that is there for use by XPConnect, so I think bholley makes the most sense as a reviewer.

Anyways, the point of this patch is that we pass in this stupid language identifier, but it is only called with JS, in XPCWrappedNative, so it drops the language identifier argument.  I also changed the return type to be more precise, because again, it is always nsIXPCScriptable.  Finally, I changed the name so that it is a little more specific.

It is a large patch, but most of it is just changing the name and arguments for methods that return null.

The interesting-ish changes in XPCWrappedNative.cpp and nsIClassInfo.idl.  If there's some name you prefer over GetXPCHelper, I can change it by doing a search and replace on the patch.  I also wasn't sure if it was worth the bother to turn it into an attribute, implemented as a getter, now that it takes no arguments, so I just left it as a method.

There are some sort of interesting changes in these files, mostly just removing branching on JAVASCRIPT and modernizing the addref of return values in nsIClassInfoImpl.h, nsDOMClassInfo.cpp, XPCJSID.cpp, XPCRuntimeService.cpp, mozStorageAsyncStatement.cpp, and mozStorageStatement.cpp.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8f2a75f20f9

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8584808 [details] [diff] [review]
Change nsIClassInfo::getHelperForLanguage() to getXPCHelper().

Review of attachment 8584808 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/nsDOMClassInfo.cpp
@@ +829,5 @@
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> +nsDOMClassInfo::GetXPCHelper(nsIXPCScriptable **_retval)

This should be called GetScriptableHelper, I think, given the 'SH' naming convention in nsDOMClassInfo.cpp.

Comment 3

[Andrew McCreight [:mccr8]]

(In reply to Bobby Holley (:bholley) from comment #2)
> This should be called GetScriptableHelper, I think, given the 'SH' naming
> convention in nsDOMClassInfo.cpp.

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/41e157bfec1d

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/41e157bfec1d

Comment 5

[Andrew McCreight [:mccr8]]

I guess this could break addons, though I doubt anybody does anything aside from return null in JS.

Comment 6

[Andrew McCreight [:mccr8]]

I found an addon that this probably breaks, but it looks like it wasn't even updated for Firefox 4 so I think we're okay:
  https://addons.mozilla.org/en-US/firefox/addon/annoyance-remover/

Comment 7

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/33fabe9426abd39fb59ebec8d6bd52730b2676c0
Bug 1148070 - Change nsIClassInfo::getHelperForLanguage() to getScriptableHelper(). r=bholley