925916 - Handlify various things in XPConnect

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

Ok, I went a little crazy here.

Hazard:

Function 'uint32 xpc_qsUnwrapArg_HTMLElement(JSContext*, JS::Value, nsIAtom*, nsIContent**, nsISupports**, JS::Value*)' has unrooted 'val' of type 'JS::Value' live across GC call 'uint8 nsIContent::IsHTML(nsIAtom*) const' at js/xpconnect/src/nsDOMQS.h:170

I switched *UnwrapArg* stuff to Handles, and then proceeded to add Handles to everything else necessary for that. Which turned out to be a lot of stuf.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Handlify various things in XPConnect

The quickstubs changes are kind of hacky, I know.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: imported patch handle-dombindings

Collateral damage from xpconnect handlification. The WebIDL codegen changes were gratifyingly simple.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: imported patch handle-dom-base

More collateral damage. I can dump this review on someone else if you'd like, but I think it was mostly dull stuff.

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: imported patch handle-content

Collateral damage from some xpconnect Handles. I'm hoping you can review all this assortment.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 816067 [details] [diff] [review]
imported patch handle-dombindings

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

::: dom/bindings/BindingUtils.cpp
@@ -327,2 @@
>  
> -  JS::Rooted<JSObject*> obj(cx, JS_THIS_OBJECT(cx, vp));

This is wrong. You should replace this with args.computeThis(aCx).

