1432526 - Replace simple uses of NativeInterface2JSObject with ToJSValue

Status: RESOLVED WONTFIX

(Core :: XPConnect, enhancement, P2)

Description

[Andrew McCreight [:mccr8]]

NativeInterface2JSObject has a lot of options, but a couple of places don't use them, so they could just use ToJSValue.

Comment 1

[Andrew McCreight [:mccr8]]

Hopefully this is okay to do.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1432526 - Replace simple uses of NativeInterface2JSObject with ToJSValue.

ToJSValue is easier to understand.

Review commit: https://reviewboard.mozilla.org/r/214948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214948/

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8944783 [details]
Bug 1432526 - Replace simple uses of NativeInterface2JSObject with ToJSValue.

https://reviewboard.mozilla.org/r/214948/#review220618

::: js/xpconnect/src/XPCWrappedJSClass.cpp
(Diff revision 1)
>                              if (newThis) {
>                                  RootedValue v(cx);
> -                                xpcObjectHelper helper(newThis);
> +                                if (!ToJSValue(cx, newThis, &v))
> -                                bool ok =
> -                                  XPCConvert::NativeInterface2JSObject(
> -                                      &v, helper, nullptr, false, nullptr);

So this used to pass false for allowNativeWrapper but will start passing true.  That is, it will possibly start returning Xrays or other cross-compartment wrappers when it didn't use to.

That said, we then proceed to JS_WrapObject anyway, so it doesn't matter too much, I hope....

::: js/xpconnect/src/XPCWrappedNativeScope.cpp:202
(Diff revision 1)
>          mComponents = system ? new nsXPCComponents(this)
>                               : new nsXPCComponentsBase(this);
>      }
>  
>      RootedValue val(cx);
> -    xpcObjectHelper helper(mComponents);
> +    if (!ToJSValue(cx, mComponents, &val))

Similar here, but again then we proceed to JS_WrapObject... so ok.

Comment 4

[Andrew McCreight [:mccr8]]

Ehh I think I'll just leave this alone then... Thanks for the review anyways.