Bug 1371259. Rejigger DOM object unwrapping to take mutable handles to the JS value/object in a bunch of cases. r=peterv,mccr8 a=jcristau
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 10 Jul 2017 18:06:18 -0400
changeset 411894 e3d205dc31f33fd2897fd68ffb9ff15e9e3bc079
parent 411893 d44b343011f71cd09e23ca7197b7c06899e03ae5
child 411895 e8e60131e908bb4cf7f919a98646ccb548a6c0b9
push id7496
push usercbook@mozilla.com
push dateTue, 11 Jul 2017 14:24:02 +0000
treeherdermozilla-beta@f237f91d8c06 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, mccr8, jcristau
bugs1371259
milestone55.0
Bug 1371259. Rejigger DOM object unwrapping to take mutable handles to the JS value/object in a bunch of cases. r=peterv,mccr8 a=jcristau
dom/base/StructuredCloneHolder.cpp
dom/base/nsDOMClassInfo.cpp
dom/base/nsDOMClassInfo.h
dom/base/nsDOMWindowUtils.cpp
dom/base/nsDocument.cpp
dom/bindings/BindingUtils.cpp
dom/bindings/BindingUtils.h
dom/bindings/Codegen.py
dom/bindings/WebIDLGlobalNameHash.cpp
dom/indexedDB/IDBObjectStore.cpp
dom/media/WebVTTListener.cpp
dom/messagechannel/MessagePort.cpp
dom/notification/Notification.cpp
dom/presentation/PresentationSessionInfo.cpp
dom/promise/Promise.cpp
dom/workers/ScriptLoader.cpp
dom/workers/ServiceWorker.cpp
dom/workers/ServiceWorkerScriptCache.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerScope.cpp
dom/xbl/nsXBLProtoImplField.cpp
dom/xslt/xpath/XPathExpression.cpp
js/xpconnect/idl/nsIXPConnect.idl
js/xpconnect/src/ExportHelpers.cpp
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCConvert.cpp
js/xpconnect/src/XPCJSID.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCJSWeakReference.cpp
js/xpconnect/src/XPCWrappedJSClass.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcpublic.h
--- a/dom/base/StructuredCloneHolder.cpp
+++ b/dom/base/StructuredCloneHolder.cpp
@@ -441,56 +441,58 @@ StructuredCloneHolder::ReadFullySerializ
   return nullptr;
 }
 
 /* static */ bool
 StructuredCloneHolder::WriteFullySerializableObjects(JSContext* aCx,
                                                      JSStructuredCloneWriter* aWriter,
                                                      JS::Handle<JSObject*> aObj)
 {
+  JS::Rooted<JSObject*> obj(aCx, aObj);
+
   // See if this is a ImageData object.
   {
     ImageData* imageData = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(ImageData, aObj, imageData))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(ImageData, &obj, imageData))) {
       return WriteStructuredCloneImageData(aCx, aWriter, imageData);
     }
   }
 
   // Handle URLSearchParams cloning
   {
     URLSearchParams* usp = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(URLSearchParams, aObj, usp))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(URLSearchParams, &obj, usp))) {
       return JS_WriteUint32Pair(aWriter, SCTAG_DOM_URLSEARCHPARAMS, 0) &&
              usp->WriteStructuredClone(aWriter);
     }
   }
 
   // Handle Key cloning
   {
     CryptoKey* key = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(CryptoKey, aObj, key))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(CryptoKey, &obj, key))) {
       return JS_WriteUint32Pair(aWriter, SCTAG_DOM_WEBCRYPTO_KEY, 0) &&
              key->WriteStructuredClone(aWriter);
     }
   }
 
 #ifdef MOZ_WEBRTC
   {
     // Handle WebRTC Certificate cloning
     RTCCertificate* cert = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(RTCCertificate, aObj, cert))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(RTCCertificate, &obj, cert))) {
       MOZ_ASSERT(NS_IsMainThread());
       return JS_WriteUint32Pair(aWriter, SCTAG_DOM_RTC_CERTIFICATE, 0) &&
              cert->WriteStructuredClone(aWriter);
     }
   }
 #endif
 