@@ +672,5 @@
>  
>  bool
>  NativeInterface2JSObjectAndThrowIfFailed(JSContext* aCx,
> +                                         JS::HandleObject aScope,
> +                                         JS::MutableHandleValue aRetval,

Don't use the typedefs in dom.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 816067 [details] [diff] [review]
imported patch handle-dombindings

>-  if (!obj) {
>+  if (args.isConstructing()) {

Why is this a sensible thing to do?

>+  if (js::GetObjectJSClass(&args.thisv().toObject()) != clasp) {

Nothing above here guarantees args.thisv().isObject() is true.

>+                                         JS::HandleObject aScope,

Please don't change stuff to the typedefs piecemeal.  If you want to change all of dom/bindings to the typedefs, I think that's ok, as a separate patch with no other changes in it.

Similar for the rest of this patch.

r=me with those issues fixed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Why is this a sensible thing to do?

In particular, I think the sane test here is:

  if (!args.thisv().isObject()) {

Comment 8

[Steve Fink [:sfink] [:s:]]

Attachment: Handlify WrapNative

Cleaned up patch (removed unwanted typedef -> templates, moved wanted ones to a separate cleanup patch.)

Comment 9

[Steve Fink [:sfink] [:s:]]

Attachment: Random minor code cleanups

Random minor code cleanups. Made templates-vs-typedefs more consistent. Switched from using jsval -> JS::Value API.

Comment 10

[Steve Fink [:sfink] [:s:]]

Attachment: Handlify WrapNative in content/

Cleaned up patch.

Comment 11

[Bobby Holley (:bholley)]

Comment on attachment 816066 [details] [diff] [review]
Handlify various things in XPConnect

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

::: js/xpconnect/src/Sandbox.cpp
@@ +185,5 @@
>  
>  static bool
>  CreateXMLHttpRequest(JSContext *cx, unsigned argc, jsval *vp)
>  {
> +    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);

For any .cpp file: |using namespace JS;| and kill the prefixing, please. Here and elsewhere in this patch.

::: js/xpconnect/src/qsgen.py
@@ +515,2 @@
>          else:
> +            f.write("    JS::RootedValue {name}_dummy(cx);\n".format(name=name))

This stuff isn't great, but QS is going away on the soon side, so I think it's fine. Please add some comments here explaining what's going on, though.

::: js/xpconnect/wrappers/XrayWrapper.cpp
@@ +1407,5 @@
>      // Scope Polluter and thus the resulting properties are non-|own|. However,
>      // we're set up (above) to cache (on the holder) anything that comes out of
>      // resolveNativeProperty, which we don't want for something dynamic like
>      // named access. So we just handle it separately here.
> +    nsGlobalWindow *win = nullptr; // stupid gcc warning

No need for the comment, since the resulting code looks natural and isn't something someone is likely to try to undo.

Comment 12

[Tom S [:evilpie]]

Comment on attachment 816190 [details] [diff] [review]
Random minor code cleanups

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

::: dom/base/nsDOMClassInfo.cpp
@@ +3364,5 @@
>  static bool
>  LocationSetterUnwrapper(JSContext *cx, JS::Handle<JSObject*> obj_, JS::Handle<jsid> id,
>                          bool strict, JS::MutableHandle<JS::Value> vp)
>  {
> +  JS::Rooted<JSObject*> obj(cx, obj_);

obj_ is already a handle.

Comment 13

[Steve Fink [:sfink] [:s:]]

(In reply to Tom Schuster [:evilpie] from comment #12)
> Comment on attachment 816190 [details] [diff] [review]
> Random minor code cleanups
> 
> Review of attachment 816190 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/base/nsDOMClassInfo.cpp
> @@ +3364,5 @@
> >  static bool
> >  LocationSetterUnwrapper(JSContext *cx, JS::Handle<JSObject*> obj_, JS::Handle<jsid> id,
> >                          bool strict, JS::MutableHandle<JS::Value> vp)
> >  {
> > +  JS::Rooted<JSObject*> obj(cx, obj_);
> 
> obj_ is already a handle.

Sadly, that's not good enough. The following code is:

  JSObject *wrapped = XPCWrapper::UnsafeUnwrapSecurityWrapper(obj);
  if (wrapped) {
    obj = wrapped;
  }

You can't do that with a Handle, so we need a copy.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 816191 [details] [diff] [review]
Handlify WrapNative in content/

content/ and dom/ both use the same coding style.

If we want to start using typedefs everywhere, please do it in a separate
bug.

Assuming you de-typedefify this, r+

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 816189 [details] [diff] [review]
Handlify WrapNative

># HG changeset patch
># User Steve Fink <[e-mail]>
># Date 1381554159 25200
># Node ID 1b6b7cedeee7a0418c3956bee7d4c5fb8b29d46e
># Parent  476ab9d5aaa10daeba90cdf6f50cd79b6f3f8f09
>Handlify WrapNative
>
>diff --git a/dom/base/MessagePort.cpp b/dom/base/MessagePort.cpp
>--- a/dom/base/MessagePort.cpp
>+++ b/dom/base/MessagePort.cpp
>@@ -116,21 +116,21 @@ PostMessageReadStructuredClone(JSContext
>   if (tag == SCTAG_DOM_BLOB || tag == SCTAG_DOM_FILELIST) {
>     NS_ASSERTION(!data, "Data should be empty");
> 
>     nsISupports* supports;
>     if (JS_ReadBytes(reader, &supports, sizeof(supports))) {
>       JS::Rooted<JSObject*> global(cx, JS::CurrentGlobalOrNull(cx));
>       if (global) {
>         JS::Rooted<JS::Value> val(cx);
>         nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;
>         if (NS_SUCCEEDED(nsContentUtils::WrapNative(cx, global, supports,
>-                                                    val.address(),
>+                                                    &val,
>                                                     getter_AddRefs(wrapper)))) {
>           return JSVAL_TO_OBJECT(val);
>         }
>       }
>     }
>   }
> 
>   if (tag == SCTAG_DOM_MESSAGEPORT) {
>     NS_ASSERTION(!data, "Data should be empty");
> 
>diff --git a/dom/base/Navigator.cpp b/dom/base/Navigator.cpp
>--- a/dom/base/Navigator.cpp
>+++ b/dom/base/Navigator.cpp
>@@ -1582,21 +1582,21 @@ Navigator::DoNewResolve(JSContext* aCx, 
>     }
> 
>     rv = gpi->Init(mWindow, prop_val.address());
>     if (NS_FAILED(rv)) {
>       return Throw(aCx, rv);
>     }
>   }
> 
>   if (JSVAL_IS_PRIMITIVE(prop_val) && !JSVAL_IS_NULL(prop_val)) {
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>-    rv = nsContentUtils::WrapNative(aCx, aObject, native, prop_val.address(),
>+    rv = nsContentUtils::WrapNative(aCx, aObject, native, &prop_val,
>                                     getter_AddRefs(holder), true);
> 
>     if (NS_FAILED(rv)) {
>       return Throw(aCx, rv);
>     }
>   }
> 
>   if (!JS_WrapValue(aCx, prop_val.address())) {
>     return Throw(aCx, NS_ERROR_UNEXPECTED);
>   }
>diff --git a/dom/base/nsDOMClassInfo.cpp b/dom/base/nsDOMClassInfo.cpp
>--- a/dom/base/nsDOMClassInfo.cpp
>+++ b/dom/base/nsDOMClassInfo.cpp
>@@ -636,68 +636,69 @@ IdToString(JSContext *cx, jsid id)
>   if (JSID_IS_STRING(id))
>     return JSID_TO_STRING(id);
>   jsval idval;
>   if (!::JS_IdToValue(cx, id, &idval))
>     return nullptr;
>   return JS_ValueToString(cx, idval);
> }
> 
> static inline nsresult
> WrapNative(JSContext *cx, JSObject *scope, nsISupports *native,
>-           nsWrapperCache *cache, const nsIID* aIID, jsval *vp,
>+           nsWrapperCache *cache, const nsIID* aIID, JS::MutableHandle<JS::Value> vp,
>            nsIXPConnectJSObjectHolder** aHolder, bool aAllowWrapping)
> {
>   if (!native) {
>     NS_ASSERTION(!aHolder || !*aHolder, "*aHolder should be null!");
> 
>-    *vp = JSVAL_NULL;
>+    vp.setNull();
> 
>     return NS_OK;
>   }
> 
>   JSObject *wrapper = xpc_FastGetCachedWrapper(cache, scope, vp);
>   if (wrapper) {
>     return NS_OK;
>   }
> 
>   return nsDOMClassInfo::XPConnect()->WrapNativeToJSVal(cx, scope, native,
>                                                         cache, aIID,
>-                                                        aAllowWrapping, vp,
>+                                                        aAllowWrapping, vp.address(),
>                                                         aHolder);
> }
> 
> static inline nsresult
> WrapNative(JSContext *cx, JSObject *scope, nsISupports *native,
>-           const nsIID* aIID, bool aAllowWrapping, jsval *vp,
>+           const nsIID* aIID, bool aAllowWrapping, JS::MutableHandle<JS::Value> vp,
>            // If non-null aHolder will keep the jsval alive
>            // while there's a ref to it
>            nsIXPConnectJSObjectHolder** aHolder = nullptr)
> {
>   return WrapNative(cx, scope, native, nullptr, aIID, vp, aHolder,
>                     aAllowWrapping);
> }
> 
> // Same as the WrapNative above, but use these if aIID is nsISupports' IID.
> static inline nsresult
> WrapNative(JSContext *cx, JSObject *scope, nsISupports *native,
>-           bool aAllowWrapping, jsval *vp,
>+           bool aAllowWrapping, JS::MutableHandle<JS::Value> vp,
>            // If non-null aHolder will keep the jsval alive
>            // while there's a ref to it
>            nsIXPConnectJSObjectHolder** aHolder = nullptr)
> {
>   return WrapNative(cx, scope, native, nullptr, nullptr, vp, aHolder,
>                     aAllowWrapping);
> }
> 
> static inline nsresult
> WrapNative(JSContext *cx, JSObject *scope, nsISupports *native,
>-           nsWrapperCache *cache, bool aAllowWrapping, jsval *vp,
>+           nsWrapperCache *cache, bool aAllowWrapping,
>+           JS::MutableHandle<JS::Value> vp,
>            // If non-null aHolder will keep the jsval alive
>            // while there's a ref to it
>            nsIXPConnectJSObjectHolder** aHolder = nullptr)
> {
>   return WrapNative(cx, scope, native, cache, nullptr, vp, aHolder,
>                     aAllowWrapping);
> }
> 
> // Helper to handle torn-down inner windows.
> static inline nsresult
>@@ -2214,50 +2215,49 @@ BaseStubConstructor(nsIWeakReference* aW
>           !funval.isObject()) {
>         return NS_ERROR_UNEXPECTED;
>       }
> 
>       // Check if the object is even callable.
>       NS_ENSURE_STATE(JS_ObjectIsCallable(cx, &funval.toObject()));
>       {
>         // wrap parameters in the target compartment
>         // we also pass in the calling window as the first argument
>         unsigned argc = args.length() + 1;
>-        nsAutoArrayPtr<JS::Value> argv(new JS::Value[argc]);
>-        JS::AutoArrayRooter rooter(cx, 0, argv);
>+        JS::AutoValueVector argv(cx);
>+        argv.resize(argc);
> 
>         nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>         nsCOMPtr<nsIDOMWindow> currentWin(do_GetInterface(currentInner));
>         rv = WrapNative(cx, obj, currentWin, &NS_GET_IID(nsIDOMWindow),
>-                        true, &argv[0], getter_AddRefs(holder));
>+                        true, argv.handleAt(0), getter_AddRefs(holder));
>         if (!JS_WrapValue(cx, &argv[0]))
>           return NS_ERROR_FAILURE;
>-        rooter.changeLength(1);
> 
>         for (size_t i = 1; i < argc; ++i) {
>           argv[i] = args[i - 1];
>           if (!JS_WrapValue(cx, &argv[i]))
>             return NS_ERROR_FAILURE;
>-          rooter.changeLength(i + 1);
>         }
> 
>         JS::Rooted<JS::Value> frval(cx);
>-        bool ret = JS_CallFunctionValue(cx, thisObject, funval, argc, argv,
>+        bool ret = JS_CallFunctionValue(cx, thisObject, funval,
>+                                        argc, argv.begin(),
>                                         frval.address());
> 
>         if (!ret) {
>           return NS_ERROR_FAILURE;
>         }
>       }
>     }
>   }
> 
>-  return WrapNative(cx, obj, native, true, args.rval().address());
>+  return WrapNative(cx, obj, native, true, args.rval());
> }
> 
> static nsresult
> DefineInterfaceConstants(JSContext *cx, JS::Handle<JSObject*> obj, const nsIID *aIID)
> {
>   nsCOMPtr<nsIInterfaceInfoManager>
>     iim(do_GetService(NS_INTERFACEINFOMANAGER_SERVICE_CONTRACTID));
>   NS_ENSURE_TRUE(iim, NS_ERROR_UNEXPECTED);
> 
>   nsCOMPtr<nsIInterfaceInfo> if_info;
>@@ -2811,21 +2811,21 @@ ResolvePrototype(nsIXPConnect *aXPConnec
> 
>   nsRefPtr<nsDOMConstructor> constructor;
>   nsresult rv = nsDOMConstructor::Create(name, ci_data, name_struct, aWin,
>                                          getter_AddRefs(constructor));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>   JS::Rooted<JS::Value> v(cx);
> 
>   rv = WrapNative(cx, obj, constructor, &NS_GET_IID(nsIDOMDOMConstructor),
>-                  false, v.address(), getter_AddRefs(holder));
>+                  false, &v, getter_AddRefs(holder));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   if (install) {
>     rv = constructor->Install(cx, obj, v);
>     NS_ENSURE_SUCCESS(rv, rv);
>   }
> 
>   JS::Rooted<JSObject*> class_obj(cx, holder->GetJSObject());
>   NS_ASSERTION(class_obj, "The return value lied");
> 
>@@ -3093,21 +3093,21 @@ nsWindowSH::GlobalResolve(nsGlobalWindow
>     rv = nsDOMConstructor::Create(class_name,
>                                   nullptr,
>                                   name_struct,
>                                   static_cast<nsPIDOMWindow*>(aWin),
>                                   getter_AddRefs(constructor));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>     JS::Rooted<JS::Value> v(cx);
>     rv = WrapNative(cx, obj, constructor, &NS_GET_IID(nsIDOMDOMConstructor),
>-                    false, v.address(), getter_AddRefs(holder));
>+                    false, &v, getter_AddRefs(holder));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     rv = constructor->Install(cx, obj, v);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     JS::Rooted<JSObject*> class_obj(cx, holder->GetJSObject());
>     NS_ASSERTION(class_obj, "The return value lied");
> 
>     // ... and define the constants from the DOM interface on that
>     // constructor object.
>@@ -3195,21 +3195,21 @@ nsWindowSH::GlobalResolve(nsGlobalWindow
>   if (name_struct->mType == nsGlobalNameStruct::eTypeExternalConstructor) {
>     nsRefPtr<nsDOMConstructor> constructor;
>     rv = nsDOMConstructor::Create(class_name, nullptr, name_struct,
>                                   static_cast<nsPIDOMWindow*>(aWin),
>                                   getter_AddRefs(constructor));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     JS::Rooted<JS::Value> val(cx);
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>     rv = WrapNative(cx, obj, constructor, &NS_GET_IID(nsIDOMDOMConstructor),
>-                    false, val.address(), getter_AddRefs(holder));
>+                    false, &val, getter_AddRefs(holder));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     rv = constructor->Install(cx, obj, val);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     NS_ASSERTION(holder->GetJSObject(), "Why didn't we get a JSObject?");
> 
>     *did_resolve = true;
> 
>     return NS_OK;
>@@ -3244,21 +3244,21 @@ nsWindowSH::GlobalResolve(nsGlobalWindow
> 
>       if (aWin->IsOuterWindow()) {
>         nsGlobalWindow *inner = aWin->GetCurrentInnerWindowInternal();
>         NS_ENSURE_TRUE(inner, NS_ERROR_UNEXPECTED);
> 
>         scope = inner->GetGlobalJSObject();
>       } else {
>         scope = aWin->GetGlobalJSObject();
>       }
> 
>-      rv = WrapNative(cx, scope, native, true, prop_val.address(),
>+      rv = WrapNative(cx, scope, native, true, &prop_val,
>                       getter_AddRefs(holder));
>     }
> 
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     if (!JS_WrapValue(cx, prop_val.address())) {
>       return NS_ERROR_UNEXPECTED;
>     }
> 
>     bool ok = ::JS_DefinePropertyById(cx, obj, id, prop_val,
>@@ -3296,40 +3296,40 @@ ContentWindowGetter(JSContext *cx, unsig
>     return false;
> 
>   JS::Rooted<JS::Value> value(cx);
>   bool result = ::JS_GetProperty(cx, obj, "content", &value);
>   *vp = value;
>   return result;
> }
> 
> template<class Interface>
> static nsresult
>-LocationSetterGuts(JSContext *cx, JSObject *obj, jsval *vp)
>+LocationSetterGuts(JSContext *cx, JSObject *obj, JS::MutableHandle<JS::Value> vp)
> {
>   // This function duplicates some of the logic in XPC_WN_HelperSetProperty
>   obj = js::CheckedUnwrap(obj, /* stopAtOuter = */ false);
>   if (!IS_WN_REFLECTOR(obj))
>       return NS_ERROR_XPC_BAD_CONVERT_JS;
>   XPCWrappedNative *wrapper = XPCWrappedNative::Get(obj);
> 
>   // The error checks duplicate code in THROW_AND_RETURN_IF_BAD_WRAPPER
>   NS_ENSURE_TRUE(!wrapper || wrapper->IsValid(), NS_ERROR_XPC_HAS_BEEN_SHUTDOWN);
> 
>   nsCOMPtr<Interface> xpcomObj = do_QueryWrappedNative(wrapper, obj);
>   NS_ENSURE_TRUE(xpcomObj, NS_ERROR_UNEXPECTED);
> 
>   nsCOMPtr<nsIDOMLocation> location;
>   nsresult rv = xpcomObj->GetLocation(getter_AddRefs(location));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   // Grab the value we're being set to before we stomp on |vp|
>-  JS::Rooted<JSString*> val(cx, ::JS_ValueToString(cx, *vp));
>+  JS::Rooted<JSString*> val(cx, ::JS_ValueToString(cx, vp));
>   NS_ENSURE_TRUE(val, NS_ERROR_UNEXPECTED);
> 
>   // Make sure |val| stays alive below
>   JS::Anchor<JSString *> anchor(val);
> 
>   // We have to wrap location into vp before null-checking location, to
>   // avoid assigning the wrong thing into the slot.
>   nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>   rv = WrapNative(cx, JS::CurrentGlobalOrNull(cx), location,
>                   &NS_GET_IID(nsIDOMLocation), true, vp,
>@@ -3345,21 +3345,21 @@ LocationSetterGuts(JSContext *cx, JSObje
>   NS_ENSURE_TRUE(depStr.init(cx, val), NS_ERROR_UNEXPECTED);
> 
>   return location->SetHref(depStr);
> }
> 
> template<class Interface>
> static bool
> LocationSetter(JSContext *cx, JS::Handle<JSObject*> obj, JS::Handle<jsid> id, bool strict,
>                JS::MutableHandle<JS::Value> vp)
> {
>-  nsresult rv = LocationSetterGuts<Interface>(cx, obj, vp.address());
>+  nsresult rv = LocationSetterGuts<Interface>(cx, obj, vp);
>   if (NS_FAILED(rv)) {
>     xpc::Throw(cx, rv);
>     return false;
>   }
> 
>   return true;
> }
> 
> static bool
> LocationSetterUnwrapper(JSContext *cx, JS::Handle<JSObject*> obj_, JS::Handle<jsid> id,
>@@ -3558,21 +3558,21 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
>     nsCOMPtr<nsIDOMLocation> location;
>     nsresult rv = win->GetLocation(getter_AddRefs(location));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     // Make sure we wrap the location object in the window's scope.
>     JS::Rooted<JSObject*> scope(cx, wrapper->GetJSObject());
> 
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>     JS::Rooted<JS::Value> v(cx);
>     rv = WrapNative(cx, scope, location, &NS_GET_IID(nsIDOMLocation), true,
>-                    v.address(), getter_AddRefs(holder));
>+                    &v, getter_AddRefs(holder));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     bool ok = JS_WrapValue(cx, v.address()) &&
>                 JS_DefinePropertyById(cx, obj, id, v, JS_PropertyStub,
>                                       LocationSetterUnwrapper,
>                                       JSPROP_PERMANENT | JSPROP_ENUMERATE);
> 
>     if (!ok) {
>       return NS_ERROR_FAILURE;
>     }
>@@ -3583,21 +3583,21 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
>   }
> 
>   if (sTop_id == id) {
>     nsCOMPtr<nsIDOMWindow> top;
>     nsresult rv = win->GetScriptableTop(getter_AddRefs(top));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     JS::Rooted<JS::Value> v(cx);
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>     rv = WrapNative(cx, obj, top, &NS_GET_IID(nsIDOMWindow), true,
>-                    v.address(), getter_AddRefs(holder));
>+                    &v, getter_AddRefs(holder));
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     // Hold on to the top window object as a global property so we
>     // don't need to worry about losing expando properties etc.
>     if (!JS_DefinePropertyById(cx, obj, id, v, JS_PropertyStub, JS_StrictPropertyStub,
>                                JSPROP_READONLY | JSPROP_PERMANENT |
>                                JSPROP_ENUMERATE)) {
>       return NS_ERROR_FAILURE;
>     }
>     *objp = obj;
>@@ -3672,21 +3672,21 @@ nsWindowSH::NewResolve(nsIXPConnectWrapp
>       *objp = obj;
> 
>       return NS_OK;
>     }
>   } else {
>     if (sDocument_id == id) {
>       nsCOMPtr<nsIDocument> document = win->GetDoc();
>       JS::Rooted<JS::Value> v(cx);
>       nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>       rv = WrapNative(cx, JS::CurrentGlobalOrNull(cx), document, document,
>-                      &NS_GET_IID(nsIDOMDocument), v.address(), getter_AddRefs(holder),
>+                      &NS_GET_IID(nsIDOMDocument), &v, getter_AddRefs(holder),
>                       false);
>       NS_ENSURE_SUCCESS(rv, rv);
> 
>       // The PostCreate hook for the document will handle defining the
>       // property
>       *objp = obj;
> 
>       // NB: We need to do this for any Xray wrapper.
>       if (xpc::WrapperFactory::IsXrayWrapper(obj)) {
>         // Unless our object is a native wrapper, in which case we have to
>@@ -3979,23 +3979,25 @@ nsArraySH::GetProperty(nsIXPConnectWrapp
> 
>     // Make sure rv == NS_OK here, so GetItemAt implementations that never fail
>     // don't have to set rv.
>     rv = NS_OK;
>     nsWrapperCache *cache = nullptr;
>     nsISupports* array_item =
>       GetItemAt(GetNative(wrapper, obj), n, &cache, &rv);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     if (array_item) {
>+      JS::Rooted<JS::Value> rval(cx);
>       rv = WrapNative(cx, JS::CurrentGlobalOrNull(cx), array_item, cache,
>-                      true, vp);
>+                      true, &rval);
>       NS_ENSURE_SUCCESS(rv, rv);
>+      *vp = rval;
> 
>       rv = NS_SUCCESS_I_DID_SOMETHING;
>     }
>   }
> 
>   return rv;
> }
> 
> 
> // StringList scriptable helper
>@@ -4078,21 +4080,21 @@ nsHTMLDocumentSH::GetDocumentAllNodeList
> 
>     nsRefPtr<nsContentList> list =
>       domdoc->GetElementsByTagName(NS_LITERAL_STRING("*"));
>     if (!list) {
>       rv = NS_ERROR_OUT_OF_MEMORY;
>     }
> 
>     nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
>     nsresult tmp = WrapNative(cx, JS::CurrentGlobalOrNull(cx),
>                               static_cast<nsINodeList*>(list), list, false,
>-                              collection.address(), getter_AddRefs(holder));
>+                              &collection, getter_AddRefs(holder));
>     if (NS_FAILED(tmp)) {
>       rv = tmp;
>     }
> 
>     list.forget(nodeList);
> 
>     // ... and store it in our reserved slot.
>     JS_SetReservedSlot(obj, 0, collection);
>   }
> 
>@@ -4182,21 +4184,21 @@ nsHTMLDocumentSH::DocumentAllGetProperty
> 
>     nsIContent *node = nodeList->Item(JSID_TO_INT(id));
> 
>     result = node;
>     cache = node;
>   } else {
>     result = nullptr;
>   }
> 
>   if (result) {
>-    rv = WrapNative(cx, JS::CurrentGlobalOrNull(cx), result, cache, true, vp.address());
>+    rv = WrapNative(cx, JS::CurrentGlobalOrNull(cx), result, cache, true, vp);
>     if (NS_FAILED(rv)) {
>       xpc::Throw(cx, rv);
> 
>       return false;
>     }
>   } else {
>     vp.setUndefined();
>   }
> 
>   return true;
>@@ -4319,22 +4321,24 @@ nsStringArraySH::GetProperty(nsIXPConnec
>   nsAutoString val;
> 
>   nsresult rv = GetStringAt(GetNative(wrapper, obj), n, val);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   if (DOMStringIsNull(val)) {
>     *vp = JSVAL_VOID;
>     return NS_SUCCESS_I_DID_SOMETHING;
>   }
> 
>-  NS_ENSURE_TRUE(xpc::NonVoidStringToJsval(cx, val, vp),
>+  JS::Rooted<JS::Value> rval(cx);
>+  NS_ENSURE_TRUE(xpc::NonVoidStringToJsval(cx, val, &rval),
>                  NS_ERROR_OUT_OF_MEMORY);
>+  *vp = rval;
>   return NS_SUCCESS_I_DID_SOMETHING;
> }
> 
> 
> // MediaList helper
> 
> nsresult
> nsMediaListSH::GetStringAt(nsISupports *aNative, int32_t aIndex,
>                            nsAString& aResult)
> {
>diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp
>--- a/dom/base/nsGlobalWindow.cpp
>+++ b/dom/base/nsGlobalWindow.cpp
>@@ -3625,22 +3625,25 @@ NS_IMETHODIMP
> nsGlobalWindow::GetScriptableContent(JSContext* aCx, JS::Value* aVal)
> {
>   nsCOMPtr<nsIDOMWindow> content;
>   nsresult rv = GetContent(getter_AddRefs(content));
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   if (content || !nsContentUtils::IsCallerChrome() || !IsChromeWindow()) {
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     if (content && global) {
>       nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;
>-      return nsContentUtils::WrapNative(aCx, global, content, aVal,
>-                                        getter_AddRefs(wrapper));
>+      JS::Rooted<JS::Value> rval(aCx);
>+      nsresult rv = nsContentUtils::WrapNative(aCx, global, content, &rval,
>+                                               getter_AddRefs(wrapper));
>+      *aVal = rval;
>+      return rv;
>     }
>     return NS_ERROR_FAILURE;
>   }
> 
>   // Something tries to get .content on a ChromeWindow, try to fetch the CPOW.
>   nsCOMPtr<nsIDocShellTreeOwner> treeOwner = GetTreeOwner();
>   NS_ENSURE_TRUE(treeOwner, NS_ERROR_FAILURE);
>   return treeOwner->GetContentWindow(aCx, aVal);
> }
> 
>@@ -6799,21 +6802,21 @@ PostMessageReadStructuredClone(JSContext
>   if (tag == SCTAG_DOM_BLOB || tag == SCTAG_DOM_FILELIST) {
>     NS_ASSERTION(!data, "Data should be empty");
> 
>     nsISupports* supports;
>     if (JS_ReadBytes(reader, &supports, sizeof(supports))) {
>       JS::Rooted<JSObject*> global(cx, JS::CurrentGlobalOrNull(cx));
>       if (global) {
>         JS::Rooted<JS::Value> val(cx);
>         nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;
>         if (NS_SUCCEEDED(nsContentUtils::WrapNative(cx, global, supports,
>-                                                    val.address(),
>+                                                    &val,
>                                                     getter_AddRefs(wrapper)))) {
>           return JSVAL_TO_OBJECT(val);
>         }
>       }
>     }
>   }
> 
>   if (MessageChannel::PrefEnabled() && tag == SCTAG_DOM_MESSAGEPORT) {
>     NS_ASSERTION(!data, "Data should be empty");
> 
>diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp
>--- a/dom/base/nsJSEnvironment.cpp
>+++ b/dom/base/nsJSEnvironment.cpp
>@@ -1028,37 +1028,35 @@ nsJSContext::JSObjectFromInterface(nsISu
>     *aRet = nullptr;
>     return NS_OK;
>   }
> 
>   AutoJSContext cx;
> 
>   // Get the jsobject associated with this target
>   // We don't wrap here because we trust the JS engine to wrap the target
>   // later.
>   JS::Rooted<JS::Value> v(cx);
>-  nsresult rv = nsContentUtils::WrapNative(cx, aScope, aTarget,
>-                                           v.address());
>+  nsresult rv = nsContentUtils::WrapNative(cx, aScope, aTarget, &v);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-#ifdef DEBUG
>-  nsCOMPtr<nsISupports> targetSupp = do_QueryInterface(aTarget);
>-  nsCOMPtr<nsISupports> native =
>-    nsContentUtils::XPConnect()->GetNativeOfWrapper(cx,
>-                                                    JSVAL_TO_OBJECT(v));
>-  NS_ASSERTION(native == targetSupp, "Native should be the target!");
>-#endif
>-
>   JSObject* obj = v.toObjectOrNull();
>   if (obj) {
>     JS::ExposeObjectToActiveJS(obj);
>   }
> 
>+#ifdef DEBUG
>+  nsCOMPtr<nsISupports> targetSupp = do_QueryInterface(aTarget);
>+  nsCOMPtr<nsISupports> native =
>+    nsContentUtils::XPConnect()->GetNativeOfWrapper(cx, obj);
>+  NS_ASSERTION(native == targetSupp, "Native should be the target!");
>+#endif
>+
>   *aRet = obj;
>   return NS_OK;
> }
> 
> nsresult
> nsJSContext::BindCompiledEventHandler(nsISupports* aTarget,
>                                       JS::Handle<JSObject*> aScope,
>                                       JS::Handle<JSObject*> aHandler,
>                                       JS::MutableHandle<JSObject*> aBoundHandler)
> {
>@@ -1291,21 +1289,21 @@ nsJSContext::ConvertSupportsTojsvals(nsI
>           // just wrap it.
> #ifdef DEBUG
>           // but first, check its not another nsISupportsPrimitive, as
>           // these are now deprecated for use with script contexts.
>           nsCOMPtr<nsISupportsPrimitive> prim(do_QueryInterface(arg));
>           NS_ASSERTION(prim == nullptr,
>                        "Don't pass nsISupportsPrimitives - use nsIVariant!");
> #endif
>           nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;
>           JS::Rooted<JS::Value> v(cx);
>-          rv = nsContentUtils::WrapNative(cx, aScope, arg, v.address(),
>+          rv = nsContentUtils::WrapNative(cx, aScope, arg, &v,
>                                           getter_AddRefs(wrapper));
>           if (NS_SUCCEEDED(rv)) {
>             *thisval = v;
>           }
>         }
>       }
>     }
>   } else {
>     nsCOMPtr<nsIVariant> variant = do_QueryInterface(aArgs);
>     if (variant) {
>@@ -1493,21 +1491,21 @@ nsJSContext::AddSupportsPrimitiveTojsval
>       p->GetData(getter_AddRefs(data));
>       p->GetDataIID(&iid);
>       NS_ENSURE_TRUE(iid, NS_ERROR_UNEXPECTED);
> 
>       AutoFree iidGuard(iid); // Free iid upon destruction.
> 
>       nsCOMPtr<nsIXPConnectJSObjectHolder> wrapper;
>       JS::Rooted<JSObject*> global(cx, GetWindowProxy());
>       JS::Rooted<JS::Value> v(cx);
>       nsresult rv = nsContentUtils::WrapNative(cx, global,
>-                                               data, iid, v.address(),
>+                                               data, iid, &v,
>                                                getter_AddRefs(wrapper));
>       NS_ENSURE_SUCCESS(rv, rv);
> 
>       *aArgv = v;
> 
>       break;
>     }
>     case nsISupportsPrimitive::TYPE_ID :
>     case nsISupportsPrimitive::TYPE_PRUINT64 :
>     case nsISupportsPrimitive::TYPE_PRINT64 :
>diff --git a/dom/devicestorage/nsDeviceStorage.cpp b/dom/devicestorage/nsDeviceStorage.cpp
>--- a/dom/devicestorage/nsDeviceStorage.cpp
>+++ b/dom/devicestorage/nsDeviceStorage.cpp
>@@ -1367,25 +1367,22 @@ InterfaceToJsval(nsPIDOMWindow* aWindow,
>   if (!sgo) {
>     return JSVAL_NULL;
>   }
> 
>   JS::RootedObject scopeObj(cx, sgo->GetGlobalJSObject());
>   NS_ENSURE_TRUE(scopeObj, JSVAL_NULL);
>   JSAutoCompartment ac(cx, scopeObj);
> 
> 
>   JS::Rooted<JS::Value> someJsVal(cx);
>-  nsresult rv = nsContentUtils::WrapNative(cx,
>-                                           scopeObj,
>-                                           aObject,
>-                                           aIID,
>-                                           someJsVal.address());
>+  nsresult rv =
>+    nsContentUtils::WrapNative(cx, scopeObj, aObject, aIID, &someJsVal);
>   if (NS_FAILED(rv)) {
>     return JSVAL_NULL;
>   }
> 
>   return someJsVal;
> }
> 
> JS::Value
> nsIFileToJsval(nsPIDOMWindow* aWindow, DeviceStorageFile* aFile)
> {
>@@ -1430,21 +1427,21 @@ JS::Value StringToJsval(nsPIDOMWindow* a
>   nsIScriptContext *scriptContext = sgo->GetScriptContext();
>   if (!scriptContext) {
>     return JSVAL_NULL;
>   }
> 
>   AutoPushJSContext cx(scriptContext->GetNativeContext());
>   if (!cx) {
>     return JSVAL_NULL;
>   }
> 
>-  JS::Value result = JSVAL_NULL;
>+  JS::Rooted<JS::Value> result(cx);
>   if (!xpc::StringToJsval(cx, aString, &result)) {
>     return JSVAL_NULL;
>   }
> 
>   return result;
> }
> 
> class DeviceStorageCursorRequest MOZ_FINAL
>   : public nsIContentPermissionRequest
>   , public PCOMContentPermissionRequestChild
>diff --git a/dom/file/ArchiveRequest.cpp b/dom/file/ArchiveRequest.cpp
>--- a/dom/file/ArchiveRequest.cpp
>+++ b/dom/file/ArchiveRequest.cpp
>@@ -140,25 +140,25 @@ ArchiveRequest::ReaderReady(nsTArray<nsC
> 
>   JSAutoCompartment ac(cx, global);
> 
>   JS::Rooted<JS::Value> result(cx);
>   switch (mOperation) {
>     case GetFilenames:
>       rv = GetFilenamesResult(cx, result.address(), aFileList);
>       break;
> 
>     case GetFile:
>-      rv = GetFileResult(cx, result.address(), aFileList);
>+      rv = GetFileResult(cx, &result, aFileList);
>       break;
> 
>       case GetFiles:
>-        rv = GetFilesResult(cx, result.address(), aFileList);
>+        rv = GetFilesResult(cx, &result, aFileList);
>         break;
>   }
> 
>   if (NS_FAILED(rv)) {
>     NS_WARNING("Get*Result failed!");
>   }
> 
>   if (NS_SUCCEEDED(rv)) {
>     FireSuccess(result);
>   }
>@@ -201,21 +201,21 @@ ArchiveRequest::GetFilenamesResult(JSCon
>   if (!JS_FreezeObject(aCx, array)) {
>     return NS_ERROR_FAILURE;
>   }
> 
>   *aValue = OBJECT_TO_JSVAL(array);
>   return NS_OK;
> }
> 
> nsresult
> ArchiveRequest::GetFileResult(JSContext* aCx,
>-                              JS::Value* aValue,
>+                              JS::MutableHandle<JS::Value> aValue,
>                               nsTArray<nsCOMPtr<nsIDOMFile> >& aFileList)
> {
>   for (uint32_t i = 0; i < aFileList.Length(); ++i) {
>     nsCOMPtr<nsIDOMFile> file = aFileList[i];
> 
>     nsString filename;
>     nsresult rv = file->GetName(filename);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     if (filename == mFilename) {
>@@ -223,42 +223,42 @@ ArchiveRequest::GetFileResult(JSContext*
>       return nsContentUtils::WrapNative(aCx, global, file,
>                                         &NS_GET_IID(nsIDOMFile), aValue);
>     }
>   }
> 
>   return NS_ERROR_FAILURE;
> }
> 
> nsresult
> ArchiveRequest::GetFilesResult(JSContext* aCx,
>-                               JS::Value* aValue,
>+                               JS::MutableHandle<JS::Value> aValue,
>                                nsTArray<nsCOMPtr<nsIDOMFile> >& aFileList)
> {
>   JS::Rooted<JSObject*> array(aCx, JS_NewArrayObject(aCx, aFileList.Length(), nullptr));
>   if (!array) {
>     return NS_ERROR_OUT_OF_MEMORY;
>   }
> 
>   for (uint32_t i = 0; i < aFileList.Length(); ++i) {
>     nsCOMPtr<nsIDOMFile> file = aFileList[i];
> 
>     JS::Rooted<JS::Value> value(aCx);
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     nsresult rv = nsContentUtils::WrapNative(aCx, global, file,
>                                              &NS_GET_IID(nsIDOMFile),
>-                                             value.address());
>+                                             &value);
>     if (NS_FAILED(rv) || !JS_SetElement(aCx, array, i, &value)) {
>       return NS_ERROR_FAILURE;
>     }
>   }
> 
>-  aValue->setObject(*array);
>+  aValue.setObject(*array);
>   return NS_OK;
> }
> 
> // static
> already_AddRefed<ArchiveRequest>
> ArchiveRequest::Create(nsIDOMWindow* aOwner,
>                        ArchiveReader* aReader)
> {
>   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
> 
>diff --git a/dom/file/ArchiveRequest.h b/dom/file/ArchiveRequest.h
>--- a/dom/file/ArchiveRequest.h
>+++ b/dom/file/ArchiveRequest.h
>@@ -59,24 +59,24 @@ public: // static
>   static already_AddRefed<ArchiveRequest> Create(nsIDOMWindow* aOwner,
>                                                  ArchiveReader* aReader);
> 
> private:
>   ~ArchiveRequest();
> 
>   nsresult GetFilenamesResult(JSContext* aCx,
>                               JS::Value* aValue,
>                               nsTArray<nsCOMPtr<nsIDOMFile> >& aFileList);
>   nsresult GetFileResult(JSContext* aCx,
>-                         JS::Value* aValue,
>+                         JS::MutableHandle<JS::Value> aValue,
>                          nsTArray<nsCOMPtr<nsIDOMFile> >& aFileList);
>   nsresult GetFilesResult(JSContext* aCx,
>-                          JS::Value* aValue,
>+                          JS::MutableHandle<JS::Value> aValue,
>                           nsTArray<nsCOMPtr<nsIDOMFile> >& aFileList);
> 
> protected:
>   // The reader:
>   nsRefPtr<ArchiveReader> mArchiveReader;
> 
>   // The operation:
>   enum {
>     GetFilenames,
>     GetFile,
>diff --git a/dom/file/FileHandle.cpp b/dom/file/FileHandle.cpp
>--- a/dom/file/FileHandle.cpp
>+++ b/dom/file/FileHandle.cpp
>@@ -178,24 +178,26 @@ FileHandle::GetFileInfo()
>   return nullptr;
> }
> 
> nsresult
> GetFileHelper::GetSuccessResult(JSContext* aCx, JS::Value* aVal)
> {
>   nsCOMPtr<nsIDOMFile> domFile =
>     mFileHandle->CreateFileObject(mLockedFile, mParams->Size());
> 
>   JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>+  JS::Rooted<JS::Value> rval(aCx);
>   nsresult rv =
>     nsContentUtils::WrapNative(aCx, global, domFile,
>-                               &NS_GET_IID(nsIDOMFile), aVal);
>+                               &NS_GET_IID(nsIDOMFile), &rval);
>   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_FILEHANDLE_UNKNOWN_ERR);
>+  *aVal = rval;
> 
>   return NS_OK;
> }
> 
> /* virtual */
> JSObject*
> FileHandle::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aScope)
> {
>   return FileHandleBinding::Wrap(aCx, aScope, this);
> }
>diff --git a/dom/file/LockedFile.cpp b/dom/file/LockedFile.cpp
>--- a/dom/file/LockedFile.cpp
>+++ b/dom/file/LockedFile.cpp
>@@ -1059,25 +1059,27 @@ ReadTextHelper::GetSuccessResult(JSConte
>   nsCString charset;
>   if (!EncodingUtils::FindEncodingForLabel(charsetGuess, charset)) {
>     return NS_ERROR_DOM_ENCODING_NOT_SUPPORTED_ERR;
>   }
> 
>   nsString tmpString;
>   rv = nsContentUtils::ConvertStringFromCharset(charset, mStream->Data(),
>                                                 tmpString);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  if (!xpc::StringToJsval(aCx, tmpString, aVal)) {
>+  JS::Rooted<JS::Value> rval(aCx);
>+  if (!xpc::StringToJsval(aCx, tmpString, &rval)) {
>     NS_WARNING("Failed to convert string!");
>     return NS_ERROR_FAILURE;
>   }
> 
>+  *aVal = rval;
>   return NS_OK;
> }
> 
> nsresult
> WriteHelper::DoAsyncRun(nsISupports* aStream)
> {
>   NS_ASSERTION(aStream, "Passed a null stream!");
> 
>   uint32_t flags = FileStreamWrapper::NOTIFY_PROGRESS;
> 
>diff --git a/dom/indexedDB/AsyncConnectionHelper.cpp b/dom/indexedDB/AsyncConnectionHelper.cpp
>--- a/dom/indexedDB/AsyncConnectionHelper.cpp
>+++ b/dom/indexedDB/AsyncConnectionHelper.cpp
>@@ -118,21 +118,21 @@ HelperBase::WrapNative(JSContext* aCx,
>   NS_ASSERTION(aCx, "Null context!");
>   NS_ASSERTION(aNative, "Null pointer!");
>   NS_ASSERTION(aResult.address(), "Null pointer!");
>   NS_ASSERTION(mRequest, "Null request!");
> 
>   nsRefPtr<IDBWrapperCache> wrapper = static_cast<IDBWrapperCache*>(mRequest);
>   JS::Rooted<JSObject*> global(aCx, wrapper->GetParentObject());
>   NS_ASSERTION(global, "This should never be null!");
> 
>   nsresult rv =
>-    nsContentUtils::WrapNative(aCx, global, aNative, aResult.address());
>+    nsContentUtils::WrapNative(aCx, global, aNative, aResult);
>   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
> 
>   return NS_OK;
> }
> 
> void
> HelperBase::ReleaseMainThreadObjects()
> {
>   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
> 
>diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp
>--- a/dom/indexedDB/IDBObjectStore.cpp
>+++ b/dom/indexedDB/IDBObjectStore.cpp
>@@ -770,21 +770,21 @@ public:
> 
>     nsRefPtr<IDBFileHandle> fileHandle = IDBFileHandle::Create(aDatabase,
>       aData.name, aData.type, fileInfo.forget());
> 
>     JS::Rooted<JS::Value> wrappedFileHandle(aCx);
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     nsresult rv =
>       nsContentUtils::WrapNative(aCx, global,
>                                  static_cast<nsIDOMFileHandle*>(fileHandle),
>                                  &NS_GET_IID(nsIDOMFileHandle),
>-                                 wrappedFileHandle.address());
>+                                 &wrappedFileHandle);
>     if (NS_FAILED(rv)) {
>       NS_WARNING("Failed to wrap native!");
>       return nullptr;
>     }
> 
>     return JSVAL_TO_OBJECT(wrappedFileHandle);
>   }
> 
>   static JSObject* CreateAndWrapBlobOrFile(JSContext* aCx,
>                                            IDBDatabase* aDatabase,
>@@ -829,21 +829,21 @@ public:
>       }
>       else {
>         domBlob = new nsDOMFileFile(aData.type, aData.size, nativeFile,
>                                     fileInfo);
>       }
> 
>       JS::Rooted<JS::Value> wrappedBlob(aCx);
>       JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>       rv = nsContentUtils::WrapNative(aCx, global, domBlob,
>                                       &NS_GET_IID(nsIDOMBlob),
>-                                      wrappedBlob.address());
>+                                      &wrappedBlob);
>       if (NS_FAILED(rv)) {
>         NS_WARNING("Failed to wrap native!");
>         return nullptr;
>       }
> 
>       return JSVAL_TO_OBJECT(wrappedBlob);
>     }
> 
>     nsCOMPtr<nsIDOMFile> domFile;
>     if (aFile.mFile) {
>@@ -856,21 +856,21 @@ public:
>     }
>     else {
>       domFile = new nsDOMFileFile(aData.name, aData.type, aData.size,
>                                   nativeFile, fileInfo);
>     }
> 
>     JS::Rooted<JS::Value> wrappedFile(aCx);
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     rv = nsContentUtils::WrapNative(aCx, global, domFile,
>                                     &NS_GET_IID(nsIDOMFile),
>-                                    wrappedFile.address());
>+                                    &wrappedFile);
>     if (NS_FAILED(rv)) {
>       NS_WARNING("Failed to wrap native!");
>       return nullptr;
>     }
> 
>     return JSVAL_TO_OBJECT(wrappedFile);
>   }
> };
> 
> 
>diff --git a/dom/indexedDB/Key.cpp b/dom/indexedDB/Key.cpp
>--- a/dom/indexedDB/Key.cpp
>+++ b/dom/indexedDB/Key.cpp
>@@ -221,21 +221,21 @@ Key::DecodeJSValInternal(const unsigned 
> 
>     NS_ASSERTION(aPos >= aEnd || (*aPos % eMaxType) == eTerminator,
>                  "Should have found end-of-array marker");
>     ++aPos;
> 
>     aVal.setObject(*array);
>   }
>   else if (*aPos - aTypeOffset == eString) {
>     nsString key;
>     DecodeString(aPos, aEnd, key);
>-    if (!xpc::StringToJsval(aCx, key, aVal.address())) {
>+    if (!xpc::StringToJsval(aCx, key, aVal)) {
>       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>     }
>   }
>   else if (*aPos - aTypeOffset == eDate) {
>     double msec = static_cast<double>(DecodeNumber(aPos, aEnd));
>     JSObject* date = JS_NewDateObjectMsec(aCx, msec);
>     if (!date) {
>       NS_WARNING("Failed to make date!");
>       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>     }
>diff --git a/dom/indexedDB/KeyPath.cpp b/dom/indexedDB/KeyPath.cpp
>--- a/dom/indexedDB/KeyPath.cpp
>+++ b/dom/indexedDB/KeyPath.cpp
>@@ -34,21 +34,21 @@ IsValidKeyPathString(JSContext* aCx, con
> 
>   KeyPathTokenizer tokenizer(aKeyPath, '.');
> 
>   while (tokenizer.hasMoreTokens()) {
>     nsString token(tokenizer.nextToken());
> 
>     if (!token.Length()) {
>       return false;
>     }
> 
>-    jsval stringVal;
>+    JS::Rooted<JS::Value> stringVal(aCx);
>     if (!xpc::StringToJsval(aCx, token, &stringVal)) {
>       return false;
>     }
> 
>     NS_ASSERTION(stringVal.toString(), "This should never happen");
>     JS::RootedString str(aCx, stringVal.toString());
> 
>     bool isIdentifier = false;
>     if (!JS_IsIdentifier(aCx, str, &isIdentifier) || !isIdentifier) {
>       return false;
>@@ -495,36 +495,36 @@ KeyPath::ToJSVal(JSContext* aCx, JS::Mut
>     uint32_t len = mStrings.Length();
>     JS::Rooted<JSObject*> array(aCx, JS_NewArrayObject(aCx, len, nullptr));
>     if (!array) {
>       NS_WARNING("Failed to make array!");
>       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>     }
> 
>     for (uint32_t i = 0; i < len; ++i) {
>       JS::Rooted<JS::Value> val(aCx);
>       nsString tmp(mStrings[i]);
>-      if (!xpc::StringToJsval(aCx, tmp, val.address())) {
>+      if (!xpc::StringToJsval(aCx, tmp, &val)) {
>         return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>       }
> 
>       if (!JS_SetElement(aCx, array, i, &val)) {
>         return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>       }
>     }
> 
>     aValue.setObject(*array);
>     return NS_OK;
>   }
> 
>   if (IsString()) {
>     nsString tmp(mStrings[0]);
>-    if (!xpc::StringToJsval(aCx, tmp, aValue.address())) {
>+    if (!xpc::StringToJsval(aCx, tmp, aValue)) {
>       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
>     }
>     return NS_OK;
>   }
> 
>   aValue.setNull();
>   return NS_OK;
> }
> 
> nsresult
>diff --git a/dom/ipc/StructuredCloneUtils.cpp b/dom/ipc/StructuredCloneUtils.cpp
>--- a/dom/ipc/StructuredCloneUtils.cpp
>+++ b/dom/ipc/StructuredCloneUtils.cpp
>@@ -51,21 +51,21 @@ Read(JSContext* aCx, JSStructuredCloneRe
>       bool isMutable;
>       MOZ_ASSERT(NS_SUCCEEDED(mutableFile->GetMutable(&isMutable)));
>       MOZ_ASSERT(!isMutable);
>     }
> #endif
> 
>     JS::Rooted<JS::Value> wrappedFile(aCx);
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     nsresult rv = nsContentUtils::WrapNative(aCx, global, file,
>                                              &NS_GET_IID(nsIDOMFile),
>-                                             wrappedFile.address());
>+                                             &wrappedFile);
>     if (NS_FAILED(rv)) {
>       Error(aCx, nsIDOMDOMException::DATA_CLONE_ERR);
>       return nullptr;
>     }
> 
>     return &wrappedFile.toObject();
>   }
> 
>   if (aTag == SCTAG_DOM_BLOB) {
>     MOZ_ASSERT(aData < closure->mBlobs.Length());
>@@ -80,21 +80,21 @@ Read(JSContext* aCx, JSStructuredCloneRe
>       bool isMutable;
>       MOZ_ASSERT(NS_SUCCEEDED(mutableBlob->GetMutable(&isMutable)));
>       MOZ_ASSERT(!isMutable);
>     }
> #endif
> 
>     JS::Rooted<JS::Value> wrappedBlob(aCx);
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     nsresult rv = nsContentUtils::WrapNative(aCx, global, blob,
>                                              &NS_GET_IID(nsIDOMBlob),
>-                                             wrappedBlob.address());
>+                                             &wrappedBlob);
>     if (NS_FAILED(rv)) {
>       Error(aCx, nsIDOMDOMException::DATA_CLONE_ERR);
>       return nullptr;
>     }
> 
>     return &wrappedBlob.toObject();
>   }
> 
>   return NS_DOMReadStructuredClone(aCx, aReader, aTag, aData, nullptr);
> }
>diff --git a/dom/mobilemessage/src/MmsMessage.cpp b/dom/mobilemessage/src/MmsMessage.cpp
>--- a/dom/mobilemessage/src/MmsMessage.cpp
>+++ b/dom/mobilemessage/src/MmsMessage.cpp
>@@ -510,21 +510,21 @@ MmsMessage::GetAttachments(JSContext* aC
>                            NULL, NULL, JSPROP_ENUMERATE)) {
>       return NS_ERROR_FAILURE;
>     }
> 
>     // Get |attachment.mContent|.
>     JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>     nsresult rv = nsContentUtils::WrapNative(aCx,
>                                              global,
>                                              attachment.content,
>                                              &NS_GET_IID(nsIDOMBlob),
>-                                             tmpJsVal.address());
>+                                             &tmpJsVal);
>     NS_ENSURE_SUCCESS(rv, rv);
> 
>     if (!JS_DefineProperty(aCx, attachmentObj, "content", tmpJsVal,
>                            NULL, NULL, JSPROP_ENUMERATE)) {
>       return NS_ERROR_FAILURE;
>     }
> 
>     tmpJsVal = OBJECT_TO_JSVAL(attachmentObj);
>     if (!JS_SetElement(aCx, attachments, i, &tmpJsVal)) {
>       return NS_ERROR_FAILURE;
>diff --git a/dom/mobilemessage/src/MobileMessageCallback.cpp b/dom/mobilemessage/src/MobileMessageCallback.cpp
>--- a/dom/mobilemessage/src/MobileMessageCallback.cpp
>+++ b/dom/mobilemessage/src/MobileMessageCallback.cpp
>@@ -64,22 +64,21 @@ MobileMessageCallback::NotifySuccess(nsI
> 
>   AutoPushJSContext cx(scriptContext->GetNativeContext());
>   NS_ENSURE_TRUE(cx, NS_ERROR_FAILURE);
> 
>   JS::Rooted<JSObject*> global(cx, scriptContext->GetWindowProxy());
>   NS_ENSURE_TRUE(global, NS_ERROR_FAILURE);
> 
>   JSAutoCompartment ac(cx, global);
> 
>   JS::Rooted<JS::Value> wrappedMessage(cx);
>-  rv = nsContentUtils::WrapNative(cx, global, aMessage,
>-                                  wrappedMessage.address());
>+  rv = nsContentUtils::WrapNative(cx, global, aMessage, &wrappedMessage);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   return NotifySuccess(wrappedMessage, aAsync);
> }
> 
> nsresult
> MobileMessageCallback::NotifyError(int32_t aError, bool aAsync)
> {
>   nsAutoString errorStr;
>   switch (aError) {
>diff --git a/dom/mobilemessage/src/MobileMessageCursorCallback.cpp b/dom/mobilemessage/src/MobileMessageCursorCallback.cpp
>--- a/dom/mobilemessage/src/MobileMessageCursorCallback.cpp
>+++ b/dom/mobilemessage/src/MobileMessageCursorCallback.cpp
>@@ -66,21 +66,21 @@ MobileMessageCursorCallback::NotifyCurso
> 
>   AutoPushJSContext cx(scriptContext->GetNativeContext());
>   NS_ENSURE_TRUE(cx, NS_ERROR_FAILURE);
> 
>   JS::Rooted<JSObject*> global(cx, scriptContext->GetWindowProxy());
>   NS_ENSURE_TRUE(global, NS_ERROR_FAILURE);
> 
>   JSAutoCompartment ac(cx, global);
> 
>   JS::Rooted<JS::Value> wrappedResult(cx);
>-  rv = nsContentUtils::WrapNative(cx, global, aResult, wrappedResult.address());
>+  rv = nsContentUtils::WrapNative(cx, global, aResult, &wrappedResult);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   mDOMCursor->FireSuccess(wrappedResult);
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
> MobileMessageCursorCallback::NotifyCursorDone()
> {
>   MOZ_ASSERT(mDOMCursor);
>diff --git a/dom/mobilemessage/src/MobileMessageManager.cpp b/dom/mobilemessage/src/MobileMessageManager.cpp
>--- a/dom/mobilemessage/src/MobileMessageManager.cpp
>+++ b/dom/mobilemessage/src/MobileMessageManager.cpp
>@@ -129,28 +129,30 @@ MobileMessageManager::Send(JSContext* aC
> 
>   nsRefPtr<DOMRequest> request = new DOMRequest(GetOwner());
>   nsCOMPtr<nsIMobileMessageCallback> msgCallback =
>     new MobileMessageCallback(request);
> 
>   // By default, we don't send silent messages via MobileMessageManager.
>   nsresult rv = smsService->Send(number, aMessage, false, msgCallback);
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>   JS::Rooted<JSObject*> global(aCx, aGlobal);
>+  JS::Rooted<JS::Value> rval(aCx);
>   rv = nsContentUtils::WrapNative(aCx, global,
>                                   static_cast<nsIDOMDOMRequest*>(request.get()),
>-                                  aRequest);
>+                                  &rval);
>   if (NS_FAILED(rv)) {
>     NS_ERROR("Failed to create the js value!");
>     return rv;
>   }
> 
>+  *aRequest = rval;
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
> MobileMessageManager::Send(const JS::Value& aNumber_, const nsAString& aMessage, JS::Value* aReturn)
> {
>   nsresult rv;
>   nsIScriptContext* sc = GetContextForEventHandlers(&rv);
>   NS_ENSURE_STATE(sc);
>   AutoPushJSContext cx(sc->GetNativeContext());
>diff --git a/dom/mobilemessage/src/ipc/SmsParent.cpp b/dom/mobilemessage/src/ipc/SmsParent.cpp
>--- a/dom/mobilemessage/src/ipc/SmsParent.cpp
>+++ b/dom/mobilemessage/src/ipc/SmsParent.cpp
>@@ -55,21 +55,21 @@ MmsAttachmentDataToJSObject(JSContext* a
>     return nullptr;
>   }
> 
>   nsCOMPtr<nsIDOMBlob> blob = static_cast<BlobParent*>(aAttachment.contentParent())->GetBlob();
>   JS::Rooted<JS::Value> content(aContext);
>   JS::Rooted<JSObject*> global(aContext, JS::CurrentGlobalOrNull(aContext));
>   nsresult rv = nsContentUtils::WrapNative(aContext,
>                                            global,
>                                            blob,
>                                            &NS_GET_IID(nsIDOMBlob),
>-                                           content.address());
>+                                           &content);
>   NS_ENSURE_SUCCESS(rv, nullptr);
>   if (!JS_DefineProperty(aContext, obj, "content", content,
>                          nullptr, nullptr, 0)) {
>     return nullptr;
>   }
> 
>   return obj;
> }
> 
> static bool
>diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp
>--- a/dom/workers/WorkerPrivate.cpp
>+++ b/dom/workers/WorkerPrivate.cpp
>@@ -398,21 +398,21 @@ struct MainThreadWorkerStructuredCloneCa
>                        "Only immutable file should be passed to worker");
>         }
> #endif
> 
>         // nsIDOMFiles should be threadsafe, thus we will use the same instance
>         // on the main thread.
>         JS::Rooted<JS::Value> wrappedFile(aCx);
>         JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>         nsresult rv = nsContentUtils::WrapNative(aCx, global, file,
>                                                  &NS_GET_IID(nsIDOMFile),
>-                                                 wrappedFile.address());
>+                                                 &wrappedFile);
>         if (NS_FAILED(rv)) {
>           Error(aCx, nsIDOMDOMException::DATA_CLONE_ERR);
>           return nullptr;
>         }
> 
>         return &wrappedFile.toObject();
>       }
>     }
>     // See if object is a nsIDOMBlob pointer.
>     else if (aTag == DOMWORKER_SCTAG_BLOB) {
>@@ -432,21 +432,21 @@ struct MainThreadWorkerStructuredCloneCa
>                        "Only immutable blob should be passed to worker");
>         }
> #endif
> 
>         // nsIDOMBlobs should be threadsafe, thus we will use the same instance
>         // on the main thread.
>         JS::Rooted<JS::Value> wrappedBlob(aCx);
>         JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
>         nsresult rv = nsContentUtils::WrapNative(aCx, global, blob,
>                                                  &NS_GET_IID(nsIDOMBlob),
>-                                                 wrappedBlob.address());
>+                                                 &wrappedBlob);
>         if (NS_FAILED(rv)) {
>           Error(aCx, nsIDOMDOMException::DATA_CLONE_ERR);
>           return nullptr;
>         }
> 
>         return &wrappedBlob.toObject();
>       }
>     }
> 
>     JS_ClearPendingException(aCx);

Comment 16

[Steve Fink [:sfink] [:s:]]

Attachment: Handlify a few things in bluetooth

I couldn't land this because it broke some b2g builds. Surprisingly, this patch appears to be all that's necessary to get them back up.

Comment 17

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cfe47cb78e84
https://hg.mozilla.org/integration/mozilla-inbound/rev/1aae4861f351
https://hg.mozilla.org/integration/mozilla-inbound/rev/2af6130b15e0
https://hg.mozilla.org/integration/mozilla-inbound/rev/28d01dfcb357
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c43a39c0f6
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0097bc342f9

Comment 18

[Steve Fink [:sfink] [:s:]]

Argh! Those should have been landed as one patch. Nothing compiles independently.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cfe47cb78e84
https://hg.mozilla.org/mozilla-central/rev/1aae4861f351
https://hg.mozilla.org/mozilla-central/rev/2af6130b15e0
https://hg.mozilla.org/mozilla-central/rev/28d01dfcb357
https://hg.mozilla.org/mozilla-central/rev/e4c43a39c0f6
https://hg.mozilla.org/mozilla-central/rev/d0097bc342f9

Comment 20

[Steve Fink [:sfink] [:s:]]

Attachment: Fix incorrectly applied reviewer feedback.

Landing a followup fixup.

Comment 21

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/458a92390160

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/458a92390160