1147005 - Change JSAddPropertyOp signature

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S [:evilpie]]

I want to split setProperty and addProperty, so might as well fix addProperty first.

Comment 1

[Tom S [:evilpie]]

Attachment: Remove mutable value parameter from JSAddPropertyOp

Comment 2

[Peter Van der Beken [:peterv]]

Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

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

::: js/xpconnect/idl/nsIXPCScriptable.idl
@@ +78,5 @@
>                       in JSObjectPtr globalObj, out JSObjectPtr parentObj);
>  
>      boolean addProperty(in nsIXPConnectWrappedNative wrapper,
>                         in JSContextPtr cx, in JSObjectPtr obj, in jsid id,
> +                       in jsval val);

Need to change the IID of this interface (mach update-uuids).

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

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

Nice. Scripted WANT_ADDPROPERTY users are a wildcard; maybe Peter knows more about them? Anyway, it's worth a shot.

::: dom/base/nsDOMClassInfo.cpp
@@ +911,5 @@
>  }
>  
>  NS_IMETHODIMP
>  nsDOMClassInfo::AddProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
> +                            JSObject *obj, jsid id, JS::Handle<JS::Value> val,

JS::HandleValue is in the public API and is used everywhere, right?

::: js/xpconnect/idl/nsIXPCScriptable.idl
@@ +78,5 @@
>                       in JSObjectPtr globalObj, out JSObjectPtr parentObj);
>  
>      boolean addProperty(in nsIXPConnectWrappedNative wrapper,
>                         in JSContextPtr cx, in JSObjectPtr obj, in jsid id,
> +                       in jsval val);

When you change an IDL file, you have to change this interface's UUID and the UUIDs of all other interfaces that mention it. 'mach update-uuids' can do this for you, I think.

Comment 4

[Jason Orendorff [:jorendorff]]

Comment on attachment 8582571 [details] [diff] [review]
Remove mutable value parameter from JSAddPropertyOp

Restoring peterv's review which I accidentally clobbered.

Comment 5

[Tom S [:evilpie]]

Gecko uses JS::Handle<JS::Value> and Peter already made me update the IDL.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bfe3954c2784
https://hg.mozilla.org/integration/mozilla-inbound/rev/d72144153749

Comment 6

[Phil Ringnalda (:philor)]

Sorry, sucks to be you. Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/055c19334b2e to get a clean backout of another patch to finally get a clean backout of bustage from Friday afternoon.

Comment 7

[Tom S [:evilpie]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c89c30a9b45f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c89c30a9b45f