-  if (NS_IsMainThread() && xpc::IsReflector(aObj)) {
-    nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(aObj);
+  if (NS_IsMainThread() && xpc::IsReflector(obj)) {
+    nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(obj);
     nsCOMPtr<nsIPrincipal> principal = do_QueryInterface(base);
     if (principal) {
       auto nsjsprincipals = nsJSPrincipals::get(principal);
       return nsjsprincipals->write(aCx, aWriter);
     }
   }
 
   // Don't know what this is
@@ -1006,72 +1008,74 @@ bool
 StructuredCloneHolder::CustomWriteHandler(JSContext* aCx,
                                           JSStructuredCloneWriter* aWriter,
                                           JS::Handle<JSObject*> aObj)
 {
   if (!mSupportsCloning) {
     return false;
   }
 
+  JS::Rooted<JSObject*> obj(aCx, aObj);
+
   // See if this is a File/Blob object.
   {
     Blob* blob = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, aObj, blob))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, &obj, blob))) {
       return WriteBlob(aWriter, blob, this);
     }
   }
 
   // See if this is a Directory object.
   {
     Directory* directory = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(Directory, aObj, directory))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(Directory, &obj, directory))) {
       return WriteDirectory(aWriter, directory);
     }
   }
 
   // See if this is a FileList object.
   {
     FileList* fileList = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(FileList, aObj, fileList))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(FileList, &obj, fileList))) {
       return WriteFileList(aWriter, fileList, this);
     }
   }
 
   // See if this is a FormData object.
   {
     FormData* formData = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(FormData, aObj, formData))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(FormData, &obj, formData))) {
       return WriteFormData(aWriter, formData, this);
     }
   }
 
   // See if this is an ImageBitmap object.
   if (mStructuredCloneScope == StructuredCloneScope::SameProcessSameThread ||
       mStructuredCloneScope == StructuredCloneScope::SameProcessDifferentThread) {
     ImageBitmap* imageBitmap = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(ImageBitmap, aObj, imageBitmap))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(ImageBitmap, &obj, imageBitmap))) {
       return ImageBitmap::WriteStructuredClone(aWriter,
                                                GetSurfaces(),
                                                imageBitmap);
     }
   }
 
   // See if this is a StructuredCloneBlob object.
   {
     StructuredCloneBlob* holder = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(StructuredCloneHolder, aObj, holder))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(StructuredCloneHolder, &obj, holder))) {
       return holder->WriteStructuredClone(aCx, aWriter, this);
     }
   }
 
   // See if this is a WasmModule.
   if ((mStructuredCloneScope == StructuredCloneScope::SameProcessSameThread ||
        mStructuredCloneScope == StructuredCloneScope::SameProcessDifferentThread) &&
-      JS::IsWasmModuleObject(aObj)) {
-    RefPtr<JS::WasmModule> module = JS::GetWasmModule(aObj);
+      JS::IsWasmModuleObject(obj)) {
+    RefPtr<JS::WasmModule> module = JS::GetWasmModule(obj);
     MOZ_ASSERT(module);
 
     return WriteWasmModule(aWriter, module, this);
   }
 
   {
     nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(aObj);
     nsCOMPtr<nsIInputStream> inputStream = do_QueryInterface(base);
@@ -1169,19 +1173,21 @@ StructuredCloneHolder::CustomWriteTransf
                                                   JS::TransferableOwnership* aOwnership,
                                                   void** aContent,
                                                   uint64_t* aExtraData)
 {
   if (!mSupportsTransferring) {
     return false;
   }
 
+  JS::Rooted<JSObject*> obj(aCx, aObj);
+
   {
     MessagePort* port = nullptr;
-    nsresult rv = UNWRAP_OBJECT(MessagePort, aObj, port);
+    nsresult rv = UNWRAP_OBJECT(MessagePort, &obj, port);
     if (NS_SUCCEEDED(rv)) {
       // We use aExtraData to store the index of this new port identifier.
       *aExtraData = mPortIdentifiers.Length();
       MessagePortIdentifier* identifier = mPortIdentifiers.AppendElement();
 
       port->CloneAndDisentangle(*identifier);
 
       *aTag = SCTAG_DOM_MAP_MESSAGEPORT;
@@ -1189,32 +1195,32 @@ StructuredCloneHolder::CustomWriteTransf
       *aContent = nullptr;
 
       return true;
     }
 
     if (mStructuredCloneScope == StructuredCloneScope::SameProcessSameThread ||
         mStructuredCloneScope == StructuredCloneScope::SameProcessDifferentThread) {
       OffscreenCanvas* canvas = nullptr;
-      rv = UNWRAP_OBJECT(OffscreenCanvas, aObj, canvas);
+      rv = UNWRAP_OBJECT(OffscreenCanvas, &obj, canvas);
       if (NS_SUCCEEDED(rv)) {
         MOZ_ASSERT(canvas);
 
         *aExtraData = 0;
         *aTag = SCTAG_DOM_CANVAS;
         *aOwnership = JS::SCTAG_TMO_CUSTOM;
         *aContent = canvas->ToCloneData();
         MOZ_ASSERT(*aContent);
         canvas->SetNeutered();
 
         return true;
       }
 
       ImageBitmap* bitmap = nullptr;
-      rv = UNWRAP_OBJECT(ImageBitmap, aObj, bitmap);
+      rv = UNWRAP_OBJECT(ImageBitmap, &obj, bitmap);
       if (NS_SUCCEEDED(rv)) {
         MOZ_ASSERT(bitmap);
 
         *aExtraData = 0;
         *aTag = SCTAG_DOM_IMAGEBITMAP;
         *aOwnership = JS::SCTAG_TMO_CUSTOM;
         *aContent = bitmap->ToCloneData().release();
         MOZ_ASSERT(*aContent);
--- a/dom/base/nsDOMClassInfo.cpp
+++ b/dom/base/nsDOMClassInfo.cpp
@@ -880,36 +880,26 @@ nsDOMClassInfo::PostCreatePrototype(JSCo
 
   // Make prototype delegation work correctly. Consider if a site sets
   // HTMLElement.prototype.foopy = function () { ... } Now, calling
   // document.body.foopy() needs to ensure that looking up foopy on
   // document.body's prototype will find the right function.
   JS::Rooted<JSObject*> global(cx, ::JS_GetGlobalForObject(cx, proto));
 
   // Only do this if the global object is a window.
-  // XXX Is there a better way to check this?
-  nsISupports *globalNative = XPConnect()->GetNativeOfWrapper(cx, global);
-  nsCOMPtr<nsPIDOMWindowInner> piwin = do_QueryInterface(globalNative);
-  if (!piwin) {
+  nsGlobalWindow* win;
+  if (NS_FAILED(UNWRAP_OBJECT(Window, &global, win))) {
+    // Not a window.
     return NS_OK;
   }
 
-  nsGlobalWindow *win = nsGlobalWindow::Cast(piwin);
   if (win->IsClosedOrClosing()) {
     return NS_OK;
   }
 
-  // If the window is in a different compartment than the global object, then
-  // it's likely that global is a sandbox object whose prototype is a window.
-  // Don't do anything in this case.
-  if (win->FastGetGlobalJSObject() &&
-      js::GetObjectCompartment(global) != js::GetObjectCompartment(win->FastGetGlobalJSObject())) {
-    return NS_OK;
-  }
-
   // Don't overwrite a property set by content.
   bool contentDefinedProperty;
   if (!::JS_AlreadyHasOwnUCProperty(cx, global, reinterpret_cast<const char16_t*>(mData->mNameUTF16),
                                     NS_strlen(mData->mNameUTF16),
                                     &contentDefinedProperty)) {
     return NS_ERROR_FAILURE;
   }
 
--- a/dom/base/nsDOMClassInfo.h
+++ b/dom/base/nsDOMClassInfo.h
@@ -138,35 +138,16 @@ const nsQueryInterfaceWithError
 do_QueryWrappedNative(nsIXPConnectWrappedNative *wrapper, JSObject *obj,
                       nsresult *aError)
 
 {
   return nsQueryInterfaceWithError(nsDOMClassInfo::GetNative(wrapper, obj),
                                    aError);
 }
 
-inline
-nsQueryInterface
-do_QueryWrapper(JSContext *cx, JSObject *obj)
-{
-  nsISupports *native =
-    nsDOMClassInfo::XPConnect()->GetNativeOfWrapper(cx, obj);
-  return nsQueryInterface(native);
-}
-
-inline
-nsQueryInterfaceWithError
-do_QueryWrapper(JSContext *cx, JSObject *obj, nsresult* error)
-{
-  nsISupports *native =
-    nsDOMClassInfo::XPConnect()->GetNativeOfWrapper(cx, obj);
-  return nsQueryInterfaceWithError(native, error);
-}
-
-
 typedef nsDOMClassInfo nsDOMGenericSH;
 
 // Makes sure that the wrapper is preserved if new properties are added.
 class nsEventTargetSH : public nsDOMGenericSH
 {
 protected:
   explicit nsEventTargetSH(nsDOMClassInfoData* aData) : nsDOMGenericSH(aData)
   {
--- a/dom/base/nsDOMWindowUtils.cpp
+++ b/dom/base/nsDOMWindowUtils.cpp
@@ -3131,26 +3131,26 @@ NS_IMETHODIMP
 nsDOMWindowUtils::GetFileId(JS::Handle<JS::Value> aFile, JSContext* aCx,
                             int64_t* _retval)
 {
   if (aFile.isPrimitive()) {
     *_retval = -1;
     return NS_OK;
   }
 
-  JSObject* obj = aFile.toObjectOrNull();
+  JS::Rooted<JSObject*> obj(aCx, aFile.toObjectOrNull());
 
   IDBMutableFile* mutableFile = nullptr;
-  if (NS_SUCCEEDED(UNWRAP_OBJECT(IDBMutableFile, obj, mutableFile))) {
+  if (NS_SUCCEEDED(UNWRAP_OBJECT(IDBMutableFile, &obj, mutableFile))) {
     *_retval = mutableFile->GetFileId();
     return NS_OK;
   }
 
   Blob* blob = nullptr;
-  if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, obj, blob))) {
+  if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, &obj, blob))) {
     *_retval = blob->GetFileId();
     return NS_OK;
   }
 
   *_retval = -1;
   return NS_OK;
 }
 
@@ -3158,20 +3158,20 @@ NS_IMETHODIMP
 nsDOMWindowUtils::GetFilePath(JS::HandleValue aFile, JSContext* aCx,
                               nsAString& _retval)
 {
   if (aFile.isPrimitive()) {
     _retval.Truncate();
     return NS_OK;
   }
 
-  JSObject* obj = aFile.toObjectOrNull();
+  JS::Rooted<JSObject*> obj(aCx, aFile.toObjectOrNull());
 
   File* file = nullptr;
-  if (NS_SUCCEEDED(UNWRAP_OBJECT(File, obj, file))) {
+  if (NS_SUCCEEDED(UNWRAP_OBJECT(File, &obj, file))) {
     nsString filePath;
     ErrorResult rv;
     file->GetMozFullPathInternal(filePath, rv);
     if (NS_WARN_IF(rv.Failed())) {
       return rv.StealNSResult();
     }
 
     _retval = filePath;
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -6010,17 +6010,18 @@ nsIDocument::CreateAttributeNS(const nsA
 
 bool
 nsDocument::CustomElementConstructor(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
 {
   JS::CallArgs args = JS::CallArgsFromVp(aArgc, aVp);
 
   JS::Rooted<JSObject*> global(aCx,
     JS_GetGlobalForObject(aCx, &args.callee()));
-  nsCOMPtr<nsPIDOMWindowInner> window = do_QueryWrapper(aCx, global);
+  RefPtr<nsGlobalWindow> window;
+  UNWRAP_OBJECT(Window, global, window);
   MOZ_ASSERT(window, "Should have a non-null window");
 
   nsDocument* document = static_cast<nsDocument*>(window->GetDoc());
 
   // Function name is the type of the custom element.
   JSString* jsFunName =
     JS_GetFunctionId(JS_ValueToFunction(aCx, args.calleev()));
   nsAutoJSString elemName;
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -1661,19 +1661,20 @@ XrayResolveOwnProperty(JSContext* cx, JS
     // explicitly. So we check if we're running in such a scope, and if so,
     // whether the wrappee is a bound element. If it is, we do a lookup via
     // specialized XBL machinery.
     //
     // While we have to do some sketchy walking through content land, we should
     // be protected by read-only/non-configurable properties, and any functions
     // we end up with should _always_ be living in our own scope (the XBL scope).
     // Make sure to assert that.
+    JS::Rooted<JSObject*> maybeElement(cx, obj);
     Element* element;
     if (xpc::ObjectScope(wrapper)->IsContentXBLScope() &&
-        NS_SUCCEEDED(UNWRAP_OBJECT(Element, obj, element))) {
+        NS_SUCCEEDED(UNWRAP_OBJECT(Element, &maybeElement, element))) {
       if (!nsContentUtils::LookupBindingMember(cx, element, id, desc)) {
         return false;
       }
 
       DEBUG_CheckXBLLookup(cx, desc.address());
 
       if (desc.object()) {
         // XBL properties shouldn't be cached on the holder, as they might be
@@ -2233,24 +2234,25 @@ ReparentWrapper(JSContext* aCx, JS::Hand
       copyTo = aObj;
     }
 
     if (!copyTo || !JS_CopyPropertiesFrom(aCx, copyTo, propertyHolder)) {
       MOZ_CRASH();
     }
   }
 
+  JS::Rooted<JSObject*> maybeObjLC(aCx, aObj);
   nsObjectLoadingContent* htmlobject;
-  nsresult rv = UNWRAP_OBJECT(HTMLObjectElement, aObj, htmlobject);
+  nsresult rv = UNWRAP_OBJECT(HTMLObjectElement, &maybeObjLC, htmlobject);
   if (NS_FAILED(rv)) {
     rv = UnwrapObject<prototypes::id::HTMLEmbedElement,
-                      HTMLSharedObjectElement>(aObj, htmlobject);
+                      HTMLSharedObjectElement>(&maybeObjLC, htmlobject);
     if (NS_FAILED(rv)) {
       rv = UnwrapObject<prototypes::id::HTMLAppletElement,
-                        HTMLSharedObjectElement>(aObj, htmlobject);
+                        HTMLSharedObjectElement>(&maybeObjLC, htmlobject);
       if (NS_FAILED(rv)) {
         htmlobject = nullptr;
       }
     }
   }
   if (htmlobject) {
     htmlobject->SetupProtoChain(aCx, aObj);
   }
@@ -2305,18 +2307,20 @@ GlobalObject::GetAsSupports() const
   // Remove everything below here once all our global objects are using new
   // bindings.  If that ever happens; it would need to include Sandbox and
   // BackstagePass.
 
   // See whether mGlobalJSObject is an XPCWrappedNative.  This will redo the
   // IsWrapper bit above and the UnwrapDOMObjectToISupports in the case when
   // we're not actually an XPCWrappedNative, but this should be a rare-ish case
   // anyway.
-  mGlobalObject = xpc::UnwrapReflectorToISupports(mGlobalJSObject);
-  if (mGlobalObject) {
+  nsCOMPtr<nsISupports> supp = xpc::UnwrapReflectorToISupports(mGlobalJSObject);
+  if (supp) {
+    // See documentation for mGlobalJSObject for why this assignment is OK.
+    mGlobalObject = supp;
     return mGlobalObject;
   }
 
   // And now a final hack.  Sandbox is not a reflector, but it does have an
   // nsIGlobalObject hanging out in its private slot.  Handle that case here,
   // (though again, this will do the useless UnwrapDOMObjectToISupports if we
   // got here for something that is somehow not a DOM object, not an
   // XPCWrappedNative _and_ not a Sandbox).
@@ -2828,19 +2832,26 @@ GenericBindingGetter(JSContext* cx, unsi
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
     return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
+  // NOTE: we want to leave obj in its initial compartment, so don't want to
+  // pass it to UnwrapObject.
+  JS::Rooted<JSObject*> rootSelf(cx, obj);
   void* self;
   {
-    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    binding_detail::MutableObjectHandleWrapper wrapper(&rootSelf);
+    nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(wrapper,
+                                                                   self,
+                                                                   protoID,
+                                                                   info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
                               rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
 
   MOZ_ASSERT(info->type() == JSJitInfo::Getter);
@@ -2867,19 +2878,26 @@ GenericPromiseReturningBindingGetter(JSC
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
     ThrowInvalidThis(cx, args, false, protoID);
     return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                      args.rval());
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
+  // NOTE: we want to leave obj in its initial compartment, so don't want to
+  // pass it to UnwrapObject.
+  JS::Rooted<JSObject*> rootSelf(cx, obj);
   void* self;
   {
-    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    binding_detail::MutableObjectHandleWrapper wrapper(&rootSelf);
+    nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(wrapper,
+                                                                   self,
+                                                                   protoID,
+                                                                   info->depth);
     if (NS_FAILED(rv)) {
       ThrowInvalidThis(cx, args, rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                        protoID);
       return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                        args.rval());
     }
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Getter);
@@ -2904,19 +2922,26 @@ GenericBindingSetter(JSContext* cx, unsi
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
     return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
+  // NOTE: we want to leave obj in its initial compartment, so don't want to
+  // pass it to UnwrapObject.
+  JS::Rooted<JSObject*> rootSelf(cx, obj);
   void* self;
   {
-    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    binding_detail::MutableObjectHandleWrapper wrapper(&rootSelf);
+    nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(wrapper,
+                                                                   self,
+                                                                   protoID,
+                                                                   info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
                               rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
   if (args.length() == 0) {
     return ThrowNoSetterArg(cx, protoID);
@@ -2939,19 +2964,26 @@ GenericBindingMethod(JSContext* cx, unsi
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   const JSJitInfo *info = FUNCTION_VALUE_TO_JITINFO(args.calleev());
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
     return ThrowInvalidThis(cx, args, false, protoID);
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
+  // NOTE: we want to leave obj in its initial compartment, so don't want to
+  // pass it to UnwrapObject.
+  JS::Rooted<JSObject*> rootSelf(cx, obj);
   void* self;
   {
-    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    binding_detail::MutableObjectHandleWrapper wrapper(&rootSelf);
+    nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(wrapper,
+                                                                   self,
+                                                                   protoID,
+                                                                   info->depth);
     if (NS_FAILED(rv)) {
       return ThrowInvalidThis(cx, args,
                               rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                               protoID);
     }
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Method);
   JSJitMethodOp method = info->method;
@@ -2977,19 +3009,26 @@ GenericPromiseReturningBindingMethod(JSC
   prototypes::ID protoID = static_cast<prototypes::ID>(info->protoID);
   if (!args.thisv().isObject()) {
     ThrowInvalidThis(cx, args, false, protoID);
     return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                      args.rval());
   }
   JS::Rooted<JSObject*> obj(cx, &args.thisv().toObject());
 
+  // NOTE: we want to leave obj in its initial compartment, so don't want to
+  // pass it to UnwrapObject.
+  JS::Rooted<JSObject*> rootSelf(cx, obj);
   void* self;
   {
-    nsresult rv = UnwrapObject<void>(obj, self, protoID, info->depth);
+    binding_detail::MutableObjectHandleWrapper wrapper(&rootSelf);
+    nsresult rv = binding_detail::UnwrapObjectInternal<void, true>(wrapper,
+                                                                   self,
+                                                                   protoID,
+                                                                   info->depth);
     if (NS_FAILED(rv)) {
       ThrowInvalidThis(cx, args, rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO,
                        protoID);
       return ConvertExceptionToPromise(cx, xpc::XrayAwareCalleeGlobal(callee),
                                        args.rval());
     }
   }
   MOZ_ASSERT(info->type() == JSJitInfo::Method);
@@ -3157,17 +3196,17 @@ UnwrapArgImpl(JSContext* cx,
               JS::Handle<JSObject*> src,
               const nsIID &iid,
               void **ppArg)
 {
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  nsISupports *iface = xpc::UnwrapReflectorToISupports(src);
+  nsCOMPtr<nsISupports> iface = xpc::UnwrapReflectorToISupports(src);
   if (iface) {
     if (NS_FAILED(iface->QueryInterface(iid, ppArg))) {
       return NS_ERROR_XPC_BAD_CONVERT_JS;
     }
 
     return NS_OK;
   }
 
@@ -3187,16 +3226,50 @@ UnwrapArgImpl(JSContext* cx,
   // We need to go through the QueryInterface logic to make this return
   // the right thing for the various 'special' interfaces; e.g.
   // nsIPropertyBag. We must use AggregatedQueryInterface in cases where
   // there is an outer to avoid nasty recursion.
   return wrappedJS->QueryInterface(iid, ppArg);
 }
 
 nsresult
+UnwrapXPConnectImpl(JSContext* cx,
+                    JS::MutableHandle<JS::Value> src,
+                    const nsIID &iid,
+                    void **ppArg)
+{
+  if (!NS_IsMainThread()) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  MOZ_ASSERT(src.isObject());
+  // Unwrap ourselves, because we're going to want access to the unwrapped
+  // object.
+  JS::Rooted<JSObject*> obj(cx,
+                            js::CheckedUnwrap(&src.toObject(),
+                                              /* stopAtWindowProxy = */ false));
+  if (!obj) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  nsCOMPtr<nsISupports> iface = xpc::UnwrapReflectorToISupports(obj);
+  if (!iface) {
+    return NS_ERROR_XPC_BAD_CONVERT_JS;
+  }
+
+  if (NS_FAILED(iface->QueryInterface(iid, ppArg))) {
+    return NS_ERROR_XPC_BAD_CONVERT_JS;
+  }
+
+  // Now update our source to keep rooting our object.
+  src.setObject(*obj);
+  return NS_OK;
+}
+
+nsresult
 UnwrapWindowProxyImpl(JSContext* cx,
                       JS::Handle<JSObject*> src,
                       nsPIDOMWindowOuter** ppArg)
 {
   nsCOMPtr<nsPIDOMWindowInner> inner;
   nsresult rv = UnwrapArg<nsPIDOMWindowInner>(cx, src, getter_AddRefs(inner));
   NS_ENSURE_SUCCESS(rv, rv);
 
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -55,33 +55,57 @@ template<typename KeyType, typename Valu
 nsresult
 UnwrapArgImpl(JSContext* cx, JS::Handle<JSObject*> src, const nsIID& iid,
               void** ppArg);
 
 nsresult
 UnwrapWindowProxyImpl(JSContext* cx, JS::Handle<JSObject*> src,
                       nsPIDOMWindowOuter** ppArg);
 
-/** Convert a jsval to an XPCOM pointer. */
+/** Convert a jsval to an XPCOM pointer. Caller must not assume that src will
+    keep the XPCOM pointer rooted. */
 template <class Interface>
 inline nsresult
 UnwrapArg(JSContext* cx, JS::Handle<JSObject*> src, Interface** ppArg)
 {
   return UnwrapArgImpl(cx, src, NS_GET_TEMPLATE_IID(Interface),
                        reinterpret_cast<void**>(ppArg));
 }
 
 template <>
 inline nsresult
 UnwrapArg<nsPIDOMWindowOuter>(JSContext* cx, JS::Handle<JSObject*> src,
                               nsPIDOMWindowOuter** ppArg)
 {
   return UnwrapWindowProxyImpl(cx, src, ppArg);
 }
 
+nsresult
+UnwrapXPConnectImpl(JSContext* cx, JS::MutableHandle<JS::Value> src,
+                    const nsIID& iid, void** ppArg);
+
+/*
+ * Convert a jsval being used as a Web IDL interface implementation to an XPCOM
+ * pointer; this is only used for Web IDL interfaces that specify
+ * hasXPConnectImpls.  This is not the same as UnwrapArg because caller _can_
+ * assume that if unwrapping succeeds "val" will be updated so it's rooting the
+ * XPCOM pointer.  Also, UnwrapXPConnect doesn't need to worry about doing
+ * XPCWrappedJS things.
+ *
+ * val must be an ObjectValue.
+ */
+template<class Interface>
+inline nsresult
+UnwrapXPConnect(JSContext* cx, JS::MutableHandle<JS::Value> val,
+                Interface** ppThis)
+{
+  return UnwrapXPConnectImpl(cx, val, NS_GET_TEMPLATE_IID(Interface),
+                             reinterpret_cast<void**>(ppThis));
+}
+
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
                  bool aSecurityError, const char* aInterfaceName);
 
 bool
 ThrowInvalidThis(JSContext* aCx, const JS::CallArgs& aArgs,
                  bool aSecurityError, prototypes::ID aProtoId);
 
@@ -181,68 +205,227 @@ UnwrapDOMObjectToISupports(JSObject* aOb
 }
 
 inline bool
 IsDOMObject(JSObject* obj)
 {
   return IsDOMClass(js::GetObjectClass(obj));
 }
 
+// There are two valid ways to use UNWRAP_OBJECT: Either obj needs to
+// be a MutableHandle<JSObject*>, or value needs to be a strong-reference
+// smart pointer type (OwningNonNull or RefPtr or nsCOMPtr), in which case obj
+// can be anything that converts to JSObject*.
 #define UNWRAP_OBJECT(Interface, obj, value)                                 \
   mozilla::dom::UnwrapObject<mozilla::dom::prototypes::id::Interface,        \
     mozilla::dom::Interface##Binding::NativeType>(obj, value)
 
+// Test whether the given object is an instance of the given interface.
+#define IS_INSTANCE_OF(Interface, obj)                                  \
+  mozilla::dom::IsInstanceOf<mozilla::dom::prototypes::id::Interface,   \
+                             mozilla::dom::Interface##Binding::NativeType>(obj)
+
+// Unwrap the given non-wrapper object.  This can be used with any obj that
+// converts to JSObject*; as long as that JSObject* is live the return value
+// will be valid.
+#define UNWRAP_NON_WRAPPER_OBJECT(Interface, obj, value)                        \
+  mozilla::dom::UnwrapNonWrapperObject<mozilla::dom::prototypes::id::Interface, \
+    mozilla::dom::Interface##Binding::NativeType>(obj, value)
+
 // Some callers don't want to set an exception when unwrapping fails
 // (for example, overload resolution uses unwrapping to tell what sort
 // of thing it's looking at).
 // U must be something that a T* can be assigned to (e.g. T* or an RefPtr<T>).
-template <class T, typename U>
+//
+// The obj argument will be mutated to point to CheckedUnwrap of itself if the
+// passed-in value is not a DOM object and CheckedUnwrap succeeds.
+//
+// If mayBeWrapper is true, there are three valid ways to invoke
+// UnwrapObjectInternal: Either obj needs to be a class wrapping a
+// MutableHandle<JSObject*>, with an assignment operator that sets the handle to
+// the given object, or U needs to be a strong-reference smart pointer type
+// (OwningNonNull or RefPtr or nsCOMPtr), or the value being stored in "value"
+// must not escape past being tested for falsiness immediately after the
+// UnwrapObjectInternal call.
+//
+// If mayBeWrapper is false, obj can just be a JSObject*, and U anything that a
+// T* can be assigned to.
+namespace binding_detail {
+template <class T, bool mayBeWrapper, typename U, typename V>
 MOZ_ALWAYS_INLINE nsresult
-UnwrapObject(JSObject* obj, U& value, prototypes::ID protoID,
-             uint32_t protoDepth)
+UnwrapObjectInternal(V& obj, U& value, prototypes::ID protoID,
+                     uint32_t protoDepth)
 {
   /* First check to see whether we have a DOM object */
   const DOMJSClass* domClass = GetDOMClass(obj);
-  if (!domClass) {
-    /* Maybe we have a security wrapper or outer window? */
-    if (!js::IsWrapper(obj)) {
-      /* Not a DOM object, not a wrapper, just bail */
-      return NS_ERROR_XPC_BAD_CONVERT_JS;
-    }
-
-    obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
-    if (!obj) {
-      return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
-    }
-    MOZ_ASSERT(!js::IsWrapper(obj));
-    domClass = GetDOMClass(obj);
-    if (!domClass) {
-      /* We don't have a DOM object */
-      return NS_ERROR_XPC_BAD_CONVERT_JS;
+  if (domClass) {
+    /* This object is a DOM object.  Double-check that it is safely
+       castable to T by checking whether it claims to inherit from the
+       class identified by protoID. */
+    if (domClass->mInterfaceChain[protoDepth] == protoID) {
+      value = UnwrapDOMObject<T>(obj);
+      return NS_OK;
     }
   }
 
-  /* This object is a DOM object.  Double-check that it is safely
-     castable to T by checking whether it claims to inherit from the
-     class identified by protoID. */
-  if (domClass->mInterfaceChain[protoDepth] == protoID) {
-    value = UnwrapDOMObject<T>(obj);
+  /* Maybe we have a security wrapper or outer window? */
+  if (!mayBeWrapper || !js::IsWrapper(obj)) {
+    /* Not a DOM object, not a wrapper, just bail */
+    return NS_ERROR_XPC_BAD_CONVERT_JS;
+  }
+
+  JSObject* unwrappedObj =
+    js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
+  if (!unwrappedObj) {
+    return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
+  }
+  MOZ_ASSERT(!js::IsWrapper(unwrappedObj));
+  // Recursive call is OK, because now we're using false for mayBeWrapper and
+  // we never reach this code if that boolean is false, so can't keep calling
+  // ourselves.
+  //
+  // Unwrap into a temporary pointer, because in general unwrapping into
+  // something of type U might trigger GC (e.g. release the value currently
+  // stored in there, with arbitrary consequences) and invalidate the
+  // "unwrappedObj" pointer.
+  T* tempValue;
+  nsresult rv = UnwrapObjectInternal<T, false>(unwrappedObj, tempValue,
+                                               protoID, protoDepth);
+  if (NS_SUCCEEDED(rv)) {
+    // It's very important to not update "obj" with the "unwrappedObj" value
+    // until we know the unwrap has succeeded.  Otherwise, in a situation in
+    // which we have an overload of object and primitive we could end up
+    // converting to the primitive from the unwrappedObj, whereas we want to do
+    // it from the original object.
+    obj = unwrappedObj;
+    // And now assign to "value"; at this point we don't care if a GC happens
+    // and invalidates unwrappedObj.
+    value = tempValue;
     return NS_OK;
   }
 
   /* It's the wrong sort of DOM object */
   return NS_ERROR_XPC_BAD_CONVERT_JS;
 }
 
-template <prototypes::ID PrototypeID, class T, typename U>
+struct MutableObjectHandleWrapper {
+  explicit MutableObjectHandleWrapper(JS::MutableHandle<JSObject*> aHandle)
+    : mHandle(aHandle)
+  {
+  }
+
+  void operator=(JSObject* aObject)
+  {
+    MOZ_ASSERT(aObject);
+    mHandle.set(aObject);
+  }
+
+  operator JSObject*() const
+  {
+    return mHandle;
+  }
+
+private:
+  JS::MutableHandle<JSObject*> mHandle;
+};
+
+struct MutableValueHandleWrapper {
+  explicit MutableValueHandleWrapper(JS::MutableHandle<JS::Value> aHandle)
+    : mHandle(aHandle)
+  {
+  }
+
+  void operator=(JSObject* aObject)
+  {
+    MOZ_ASSERT(aObject);
+    mHandle.setObject(*aObject);
+  }
+
+  operator JSObject*() const
+  {
+    return &mHandle.toObject();
+  }
+
+private:
+  JS::MutableHandle<JS::Value> mHandle;
+};
+
+} // namespace binding_detail
+
+// UnwrapObject overloads that ensure we have a MutableHandle to keep it alive.
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapObject(JS::MutableHandle<JSObject*> obj, U& value)
+{
+  binding_detail::MutableObjectHandleWrapper wrapper(obj);
+  return binding_detail::UnwrapObjectInternal<T, true>(
+    wrapper, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+}
+
+template<prototypes::ID PrototypeID, class T, typename U>
 MOZ_ALWAYS_INLINE nsresult
-UnwrapObject(JSObject* obj, U& value)
+UnwrapObject(JS::MutableHandle<JS::Value> obj, U& value)
+{
+  MOZ_ASSERT(obj.isObject());
+  binding_detail::MutableValueHandleWrapper wrapper(obj);
+  return binding_detail::UnwrapObjectInternal<T, true>(
+    wrapper, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+}
+
+// UnwrapObject overloads that ensure we have a strong ref to keep it alive.
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapObject(JSObject* obj, RefPtr<U>& value)
+{
+  return binding_detail::UnwrapObjectInternal<T, true>(
+    obj, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+}
+
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapObject(JSObject* obj, nsCOMPtr<U>& value)
+{
+  return binding_detail::UnwrapObjectInternal<T, true>(
+    obj, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+}
+
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapObject(JSObject* obj, OwningNonNull<U>& value)
 {
-  return UnwrapObject<T>(obj, value, PrototypeID,
-                         PrototypeTraits<PrototypeID>::Depth);
+  return binding_detail::UnwrapObjectInternal<T, true>(
+    obj, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+}
+
+// An UnwrapObject overload that just calls one of the JSObject* ones.
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapObject(JS::Handle<JS::Value> obj, U& value)
+{
+  MOZ_ASSERT(obj.isObject());
+  return UnwrapObject<PrototypeID, T>(&obj.toObject(), value);
+}
+
+template<prototypes::ID PrototypeID, class T>
+MOZ_ALWAYS_INLINE bool
+IsInstanceOf(JSObject* obj)
+{
+  void* ignored;
+  nsresult unwrapped = binding_detail::UnwrapObjectInternal<T, true>(
+    obj, ignored, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
+  return NS_SUCCEEDED(unwrapped);
+}
+
+template<prototypes::ID PrototypeID, class T, typename U>
+MOZ_ALWAYS_INLINE nsresult
+UnwrapNonWrapperObject(JSObject* obj, U& value)
+{
+  MOZ_ASSERT(!js::IsWrapper(obj));
+  return binding_detail::UnwrapObjectInternal<T, false>(
+    obj, value, PrototypeID, PrototypeTraits<PrototypeID>::Depth);
 }
 
 MOZ_ALWAYS_INLINE bool
 IsConvertibleToDictionary(JS::Handle<JS::Value> val)
 {
   return val.isNullOrUndefined() || val.isObject();
 }
 
--- a/dom/bindings/Codegen.py
+++ b/dom/bindings/Codegen.py
@@ -2024,19 +2024,18 @@ class CGHasInstanceHook(CGAbstractStatic
                                   "HasInstance only works for nsISupports-based classes.");
 
                     bool ok = InterfaceHasInstance(cx, argc, vp);
                     if (!ok || args.rval().toBoolean()) {
                       return ok;
                     }
 
                     // FIXME Limit this to chrome by checking xpc::AccessCheck::isChrome(obj).
-                    nsISupports* native =
-                      nsContentUtils::XPConnect()->GetNativeOfWrapper(cx,
-                                                                      js::UncheckedUnwrap(instance, /* stopAtWindowProxy = */ false));
+                    nsCOMPtr<nsISupports> native =
+                      xpc::UnwrapReflectorToISupports(js::UncheckedUnwrap(instance, /* stopAtWindowProxy = */ false));
                     nsCOMPtr<nsIDOM${name}> qiResult = do_QueryInterface(native);
                     args.rval().setBoolean(!!qiResult);
                     return true;
                     """,
                     nativeType=self.descriptor.nativeType,
                     name=self.descriptor.interface.identifier.name))
 
         hasInstanceCode = dedent("""
@@ -4248,69 +4247,78 @@ def numericValue(t, v):
             return "mozilla::NegativeInfinity<%s>()" % typeName
         if math.isnan(v):
             return "mozilla::UnspecifiedNaN<%s>()" % typeName
     return "%s%s" % (v, numericSuffixes[t])
 
 
 class CastableObjectUnwrapper():
     """
-    A class for unwrapping an object named by the "source" argument
-    based on the passed-in descriptor and storing it in a variable
-    called by the name in the "target" argument.
+    A class for unwrapping an object stored in a JS Value (or
+    MutableHandle<Value> or Handle<Value>) named by the "source" and
+    "mutableSource" arguments based on the passed-in descriptor and storing it
+    in a variable called by the name in the "target" argument.  The "source"
+    argument should be able to produce a Value or Handle<Value>; the
+    "mutableSource" argument should be able to produce a MutableHandle<Value>
 
     codeOnFailure is the code to run if unwrapping fails.
 
     If isCallbackReturnValue is "JSImpl" and our descriptor is also
     JS-implemented, fall back to just creating the right object if what we
     have isn't one already.
 
     If allowCrossOriginObj is True, then we'll first do an
     UncheckedUnwrap and then operate on the result.
     """
-    def __init__(self, descriptor, source, target, codeOnFailure,
+    def __init__(self, descriptor, source, mutableSource, target, codeOnFailure,
                  exceptionCode=None, isCallbackReturnValue=False,
                  allowCrossOriginObj=False):
         self.substitution = {
             "type": descriptor.nativeType,
             "protoID": "prototypes::id::" + descriptor.name,
             "target": target,
             "codeOnFailure": codeOnFailure,
         }
+        # Supporting both the "cross origin object" case and the "has
+        # XPConnect impls" case at the same time is a pain, so let's
+        # not do that.  That allows us to assume that our source is
+        # always a Handle or MutableHandle.
+        if allowCrossOriginObj and descriptor.hasXPConnectImpls:
+            raise TypeError("Interface %s both allows a cross-origin 'this' "
+                            "and has XPConnect impls.  We don't support that" %
+                            descriptor.name)
         if allowCrossOriginObj:
             self.substitution["uncheckedObjDecl"] = fill(
                 """
-                JS::Rooted<JSObject*> maybeUncheckedObj(cx);
-                if (xpc::WrapperFactory::IsXrayWrapper(${source})) {
-                  maybeUncheckedObj = js::UncheckedUnwrap(${source});
+                JS::Rooted<JSObject*> maybeUncheckedObj(cx, &${source}.toObject());
+                """,
+                source=source)
+            self.substitution["uncheckedObjGet"] = fill(
+                """
+                if (xpc::WrapperFactory::IsXrayWrapper(maybeUncheckedObj)) {
+                  maybeUncheckedObj = js::UncheckedUnwrap(maybeUncheckedObj);
                 } else {
-                  maybeUncheckedObj = js::CheckedUnwrap(${source});
+                  maybeUncheckedObj = js::CheckedUnwrap(maybeUncheckedObj);
                   if (!maybeUncheckedObj) {
                     $*{codeOnFailure}
                   }
                 }
                 """,
-                source=source,
                 codeOnFailure=(codeOnFailure % { 'securityError': 'true'}))
             self.substitution["source"] = "maybeUncheckedObj"
-            xpconnectUnwrap = dedent("""
-                nsresult rv;
-                { // Scope for the JSAutoCompartment, because we only
-                  // want to be in that compartment for the UnwrapArg call.
-                  JS::Rooted<JSObject*> source(cx, ${source});
-                  JSAutoCompartment ac(cx, ${source});
-                  rv = UnwrapArg<${type}>(cx, source, getter_AddRefs(objPtr));
-                }
-                """)
+            self.substitution["mutableSource"] = "&maybeUncheckedObj"
+            # No need to set up xpconnectUnwrap, since it won't be
+            # used in the allowCrossOriginObj case.
         else:
             self.substitution["uncheckedObjDecl"] = ""
+            self.substitution["uncheckedObjGet"] = ""
             self.substitution["source"] = source
+            self.substitution["mutableSource"] = mutableSource
             xpconnectUnwrap = (
-                "JS::Rooted<JSObject*> source(cx, ${source});\n"
-                "nsresult rv = UnwrapArg<${type}>(cx, source, getter_AddRefs(objPtr));\n")
+                "nsresult rv = UnwrapXPConnect<${type}>(cx, ${mutableSource}, getter_AddRefs(objPtr));\n")
 
         if descriptor.hasXPConnectImpls:
             self.substitution["codeOnFailure"] = string.Template(
                 "RefPtr<${type}> objPtr;\n" +
                 xpconnectUnwrap +
                 "if (NS_FAILED(rv)) {\n"
                 "${indentedCodeOnFailure}"
                 "}\n"
@@ -4323,24 +4331,24 @@ class CastableObjectUnwrapper():
               descriptor.interface.isJSImplemented()):
             exceptionCode = exceptionCode or codeOnFailure
             self.substitution["codeOnFailure"] = fill(
                 """
                 // Be careful to not wrap random DOM objects here, even if
                 // they're wrapped in opaque security wrappers for some reason.
                 // XXXbz Wish we could check for a JS-implemented object
                 // that already has a content reflection...
-                if (!IsDOMObject(js::UncheckedUnwrap(${source}))) {
+                if (!IsDOMObject(js::UncheckedUnwrap(&${source}.toObject()))) {
                   nsCOMPtr<nsIGlobalObject> contentGlobal;
                   JS::Handle<JSObject*> callback = CallbackOrNull();
                   if (!callback ||
                       !GetContentGlobalForJSImplementedObject(cx, callback, getter_AddRefs(contentGlobal))) {
                     $*{exceptionCode}
                   }
-                  JS::Rooted<JSObject*> jsImplSourceObj(cx, ${source});
+                  JS::Rooted<JSObject*> jsImplSourceObj(cx, &${source}.toObject());
                   ${target} = new ${type}(jsImplSourceObj, contentGlobal);
                 } else {
                   $*{codeOnFailure}
                 }
                 """,
                 exceptionCode=exceptionCode,
                 **self.substitution)
         else:
@@ -4348,35 +4356,36 @@ class CastableObjectUnwrapper():
 
     def __str__(self):
         substitution = self.substitution.copy()
         substitution["codeOnFailure"] %= {
             'securityError': 'rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO'
         }
         return fill(
             """
+            $*{uncheckedObjDecl}
             {
-              $*{uncheckedObjDecl}
-              nsresult rv = UnwrapObject<${protoID}, ${type}>(${source}, ${target});
+              $*{uncheckedObjGet}
+              nsresult rv = UnwrapObject<${protoID}, ${type}>(${mutableSource}, ${target});
               if (NS_FAILED(rv)) {
                 $*{codeOnFailure}
               }
             }
             """,
             **substitution)
 
 
 class FailureFatalCastableObjectUnwrapper(CastableObjectUnwrapper):
     """
     As CastableObjectUnwrapper, but defaulting to throwing if unwrapping fails
     """
-    def __init__(self, descriptor, source, target, exceptionCode,
+    def __init__(self, descriptor, source, mutableSource, target, exceptionCode,
                  isCallbackReturnValue, sourceDescription):
         CastableObjectUnwrapper.__init__(
-            self, descriptor, source, target,
+            self, descriptor, source, mutableSource, target,
             'ThrowErrorMessage(cx, MSG_DOES_NOT_IMPLEMENT_INTERFACE, "%s", "%s");\n'
             '%s' % (sourceDescription, descriptor.interface.identifier.name,
                     exceptionCode),
             exceptionCode,
             isCallbackReturnValue)
 
 
 def getCallbackConversionInfo(type, idlObject, isMember, isCallbackReturnValue,
@@ -4431,16 +4440,19 @@ class JSToNativeConversionInfo():
     def __init__(self, template, declType=None, holderType=None,
                  dealWithOptional=False, declArgs=None,
                  holderArgs=None):
         """
         template: A string representing the conversion code.  This will have
                   template substitution performed on it as follows:
 
           ${val} is a handle to the JS::Value in question
+          ${maybeMutableVal} May be a mutable handle to the JS::Value in
+                             question. This is only OK to use if ${val} is
+                             known to not be undefined.
           ${holderName} replaced by the holder's name, if any
           ${declName} replaced by the declaration's name
           ${haveValue} replaced by an expression that evaluates to a boolean
                        for whether we have a JS::Value.  Only used when
                        defaultValue is not None or when True is passed for
                        checkForValue to instantiateJSToNativeConversion.
                        This expression may not be already-parenthesized, so if
                        you use it with && or || make sure to put parens
@@ -4826,16 +4838,17 @@ def getJSToNativeConversionInfo(type, de
         if nullable:
             typeName = CGTemplatedType("Nullable", typeName)
             arrayRef = "${declName}.SetValue()"
         else:
             arrayRef = "${declName}"
 
         elementConversion = string.Template(elementInfo.template).substitute({
             "val": "temp" + str(nestingLevel),
+            "maybeMutableVal": "&temp" + str(nestingLevel),
             "declName": "slot" + str(nestingLevel),
             # We only need holderName here to handle isExternal()
             # interfaces, which use an internal holder for the
             # conversion even when forceOwningType ends up true.
             "holderName": "tempHolder" + str(nestingLevel),
             "passedToJSImpl": "${passedToJSImpl}"
         })
 
@@ -4935,16 +4948,17 @@ def getJSToNativeConversionInfo(type, de
         if nullable:
             declType = CGTemplatedType("Nullable", declType)
             recordRef = "${declName}.SetValue()"
         else:
             recordRef = "${declName}"
 
         valueConversion = string.Template(valueInfo.template).substitute({
             "val": "temp",
+            "maybeMutableVal": "&temp",
             "declName": "slot",
             # We only need holderName here to handle isExternal()
             # interfaces, which use an internal holder for the
             # conversion even when forceOwningType ends up true.
             "holderName": "tempHolder",
             "passedToJSImpl": "${passedToJSImpl}"
         })
 
@@ -5607,23 +5621,25 @@ def getJSToNativeConversionInfo(type, de
         if forceOwningType:
             templateBody += 'static_assert(IsRefcounted<%s>::value, "We can only store refcounted classes.");' % typeName
 
         if (not descriptor.interface.isConsequential() and
             not descriptor.interface.isExternal()):
             if failureCode is not None:
                 templateBody += str(CastableObjectUnwrapper(
                     descriptor,
-                    "&${val}.toObject()",
+                    "${val}",
+                    "${maybeMutableVal}",
                     "${declName}",
                     failureCode))
             else:
                 templateBody += str(FailureFatalCastableObjectUnwrapper(
                     descriptor,
-                    "&${val}.toObject()",
+                    "${val}",
+                    "${maybeMutableVal}",
                     "${declName}",
                     exceptionCode,
                     isCallbackReturnValue,
                     firstCap(sourceDescription)))
         else:
             # Either external, or new-binding non-castable.  We always have a
             # holder for these, because we don't actually know whether we have
             # to addref when unwrapping or not.  So we just pass an
@@ -6260,16 +6276,19 @@ def instantiateJSToNativeConversion(info
                                        pre="(", post=")")
         else:
             holderCtorArgs = None
         result.append(
             CGList([holderType, CGGeneric(" "),
                     CGGeneric(originalHolderName),
                     holderCtorArgs, CGGeneric(";\n")]))
 
+    if "maybeMutableVal" not in replacements:
+        replacements["maybeMutableVal"] = replacements["val"]
+
     conversion = CGGeneric(
         string.Template(templateBody).substitute(replacements))
 
     if checkForValue:
         if dealWithOptional:
             declConstruct = CGIndenter(
                 CGGeneric("%s.Construct(%s);\n" %
                           (originalDeclName,
@@ -6353,16 +6372,18 @@ class CGArgumentConverter(CGThing):
         # interface, arguments can possibly be undefined, but will need to be
         # converted to the key/value type of the backing object. In this case,
         # use .get() instead of direct access to the argument. This won't
         # matter for iterable since generated functions for those interface
         # don't take arguments.
         if member.isMethod() and member.isMaplikeOrSetlikeOrIterableMethod():
             self.replacementVariables["val"] = string.Template(
                 "args.get(${index})").substitute(replacer)
+            self.replacementVariables["maybeMutableVal"] = string.Template(
+                "args[${index}]").substitute(replacer)
         else:
             self.replacementVariables["val"] = string.Template(
                 "args[${index}]").substitute(replacer)
         haveValueCheck = string.Template(
             "args.hasDefined(${index})").substitute(replacer)
         self.replacementVariables["haveValue"] = haveValueCheck
         self.descriptorProvider = descriptorProvider
         if self.argument.canHaveMissingValue():
@@ -6424,16 +6445,17 @@ class CGArgumentConverter(CGThing):
                     ${elemType}& slot = *${declName}.AppendElement(mozilla::fallible);
                 """)
         ).substitute(replacer)
 
         val = string.Template("args[variadicArg]").substitute(replacer)
         variadicConversion += indent(
             string.Template(typeConversion.template).substitute({
                 "val": val,
+                "maybeMutableVal": val,
                 "declName": "slot",
                 # We only need holderName here to handle isExternal()
                 # interfaces, which use an internal holder for the
                 # conversion even when forceOwningType ends up true.
                 "holderName": "tempHolder",
                 # Use the same ${obj} as for the variadic arg itself
                 "obj": replacer["obj"],
                 "passedToJSImpl": toStringBool(isJSImplementedDescriptor(self.descriptorProvider))
@@ -8586,24 +8608,31 @@ class CGAbstractBindingMethod(CGAbstract
         self.callArgs = callArgs
         self.allowCrossOriginThis = allowCrossOriginThis
 
     def definition_body(self):
         body = self.callArgs
         if self.getThisObj is not None:
             body += self.getThisObj.define() + "\n"
         body += "%s* self;\n" % self.descriptor.nativeType
+        body += dedent(
+            """
+            JS::Rooted<JS::Value> rootSelf(cx, JS::ObjectValue(*obj));
+            """)
 
         # Our descriptor might claim that we're not castable, simply because
         # we're someone's consequential interface.  But for this-unwrapping, we
         # know that we're the real deal.  So fake a descriptor here for
         # consumption by CastableObjectUnwrapper.
         body += str(CastableObjectUnwrapper(
             self.descriptor,
-            "obj", "self", self.unwrapFailureCode,
+            "rootSelf",
+            "&rootSelf",
+            "self",
+            self.unwrapFailureCode,
             allowCrossOriginObj=self.allowCrossOriginThis))
 
         return body + self.generate_code().define()
 
     def generate_code(self):
         assert False  # Override me
 
 
@@ -10129,16 +10158,17 @@ def getUnionTypeTemplateVars(unionType, 
                 holderArgs = ""
             initHolder = "%s.emplace(%s);\n" % (holderName, holderArgs)
         else:
             initHolder = ""
 
         jsConversion = fill(
             initHolder + conversionInfo.template,
             val="value",
+            maybeMutableVal="value",
             declName="memberSlot",
             holderName=(holderName if ownsMembers else "%s.ref()" % holderName),
             destroyHolder=destroyHolder,
             passedToJSImpl="passedToJSImpl")
 
         jsConversion = fill(
             """
             tryNext = false;
@@ -10148,19 +10178,24 @@ def getUnionTypeTemplateVars(unionType, 
             }
             return true;
             """,
             structType=structType,
             name=name,
             ctorArgs=ctorArgs,
             jsConversion=jsConversion)
 
+        if ownsMembers:
+            handleType = "JS::Handle<JS::Value>"
+        else:
+            handleType = "JS::MutableHandle<JS::Value>"
+
         setter = ClassMethod("TrySetTo" + name, "bool",
                              [Argument("JSContext*", "cx"),
-                              Argument("JS::Handle<JS::Value>", "value"),
+                              Argument(handleType, "value"),
                               Argument("bool&", "tryNext"),
                               Argument("bool", "passedToJSImpl", default="false")],
                              inline=not ownsMembers,
                              bodyInHeader=not ownsMembers,
                              body=jsConversion)
 
     return {
         "name": name,
@@ -11278,24 +11313,33 @@ class CGProxySpecialOperation(CGPerSigna
             argument = arguments[1]
             info = getJSToNativeConversionInfo(
                 argument.type, descriptor,
                 treatNullAs=argument.treatNullAs,
                 sourceDescription=("value being assigned to %s setter" %
                                    descriptor.interface.identifier.name))
             if argumentHandleValue is None:
                 argumentHandleValue = "desc.value()"
+            rootedValue = fill(
+                """
+                JS::Rooted<JS::Value> rootedValue(cx, ${argumentHandleValue});
+                """,
+                argumentHandleValue = argumentHandleValue)
             templateValues = {
                 "declName": argument.identifier.name,
                 "holderName": argument.identifier.name + "_holder",
                 "val": argumentHandleValue,
+                "maybeMutableVal": "&rootedValue",
                 "obj": "obj",
                 "passedToJSImpl": "false"
             }
             self.cgRoot.prepend(instantiateJSToNativeConversion(info, templateValues))
+            # rootedValue needs to come before the conversion, so we
+            # need to prepend it last.
+            self.cgRoot.prepend(CGGeneric(rootedValue))
         elif operation.isGetter() or operation.isDeleter():
             if foundVar is None:
                 self.cgRoot.prepend(CGGeneric("bool found = false;\n"))
 
     def getArguments(self):
         args = [(a, a.identifier.name) for a in self.arguments]
         if self.idlNode.isGetter() or self.idlNode.isDeleter():
             args.append((FakeArgument(BuiltinTypes[IDLBuiltinType.Types.boolean],
@@ -13317,16 +13361,17 @@ class CGDictionary(CGThing):
         optional with default value, or optional without default
         value.  We set up a template in the 'conversion' variable for
         exactly how to do this, then substitute into it from the
         conversionReplacements dictionary.
         """
         member, conversionInfo = memberInfo
         replacements = {
             "val": "temp.ref()",
+            "maybeMutableVal": "temp.ptr()",
             "declName": self.makeMemberName(member.identifier.name),
             # We need a holder name for external interfaces, but
             # it's scoped down to the conversion so we can just use
             # anything we want.
             "holderName": "holder",
             "passedToJSImpl": "passedToJSImpl"
         }
         # We can't handle having a holderType here
--- a/dom/bindings/WebIDLGlobalNameHash.cpp
+++ b/dom/bindings/WebIDLGlobalNameHash.cpp
@@ -214,18 +214,24 @@ WebIDLGlobalNameHash::DefineIfEnabled(JS
   // or the window being touched.
   JS::Rooted<JSObject*> global(aCx,
     js::CheckedUnwrap(aObj, /* stopAtWindowProxy = */ false));
   if (!global) {
     return Throw(aCx, NS_ERROR_DOM_SECURITY_ERR);
   }
 
   {
+    // It's safe to pass "&global" here, because we've already unwrapped it, but
+    // for general sanity better to not have debug code even having the
+    // appearance of mutating things that opt code uses.
+#ifdef DEBUG
+    JS::Rooted<JSObject*> temp(aCx, global);
     DebugOnly<nsGlobalWindow*> win;
-    MOZ_ASSERT(NS_SUCCEEDED(UNWRAP_OBJECT(Window, global, win)));
+    MOZ_ASSERT(NS_SUCCEEDED(UNWRAP_OBJECT(Window, &temp, win)));
+#endif
   }
 
   if (checkEnabledForScope && !checkEnabledForScope(aCx, global)) {
     return true;
   }
 
   // The DOM constructor resolve machinery interacts with Xrays in tricky
   // ways, and there are some asymmetries that are important to understand.
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -206,18 +206,21 @@ StructuredCloneWriteCallback(JSContext* 
     MOZ_ASSERT(!cloneWriteInfo->mOffsetToKeyProp);
     cloneWriteInfo->mOffsetToKeyProp = js::GetSCOffset(aWriter);
 
     uint64_t value = 0;
     // Omit endian swap
     return JS_WriteBytes(aWriter, &value, sizeof(value));
   }
 
+  // UNWRAP_OBJECT calls might mutate this.
+  JS::Rooted<JSObject*> obj(aCx, aObj);
+
   IDBMutableFile* mutableFile;
-  if (NS_SUCCEEDED(UNWRAP_OBJECT(IDBMutableFile, aObj, mutableFile))) {
+  if (NS_SUCCEEDED(UNWRAP_OBJECT(IDBMutableFile, &obj, mutableFile))) {
     if (cloneWriteInfo->mDatabase->IsFileHandleDisabled()) {
       return false;
     }
 
     IDBDatabase* database = mutableFile->Database();
     MOZ_ASSERT(database);
 
     // Throw when trying to store IDBMutableFile objects that live in a
@@ -276,17 +279,17 @@ StructuredCloneWriteCallback(JSContext* 
     newFile->mMutableFile = mutableFile;
     newFile->mType = StructuredCloneFile::eMutableFile;
 
     return true;
   }
 
   {
     Blob* blob = nullptr;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, aObj, blob))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, &obj, blob))) {
       ErrorResult rv;
       uint64_t size = blob->GetSize(rv);
       MOZ_ASSERT(!rv.Failed());
 
       size = NativeEndian::swapToLittleEndian(size);
 
       nsString type;
       blob->GetType(type);
--- a/dom/media/WebVTTListener.cpp
+++ b/dom/media/WebVTTListener.cpp
@@ -152,18 +152,19 @@ WebVTTListener::OnDataAvailable(nsIReque
 
 NS_IMETHODIMP
 WebVTTListener::OnCue(JS::Handle<JS::Value> aCue, JSContext* aCx)
 {
   if (!aCue.isObject()) {
     return NS_ERROR_FAILURE;
   }
 
+  JS::Rooted<JSObject*> obj(aCx, &aCue.toObject());
   TextTrackCue* cue = nullptr;
-  nsresult rv = UNWRAP_OBJECT(VTTCue, &aCue.toObject(), cue);
+  nsresult rv = UNWRAP_OBJECT(VTTCue, &obj, cue);
   NS_ENSURE_SUCCESS(rv, rv);
 
   cue->SetTrackElement(mElement);
   mElement->mTrack->AddCue(*cue);
 
   return NS_OK;
 }
 
--- a/dom/messagechannel/MessagePort.cpp
+++ b/dom/messagechannel/MessagePort.cpp
@@ -397,23 +397,23 @@ MessagePort::PostMessage(JSContext* aCx,
                          ErrorResult& aRv)
 {
   // We *must* clone the data here, or the JS::Value could be modified
   // by script
 
   // Here we want to check if the transerable object list contains
   // this port.
   for (uint32_t i = 0; i < aTransferable.Length(); ++i) {
-    JSObject* object = aTransferable[i];
+    JS::Rooted<JSObject*> object(aCx, aTransferable[i]);
     if (!object) {
       continue;
     }
 
     MessagePort* port = nullptr;
-    nsresult rv = UNWRAP_OBJECT(MessagePort, object, port);
+    nsresult rv = UNWRAP_OBJECT(MessagePort, &object, port);
     if (NS_SUCCEEDED(rv) && port == this) {
       aRv.Throw(NS_ERROR_DOM_DATA_CLONE_ERR);
       return;
     }
   }
 
   JS::Rooted<JS::Value> transferable(aCx, JS::UndefinedValue());
 
--- a/dom/notification/Notification.cpp
+++ b/dom/notification/Notification.cpp
@@ -983,17 +983,17 @@ Notification::SetAlertName()
 // static
 already_AddRefed<Notification>
 Notification::Constructor(const GlobalObject& aGlobal,
                           const nsAString& aTitle,
                           const NotificationOptions& aOptions,
                           ErrorResult& aRv)
 {
   // FIXME(nsm): If the sticky flag is set, throw an error.
-  ServiceWorkerGlobalScope* scope = nullptr;
+  RefPtr<ServiceWorkerGlobalScope> scope;
   UNWRAP_OBJECT(ServiceWorkerGlobalScope, aGlobal.Get(), scope);
   if (scope) {
     aRv.ThrowTypeError<MSG_NOTIFICATION_NO_CONSTRUCTOR_IN_SERVICEWORKER>();
     return nullptr;
   }
 
   nsCOMPtr<nsIGlobalObject> global = do_QueryInterface(aGlobal.GetAsSupports());
   RefPtr<Notification> notification =
--- a/dom/presentation/PresentationSessionInfo.cpp
+++ b/dom/presentation/PresentationSessionInfo.cpp
@@ -1604,17 +1604,17 @@ PresentationPresentingInfo::ResolvedCall
   if (NS_WARN_IF(!obj)) {
     ReplyError(NS_ERROR_DOM_OPERATION_ERR);
     return;
   }
 
   // Start to listen to document state change event |STATE_TRANSFERRING|.
   // Use Element to support both HTMLIFrameElement and nsXULElement.
   Element* frame = nullptr;
-  nsresult rv = UNWRAP_OBJECT(Element, obj, frame);
+  nsresult rv = UNWRAP_OBJECT(Element, &obj, frame);
   if (NS_WARN_IF(!frame)) {
     ReplyError(NS_ERROR_DOM_OPERATION_ERR);
     return;
   }
 
   nsCOMPtr<nsIFrameLoaderOwner> owner = do_QueryInterface((nsIFrameLoaderOwner*) frame);
   if (NS_WARN_IF(!owner)) {
     ReplyError(NS_ERROR_DOM_OPERATION_ERR);
--- a/dom/promise/Promise.cpp
+++ b/dom/promise/Promise.cpp
@@ -279,24 +279,23 @@ enum class NativeHandlerTask : int32_t {
   Reject
 };
 
 static bool
 NativeHandlerCallback(JSContext* aCx, unsigned aArgc, JS::Value* aVp)
 {
   JS::CallArgs args = CallArgsFromVp(aArgc, aVp);
 
-  JS::Rooted<JS::Value> v(aCx,
-                          js::GetFunctionNativeReserved(&args.callee(),
-                                                        SLOT_NATIVEHANDLER));
+  JS::Value v = js::GetFunctionNativeReserved(&args.callee(),
+                                              SLOT_NATIVEHANDLER);
   MOZ_ASSERT(v.isObject());
 
+  JS::Rooted<JSObject*> obj(aCx, &v.toObject());
   PromiseNativeHandler* handler = nullptr;
-  if (NS_FAILED(UNWRAP_OBJECT(PromiseNativeHandler, &v.toObject(),
-                              handler))) {
+  if (NS_FAILED(UNWRAP_OBJECT(PromiseNativeHandler, &obj, handler))) {
     return Throw(aCx, NS_ERROR_UNEXPECTED);
   }
 
   v = js::GetFunctionNativeReserved(&args.callee(), SLOT_NATIVEHANDLER_TASK);
   NativeHandlerTask task = static_cast<NativeHandlerTask>(v.toInt32());
 
   if (task == NativeHandlerTask::Resolve) {
     handler->ResolvedCallback(aCx, args.get(0));
--- a/dom/workers/ScriptLoader.cpp
+++ b/dom/workers/ScriptLoader.cpp
@@ -1521,17 +1521,17 @@ CacheCreator::ResolvedCallback(JSContext
 
   if (!aValue.isObject()) {
     FailLoaders(NS_ERROR_FAILURE);
     return;
   }
 
   JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
   Cache* cache = nullptr;
-  nsresult rv = UNWRAP_OBJECT(Cache, obj, cache);
+  nsresult rv = UNWRAP_OBJECT(Cache, &obj, cache);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     FailLoaders(NS_ERROR_FAILURE);
     return;
   }
 
   mCache = cache;
   MOZ_DIAGNOSTIC_ASSERT(mCache);
 
@@ -1667,17 +1667,17 @@ CacheScriptLoader::ResolvedCallback(JSCo
     }
     return;
   }
 
   MOZ_ASSERT(aValue.isObject());
 
   JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
   mozilla::dom::Response* response = nullptr;
-  rv = UNWRAP_OBJECT(Response, obj, response);
+  rv = UNWRAP_OBJECT(Response, &obj, response);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     Fail(rv);
     return;
   }
 
   InternalHeaders* headers = response->GetInternalHeaders();
 
   IgnoredErrorResult ignored;
--- a/dom/workers/ServiceWorker.cpp
+++ b/dom/workers/ServiceWorker.cpp
@@ -30,19 +30,17 @@ namespace workers {
 
 bool
 ServiceWorkerVisible(JSContext* aCx, JSObject* aObj)
 {
   if (NS_IsMainThread()) {
     return Preferences::GetBool("dom.serviceWorkers.enabled", false);
   }
 
-  ServiceWorkerGlobalScope* scope = nullptr;
-  nsresult rv = UNWRAP_OBJECT(ServiceWorkerGlobalScope, aObj, scope);
-  return NS_SUCCEEDED(rv);
+  return IS_INSTANCE_OF(ServiceWorkerGlobalScope, aObj);
 }
 
 ServiceWorker::ServiceWorker(nsPIDOMWindowInner* aWindow,
                              ServiceWorkerInfo* aInfo)
   : DOMEventTargetHelper(aWindow),
     mInfo(aInfo)
 {
   AssertIsOnMainThread();
--- a/dom/workers/ServiceWorkerScriptCache.cpp
+++ b/dom/workers/ServiceWorkerScriptCache.cpp
@@ -391,25 +391,23 @@ public:
       }
 
       JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
       if (NS_WARN_IF(!obj)) {
         Fail(NS_ERROR_FAILURE);
         return;
       }
 
-      Cache* cache = nullptr;
+      RefPtr<Cache> cache;
       nsresult rv = UNWRAP_OBJECT(Cache, obj, cache);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         Fail(rv);
         return;
       }
 
-      // Just to be safe.
-      RefPtr<Cache> kungfuDeathGrip = cache;
       WriteToCache(cache);
       return;
     }
 
     MOZ_ASSERT(mState == WaitingForPut);
     mCallback->ComparisonResult(NS_OK, false /* aIsEqual */,
                                 mNewCacheName, mMaxScope);
     Cleanup();
@@ -914,23 +912,19 @@ CompareCache::ManageCacheResult(JSContex
   AssertIsOnMainThread();
 
   if (NS_WARN_IF(!aValue.isObject())) {
     mManager->CacheFinished(NS_ERROR_FAILURE, false);
     return;
   }
 
   JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
-  if (NS_WARN_IF(!obj)) {
-    mManager->CacheFinished(NS_ERROR_FAILURE, false);
-    return;
-  }
 
   Cache* cache = nullptr;
-  nsresult rv = UNWRAP_OBJECT(Cache, obj, cache);
+  nsresult rv = UNWRAP_OBJECT(Cache, &obj, cache);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     mManager->CacheFinished(rv, false);
     return;
   }
 
   RequestOrUSVString request;
   request.SetAsUSVString().Rebind(mURL.Data(), mURL.Length());
   ErrorResult error;
@@ -954,23 +948,19 @@ CompareCache::ManageValueResult(JSContex
   if (aValue.isUndefined()) {
     mManager->CacheFinished(NS_OK, false);
     return;
   }
 
   MOZ_ASSERT(aValue.isObject());
 
   JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
-  if (NS_WARN_IF(!obj)) {
-    mManager->CacheFinished(NS_ERROR_FAILURE, false);
-    return;
-  }
 
   Response* response = nullptr;
-  nsresult rv = UNWRAP_OBJECT(Response, obj, response);
+  nsresult rv = UNWRAP_OBJECT(Response, &obj, response);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     mManager->CacheFinished(rv, false);
     return;
   }
 
   MOZ_ASSERT(response->Ok());
 
   nsCOMPtr<nsIInputStream> inputStream;
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1043,21 +1043,21 @@ public:
         JS::Rooted<JSObject*> global(aCx, JS::CurrentGlobalOrNull(aCx));
         NS_ASSERTION(global, "This should never be null!");
 
         nsEventStatus status = nsEventStatus_eIgnore;
         nsIScriptGlobalObject* sgo;
 
         if (aWorkerPrivate) {
           WorkerGlobalScope* globalScope = nullptr;
-          UNWRAP_OBJECT(WorkerGlobalScope, global, globalScope);
+          UNWRAP_OBJECT(WorkerGlobalScope, &global, globalScope);
 
           if (!globalScope) {
             WorkerDebuggerGlobalScope* globalScope = nullptr;
-            UNWRAP_OBJECT(WorkerDebuggerGlobalScope, global, globalScope);
+            UNWRAP_OBJECT(WorkerDebuggerGlobalScope, &global, globalScope);
 
             MOZ_ASSERT_IF(globalScope, globalScope->GetWrapperPreserveColor() == global);
             if (globalScope || IsDebuggerSandbox(global)) {
               aWorkerPrivate->ReportErrorToDebugger(aReport.mFilename, aReport.mLineNumber,
                                                     aReport.mMessage);
               return;
             }
 
@@ -7114,18 +7114,19 @@ BEGIN_WORKERS_NAMESPACE
 
 WorkerCrossThreadDispatcher*
 GetWorkerCrossThreadDispatcher(JSContext* aCx, const JS::Value& aWorker)
 {
   if (!aWorker.isObject()) {
     return nullptr;
   }
 
+  JS::Rooted<JSObject*> obj(aCx, &aWorker.toObject());
   WorkerPrivate* w = nullptr;
-  UNWRAP_OBJECT(Worker, &aWorker.toObject(), w);
+  UNWRAP_OBJECT(Worker, &obj, w);
   MOZ_ASSERT(w);
   return w->GetCrossThreadDispatcher();
 }
 
 // Force instantiation.
 template class WorkerPrivateParent<WorkerPrivate>;
 
 END_WORKERS_NAMESPACE
--- a/dom/workers/WorkerScope.cpp
+++ b/dom/workers/WorkerScope.cpp
@@ -1049,27 +1049,23 @@ WorkerDebuggerGlobalScope::Dump(JSContex
   }
 }
 
 BEGIN_WORKERS_NAMESPACE
 
 bool
 IsWorkerGlobal(JSObject* object)
 {
-  nsIGlobalObject* globalObject = nullptr;
-  return NS_SUCCEEDED(UNWRAP_OBJECT(WorkerGlobalScope, object,
-                                    globalObject)) && !!globalObject;
+  return IS_INSTANCE_OF(WorkerGlobalScope, object);
 }
 
 bool
 IsDebuggerGlobal(JSObject* object)
 {
-  nsIGlobalObject* globalObject = nullptr;
-  return NS_SUCCEEDED(UNWRAP_OBJECT(WorkerDebuggerGlobalScope, object,
-                                    globalObject)) && !!globalObject;
+  return IS_INSTANCE_OF(WorkerDebuggerGlobalScope, object);
 }
 
 bool
 IsDebuggerSandbox(JSObject* object)
 {
   return SimpleGlobalObject::SimpleGlobalType(object) ==
     SimpleGlobalObject::GlobalType::WorkerDebuggerSandbox;
 }
--- a/dom/xbl/nsXBLProtoImplField.cpp
+++ b/dom/xbl/nsXBLProtoImplField.cpp
@@ -150,18 +150,17 @@ InstallXBLField(JSContext* cx,
   //
   // FieldAccessorGuard already determined whether |thisObj| was acceptable as
   // |this| in terms of not throwing a TypeError.  Assert this for good measure.
   MOZ_ASSERT(ValueHasISupportsPrivate(cx, JS::ObjectValue(*thisObj)));
 
   // But there are some cases where we must accept |thisObj| but not install a
   // property on it, or otherwise touch it.  Hence this split of |this|-vetting
   // duties.
-  nsISupports* native =
-    nsContentUtils::XPConnect()->GetNativeOfWrapper(cx, thisObj);
+  nsCOMPtr<nsISupports> native = xpc::UnwrapReflectorToISupports(thisObj);
   if (!native) {
     // Looks like whatever |thisObj| is it's not our nsIContent.  It might well
     // be the proto our binding installed, however, where the private is the
     // nsXBLDocumentInfo, so just baul out quietly.  Do NOT throw an exception
     // here.
     //
     // We could make this stricter by checking the class maybe, but whatever.
     return true;
@@ -410,18 +409,20 @@ nsXBLProtoImplField::InstallField(JS::Ha
   if (!jsapi.Init(globalObject)) {
     return NS_ERROR_UNEXPECTED;
   }
   MOZ_ASSERT(!::JS_IsExceptionPending(jsapi.cx()),
              "Shouldn't get here when an exception is pending!");
 
   JSAddonId* addonId = MapURIToAddonID(aBindingDocURI);
 
+  // Note: the UNWRAP_OBJECT may mutate boundNode; don't use it after that call.
+  JS::Rooted<JSObject*> boundNode(jsapi.cx(), aBoundNode);
   Element* boundElement = nullptr;
-  rv = UNWRAP_OBJECT(Element, aBoundNode, boundElement);
+  rv = UNWRAP_OBJECT(Element, &boundNode, boundElement);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // First, enter the xbl scope, build the element's scope chain, and use
   // that as the scope chain for the evaluation.
   JS::Rooted<JSObject*> scopeObject(jsapi.cx(),
     xpc::GetScopeForXBLExecution(jsapi.cx(), aBoundNode, addonId));
--- a/dom/xslt/xpath/XPathExpression.cpp
+++ b/dom/xslt/xpath/XPathExpression.cpp
@@ -70,17 +70,17 @@ already_AddRefed<XPathResult>
 XPathExpression::EvaluateWithContext(JSContext* aCx,
                                      nsINode& aContextNode,
                                      uint32_t aContextPosition,
                                      uint32_t aContextSize,
                                      uint16_t aType,
                                      JS::Handle<JSObject*> aInResult,
                                      ErrorResult& aRv)
 {
-    XPathResult* inResult = nullptr;
+    RefPtr<XPathResult> inResult;
     if (aInResult) {
         nsresult rv = UNWRAP_OBJECT(XPathResult, aInResult, inResult);
         if (NS_FAILED(rv) && rv != NS_ERROR_XPC_BAD_CONVERT_JS) {
             aRv.Throw(rv);
             return nullptr;
         }
     }
 
--- a/js/xpconnect/idl/nsIXPConnect.idl
+++ b/js/xpconnect/idl/nsIXPConnect.idl
@@ -408,20 +408,16 @@ interface nsIXPConnect : nsISupports
     /**
     * This only succeeds if the JSObject is a nsIXPConnectWrappedNative.
     * A new wrapper is *never* constructed.
     */
     nsIXPConnectWrappedNative
     getWrappedNativeOfJSObject(in JSContextPtr aJSContext,
                                in JSObjectPtr  aJSObj);
 
-    [noscript, notxpcom] nsISupports
-    getNativeOfWrapper(in JSContextPtr aJSContext,
-                       in JSObjectPtr  aJSObj);
-
     // Will return null if there is no JS stack right now.
     readonly attribute nsIStackFrame                CurrentJSStack;
     readonly attribute nsAXPCNativeCallContextPtr   CurrentNativeCallContext;
 
     void debugDump(in short depth);
     void debugDumpObject(in nsISupports aCOMObj, in short depth);
     void debugDumpJSStack(in boolean showArgs,
                           in boolean showLocals,
--- a/js/xpconnect/src/ExportHelpers.cpp
+++ b/js/xpconnect/src/ExportHelpers.cpp
@@ -8,16 +8,17 @@
 #include "WrapperFactory.h"
 #include "AccessCheck.h"
 #include "jsfriendapi.h"
 #include "jswrapper.h"
 #include "js/Proxy.h"
 #include "mozilla/dom/BindingUtils.h"
 #include "mozilla/dom/BlobBinding.h"
 #include "mozilla/dom/File.h"
+#include "mozilla/dom/FileListBinding.h"
 #include "mozilla/dom/StructuredCloneHolder.h"
 #include "nsGlobalWindow.h"
 #include "nsJSUtils.h"
 #include "nsIDOMFileList.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace JS;
@@ -47,23 +48,17 @@ enum StackScopedCloneTags {
 // case to maintain compatibility.
 //
 // FileList clones are supposed to give brand new objects, rather than
 // cross-compartment wrappers. For this, our current implementation relies on the
 // fact that these objects are implemented with XPConnect and have one reflector
 // per scope.
 bool IsFileList(JSObject* obj)
 {
-    nsISupports* supports = UnwrapReflectorToISupports(obj);
-    if (!supports)
-        return false;
-    nsCOMPtr<nsIDOMFileList> fileList = do_QueryInterface(supports);
-    if (fileList)
-        return true;
-    return false;
+    return IS_INSTANCE_OF(FileList, obj);
 }
 
 class MOZ_STACK_CLASS StackScopedCloneData
     : public StructuredCloneHolderBase
 {
 public:
     StackScopedCloneData(JSContext* aCx, StackScopedCloneOptions* aOptions)
         : mOptions(aOptions)
@@ -145,18 +140,19 @@ public:
         return nullptr;
     }
 
     bool CustomWriteHandler(JSContext* aCx,
                             JSStructuredCloneWriter* aWriter,
                             JS::Handle<JSObject*> aObj)
     {
         {
+            JS::Rooted<JSObject*> obj(aCx, aObj);
             Blob* blob = nullptr;
-            if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, aObj, blob))) {
+            if (NS_SUCCEEDED(UNWRAP_OBJECT(Blob, &obj, blob))) {
                 BlobImpl* blobImpl = blob->Impl();
                 MOZ_ASSERT(blobImpl);
 
                 if (!mBlobImpls.AppendElement(blobImpl))
                     return false;
 
                 size_t idx = mBlobImpls.Length() - 1;
                 return JS_WriteUint32Pair(aWriter, SCTAG_BLOB, 0) &&
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -1292,18 +1292,17 @@ ParsePrincipal(JSContext* cx, HandleStri
  * a script object principal (Document, Window).
  */
 static bool
 GetPrincipalOrSOP(JSContext* cx, HandleObject from, nsISupports** out)
 {
     MOZ_ASSERT(out);
     *out = nullptr;
 
-    nsXPConnect* xpc = nsXPConnect::XPConnect();
-    nsISupports* native = xpc->GetNativeOfWrapper(cx, from);
+    nsCOMPtr<nsISupports> native = xpc::UnwrapReflectorToISupports(from);
 
     if (nsCOMPtr<nsIScriptObjectPrincipal> sop = do_QueryInterface(native)) {
         sop.forget(out);
         return true;
     }
 
     nsCOMPtr<nsIPrincipal> principal = do_QueryInterface(native);
     principal.forget(out);
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -1702,22 +1702,20 @@ nsXPCComponents_Exception::CallOrConstru
 NS_IMETHODIMP
 nsXPCComponents_Exception::HasInstance(nsIXPConnectWrappedNative* wrapper,
                                        JSContext * cx, JSObject * obj,
                                        HandleValue val, bool* bp,
                                        bool* _retval)
 {
     using namespace mozilla::dom;
 
-    RootedValue v(cx, val);
     if (bp) {
-        Exception* e;
-        *bp = (v.isObject() &&
-               NS_SUCCEEDED(UNWRAP_OBJECT(Exception, &v.toObject(), e))) ||
-              JSValIsInterfaceOfType(cx, v, NS_GET_IID(nsIException));
+        *bp = (val.isObject() &&
+               IS_INSTANCE_OF(Exception, &val.toObject())) ||
+              JSValIsInterfaceOfType(cx, val, NS_GET_IID(nsIException));
     }
     return NS_OK;
 }
 
 /***************************************************************************/
 // This class is for the thing returned by "new Component.Constructor".
 
 // XXXjband we use this CID for security check, but security system can't see
@@ -2528,17 +2526,17 @@ NS_IMETHODIMP
 nsXPCComponents_Utils::ImportGlobalProperties(HandleValue aPropertyList,
                                               JSContext* cx)
 {
     RootedObject global(cx, CurrentGlobalOrNull(cx));
     MOZ_ASSERT(global);
 
     // Don't allow doing this if the global is a Window
     nsGlobalWindow* win;
-    if (NS_SUCCEEDED(UNWRAP_OBJECT(Window, global, win))) {
+    if (NS_SUCCEEDED(UNWRAP_OBJECT(Window, &global, win))) {
         return NS_ERROR_NOT_AVAILABLE;
     }
 
     GlobalProperties options;
     NS_ENSURE_TRUE(aPropertyList.isObject(), NS_ERROR_INVALID_ARG);
 
     RootedObject propertyList(cx, &aPropertyList.toObject());
     bool isArray;
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -909,17 +909,21 @@ XPCConvert::JSObject2NativeInterface(voi
         // If we're looking at a security wrapper, see now if we're allowed to
         // pass it to C++. If we are, then fall through to the code below. If
         // we aren't, throw an exception eagerly.
         //
         // NB: It's very important that we _don't_ unwrap in the aOuter case,
         // because the caller may explicitly want to create the XPCWrappedJS
         // around a security wrapper. XBL does this with Xrays from the XBL
         // scope - see nsBindingManager::GetBindingImplementation.
-        JSObject* inner = js::CheckedUnwrap(src, /* stopAtWindowProxy = */ false);
+        //
+        // It's also very important that "inner" be rooted here.
+        RootedObject inner(cx,
+                           js::CheckedUnwrap(src,
+                                             /* stopAtWindowProxy = */ false));
         if (!inner) {
             if (pErr)
                 *pErr = NS_ERROR_XPC_SECURITY_MANAGER_VETO;
             return false;
         }
 
         // Is this really a native xpcom object with a wrapper?
         XPCWrappedNative* wrappedNative = nullptr;
@@ -1119,17 +1123,17 @@ XPCConvert::JSValToXPCException(MutableH
             NS_ERROR("when is an object not an object?");
             return NS_ERROR_FAILURE;
         }
 
         // is this really a native xpcom object with a wrapper?
         JSObject* unwrapped = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
         if (!unwrapped)
             return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
-        if (nsISupports* supports = UnwrapReflectorToISupports(unwrapped)) {
+        if (nsCOMPtr<nsISupports> supports = UnwrapReflectorToISupports(unwrapped)) {
             nsCOMPtr<nsIException> iface = do_QueryInterface(supports);
             if (iface) {
                 // just pass through the exception (with extra ref and all)
                 nsCOMPtr<nsIException> temp = iface;
                 temp.forget(exceptn);
                 return NS_OK;
             } else {
                 // it is a wrapped native, but not an exception!
--- a/js/xpconnect/src/XPCJSID.cpp
+++ b/js/xpconnect/src/XPCJSID.cpp
@@ -490,17 +490,17 @@ xpc::HasInstance(JSContext* cx, HandleOb
         return rv;
 
     if (!obj)
         return NS_OK;
 
     if (mozilla::jsipc::IsCPOW(obj))
         return mozilla::jsipc::InstanceOf(obj, iid, bp);
 
-    nsISupports* identity = UnwrapReflectorToISupports(obj);
+    nsCOMPtr<nsISupports> identity = UnwrapReflectorToISupports(obj);
     if (!identity)
         return NS_OK;
 
     nsCOMPtr<nsISupports> supp;
     identity->QueryInterface(*iid, getter_AddRefs(supp));
     *bp = supp;
 
     // Our old HasInstance implementation operated by invoking FindTearOff on
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -525,17 +525,17 @@ CompilationScope()
 
 nsGlobalWindow*
 WindowOrNull(JSObject* aObj)
 {
     MOZ_ASSERT(aObj);
     MOZ_ASSERT(!js::IsWrapper(aObj));
 
     nsGlobalWindow* win = nullptr;
-    UNWRAP_OBJECT(Window, aObj, win);
+    UNWRAP_NON_WRAPPER_OBJECT(Window, aObj, win);
     return win;
 }
 
 nsGlobalWindow*
 WindowGlobalOrNull(JSObject* aObj)
 {
     MOZ_ASSERT(aObj);
     JSObject* glob = js::GetGlobalForObjectCrossCompartment(aObj);
@@ -2320,31 +2320,27 @@ class XPCJSRuntimeStats : public JS::Run
 
 
         for (size_t i = 0; i != zoneStatsVector.length(); ++i)
             delete static_cast<xpc::ZoneStatsExtras*>(zoneStatsVector[i].extra);
     }
 
     virtual void initExtraZoneStats(JS::Zone* zone, JS::ZoneStats* zStats) override {
         // Get the compartment's global.
-        nsXPConnect* xpc = nsXPConnect::XPConnect();
         AutoSafeJSContext cx;
         JSCompartment* comp = js::GetAnyCompartmentInZone(zone);
         xpc::ZoneStatsExtras* extras = new xpc::ZoneStatsExtras;
         extras->pathPrefix.AssignLiteral("explicit/js-non-window/zones/");
         RootedObject global(cx, JS_GetGlobalForCompartmentOrNull(cx, comp));
         if (global) {
-            // Need to enter the compartment, otherwise GetNativeOfWrapper()
-            // might crash.
-            JSAutoCompartment ac(cx, global);
-            nsISupports* native = xpc->GetNativeOfWrapper(cx, global);
-            if (nsCOMPtr<nsPIDOMWindowInner> piwindow = do_QueryInterface(native)) {
+            RefPtr<nsGlobalWindow> window;
+            if (NS_SUCCEEDED(UNWRAP_OBJECT(Window, global, window))) {
                 // The global is a |window| object.  Use the path prefix that
                 // we should have already created for it.
-                if (mTopWindowPaths->Get(piwindow->WindowID(),
+                if (mTopWindowPaths->Get(window->WindowID(),
                                          &extras->pathPrefix))
                     extras->pathPrefix.AppendLiteral("/js-");
             }
         }
 
         extras->pathPrefix += nsPrintfCString("zone(0x%p)/", (void*)zone);
 
         MOZ_ASSERT(StartsWithExplicit(extras->pathPrefix));
@@ -2365,29 +2361,25 @@ class XPCJSRuntimeStats : public JS::Run
                                    getter_AddRefs(extras->location));
             }
             // Note: cannot use amIAddonManager implementation at this point,
             // as it is a JS service and the JS heap is currently not idle.
             // Otherwise, we could have computed the add-on id at this point.
         }
 
         // Get the compartment's global.
-        nsXPConnect* xpc = nsXPConnect::XPConnect();
         AutoSafeJSContext cx;
         bool needZone = true;
         RootedObject global(cx, JS_GetGlobalForCompartmentOrNull(cx, c));
         if (global) {
-            // Need to enter the compartment, otherwise GetNativeOfWrapper()
-            // might crash.
-            JSAutoCompartment ac(cx, global);
-            nsISupports* native = xpc->GetNativeOfWrapper(cx, global);
-            if (nsCOMPtr<nsPIDOMWindowInner> piwindow = do_QueryInterface(native)) {
+            RefPtr<nsGlobalWindow> window;
+            if (NS_SUCCEEDED(UNWRAP_OBJECT(Window, global, window))) {
                 // The global is a |window| object.  Use the path prefix that
                 // we should have already created for it.
-                if (mWindowPaths->Get(piwindow->WindowID(),
+                if (mWindowPaths->Get(window->WindowID(),
                                       &extras->jsPathPrefix)) {
                     extras->domPathPrefix.Assign(extras->jsPathPrefix);
                     extras->domPathPrefix.AppendLiteral("/dom/");
                     extras->jsPathPrefix.AppendLiteral("/js-");
                     needZone = false;
                 } else {
                     extras->jsPathPrefix.AssignLiteral("explicit/js-non-window/zones/");
                     extras->domPathPrefix.AssignLiteral("explicit/dom/unknown-window-global?!/");
--- a/js/xpconnect/src/XPCJSWeakReference.cpp
+++ b/js/xpconnect/src/XPCJSWeakReference.cpp
@@ -22,18 +22,17 @@ nsresult xpcJSWeakReference::Init(JSCont
     if (!object.isObject())
         return NS_OK;
 
     JS::RootedObject obj(cx, &object.toObject());
 
     XPCCallContext ccx(cx);
 
     // See if the object is a wrapped native that supports weak references.
-    nsISupports* supports =
-        nsXPConnect::XPConnect()->GetNativeOfWrapper(cx, obj);
+    nsCOMPtr<nsISupports> supports = xpc::UnwrapReflectorToISupports(obj);
     nsCOMPtr<nsISupportsWeakReference> supportsWeakRef =
         do_QueryInterface(supports);
     if (supportsWeakRef) {
         supportsWeakRef->GetWeakReference(getter_AddRefs(mReferent));
         if (mReferent) {
             return NS_OK;
         }
     }
--- a/js/xpconnect/src/XPCWrappedJSClass.cpp
+++ b/js/xpconnect/src/XPCWrappedJSClass.cpp
@@ -247,18 +247,19 @@ nsXPCWrappedJSClass::CallQueryInterfaceO
         if (!success && JS_IsExceptionPending(cx)) {
             RootedValue jsexception(cx, NullValue());
 
             if (JS_GetPendingException(cx, &jsexception)) {
                 nsresult rv;
                 if (jsexception.isObject()) {
                     // XPConnect may have constructed an object to represent a
                     // C++ QI failure. See if that is the case.
+                    JS::Rooted<JSObject*> exceptionObj(cx, &jsexception.toObject());
                     Exception* e = nullptr;
-                    UNWRAP_OBJECT(Exception, &jsexception.toObject(), e);
+                    UNWRAP_OBJECT(Exception, &exceptionObj, e);
 
                     if (e &&
                         NS_SUCCEEDED(e->GetResult(&rv)) &&
                         rv == NS_NOINTERFACE) {
                         JS_ClearPendingException(cx);
                     }
                 } else if (jsexception.isNumber()) {
                     // JS often throws an nsresult.
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -686,17 +686,18 @@ XPC_WN_MaybeResolvingDeletePropertyStub(
     if (ccx.GetResolvingWrapper() == wrapper) {
         return result.succeed();
     }
     return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 // macro fun!
 #define PRE_HELPER_STUB                                                       \
-    JSObject* unwrapped = js::CheckedUnwrap(obj, false);                      \
+    /* It's very important for "unwrapped" to be rooted here.  */             \
+    RootedObject unwrapped(cx, js::CheckedUnwrap(obj, false));                \
     if (!unwrapped) {                                                         \
         JS_ReportErrorASCII(cx, "Permission denied to operate on object.");   \
         return false;                                                         \
     }                                                                         \
     if (!IS_WN_REFLECTOR(unwrapped)) {                                        \
         return Throw(NS_ERROR_XPC_BAD_OP_ON_WN_PROTO, cx);                    \
     }                                                                         \
     XPCWrappedNative* wrapper = XPCWrappedNative::Get(unwrapped);             \
--- a/js/xpconnect/src/nsXPConnect.cpp
+++ b/js/xpconnect/src/nsXPConnect.cpp
@@ -761,43 +761,39 @@ nsXPConnect::GetWrappedNativeOfJSObject(
         return NS_ERROR_FAILURE;
     }
 
     RefPtr<XPCWrappedNative> temp = XPCWrappedNative::Get(aJSObj);
     temp.forget(_retval);
     return NS_OK;
 }
 
-nsISupports*
+already_AddRefed<nsISupports>
 xpc::UnwrapReflectorToISupports(JSObject* reflector)
 {
     // Unwrap security wrappers, if allowed.
     reflector = js::CheckedUnwrap(reflector, /* stopAtWindowProxy = */ false);
     if (!reflector)
         return nullptr;
 
     // Try XPCWrappedNatives.
     if (IS_WN_REFLECTOR(reflector)) {
         XPCWrappedNative* wn = XPCWrappedNative::Get(reflector);
         if (!wn)
             return nullptr;
-        return wn->Native();
+        nsCOMPtr<nsISupports> native = wn->Native();
+        return native.forget();
     }
 
-    // Try DOM objects.
+    // Try DOM objects.  This QI without taking a ref first is safe, because
+    // this if non-null our thing will definitely be a DOM object, and we know
+    // their QI to nsISupports doesn't do anything weird.
     nsCOMPtr<nsISupports> canonical =
         do_QueryInterface(mozilla::dom::UnwrapDOMObjectToISupports(reflector));
-    return canonical;
-}
-
-NS_IMETHODIMP_(nsISupports*)
-nsXPConnect::GetNativeOfWrapper(JSContext* aJSContext,
-                                JSObject* aJSObj)
-{
-    return UnwrapReflectorToISupports(aJSObj);
+    return canonical.forget();
 }
 
 NS_IMETHODIMP
 nsXPConnect::GetWrappedNativeOfNativeObject(JSContext * aJSContext,
                                             JSObject * aScopeArg,
                                             nsISupports* aCOMObj,
                                             const nsIID & aIID,
                                             nsIXPConnectWrappedNative** _retval)
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -408,17 +408,17 @@ ReportJSRuntimeExplicitTreeStats(const J
  */
 bool
 Throw(JSContext* cx, nsresult rv);
 
 /**
  * Returns the nsISupports native behind a given reflector (either DOM or
  * XPCWN).
  */
-nsISupports*
+already_AddRefed<nsISupports>
 UnwrapReflectorToISupports(JSObject* reflector);
 
 /**
  * Singleton scopes for stuff that really doesn't fit anywhere else.
  *
  * If you find yourself wanting to use these compartments, you're probably doing
  * something wrong. Callers MUST consult with the XPConnect module owner before
  * using this compartment. If you don't, bholley will hunt you down.