1329846 - Remove XPCNativeScriptableInfo

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Nicholas Nethercote [inactive]]

XPCScriptableInfo is now a very thin wrapper around nsIXPCScriptable. It can be removed.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Remove XPCNativeScriptableInfo

PCNativeScriptableInfo is now a very thin wrapper around nsIXPCScriptable, and
it uses manual memory management. Removing it simplifies things quite a bit.

In particular, when setting XPCWrappedNative::mScriptable in
XPCWrappedNative::WrapNewGlobal() and XPCWrappedNative::Init() we no longer
have to worry about sharing the XPCNativeScriptableInfo object with the proto.
And XPCWrappedNative::{Init,Destroy}() have similar simplifications.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Remove NATIVE_HAS_FLAG

It's only used in three places, and it no longer makes the code more readable.

Comment 3

[Nicholas Nethercote [inactive]]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa8982cbd9fe6f4f4c8d6106e72574a6c86f1ea1

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 8825606 [details] [diff] [review]
(part 1) - Remove XPCNativeScriptableInfo

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

::: js/xpconnect/src/XPCCallContext.cpp
@@ +71,5 @@
>                                 XPC_WN_TEAROFF_FLAT_OBJECT_SLOT).toObject());
>      }
>      if (mWrapper) {
>          if (mTearOff)
> +            mScriptable = nullptr;

This case isn't needed, because mScriptable is a COM ptr. You should be able to simplify this code a little. (Something like if (mWrapper && !mTearOff))

::: js/xpconnect/src/XPCWrappedNative.cpp
@@ +535,5 @@
>  XPCWrappedNative::XPCWrappedNative(already_AddRefed<nsISupports>&& aIdentity,
>                                     XPCWrappedNativeProto* aProto)
>      : mMaybeProto(aProto),
>        mSet(aProto->GetSet()),
> +      mScriptable()

Nit: remove this initalizer.

@@ +553,5 @@
>                                     already_AddRefed<XPCNativeSet>&& aSet)
>  
>      : mMaybeScope(TagScope(aScope)),
>        mSet(aSet),
> +      mScriptable()

Nit: remove this initalizer.

@@ +572,5 @@
>  
>  void
>  XPCWrappedNative::Destroy()
>  {
> +    mScriptable = nullptr;

Hooray for simplifications!

@@ +926,5 @@
>  
>      if (HasProto())
>          proto->SystemIsBeingShutDown();
>  
> +    // We don't null mScriptable here. The destructor will do it.

"null" seems a little odd as a verb to me. Maybe "clear"?

::: js/xpconnect/src/XPCWrappedNativeProto.cpp
@@ +21,5 @@
>      : mScope(Scope),
>        mJSProtoObject(nullptr),
>        mClassInfo(ClassInfo),
>        mSet(Set),
> +      mScriptable()

Nit: remove this initalizer.

@@ +54,5 @@
>  XPCWrappedNativeProto::Init(const XPCNativeScriptableCreateInfo* scriptableCreateInfo,
>                              bool callPostCreatePrototype)
>  {
>      AutoJSContext cx;
>      nsIXPCScriptable* callback = scriptableCreateInfo ?

Maybe make this an nsCOMPtr to be consistent? and you could then use forget() below.

@@ +57,5 @@
>      AutoJSContext cx;
>      nsIXPCScriptable* callback = scriptableCreateInfo ?
>                                   scriptableCreateInfo->GetCallback() :
>                                   nullptr;
>      if (callback) {

nit: no braces

Comment 5

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/105d7762f4b06db69cef6fffd2129890dd9fbbb0
Bug 1329846 (part 1) - Remove XPCNativeScriptableInfo. r=mccr8.

https://hg.mozilla.org/integration/mozilla-inbound/rev/18ff0b6063121d502dd610560512ff91f46c97e9
Bug 1329846 (part 2) - Remove NATIVE_HAS_FLAG. r=mccr8.

Comment 6

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a289c64e5ed498911b141187666b7f45f4a98738
Bug 1329846 (follow-up) - Fix bustage. r=bustage.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/105d7762f4b0
https://hg.mozilla.org/mozilla-central/rev/18ff0b606312
https://hg.mozilla.org/mozilla-central/rev/a289c64e5ed4