Bug 1521907 part 5. Start using CheckedUnwrapStatic/Dynamic in XPConnect. r=peterv
☠☠ backed out by bcb403c04f1c ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Fri, 01 Feb 2019 18:49:04 +0000
changeset 456508 df09b7be63c5a642faf52ec29ff5da15d5ae4b04
parent 456507 585fa0024d46a6ee33266c11bbe07a40be1be470
child 456509 ac1c61bf61e990ea7100932636a096a1d09a049f
push id111656
push userdvarga@mozilla.com
push dateSat, 02 Feb 2019 09:51:54 +0000
treeherdermozilla-inbound@d8cebb3b46cf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv
bugs1521907
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1521907 part 5. Start using CheckedUnwrapStatic/Dynamic in XPConnect. r=peterv I am not a huge fan of the UnwrapReflectorToISupports setup here. Maybe we should introduce two differently-named methods that make it somewhat clear what the limitations of not taking a JSContext are? I couldn't think of sane naming... Differential Revision: https://phabricator.services.mozilla.com/D17885
dom/base/StructuredCloneHolder.cpp
dom/bindings/BindingUtils.cpp
dom/xbl/nsXBLProtoImplField.cpp
js/ipc/WrapperOwner.cpp
js/xpconnect/src/ExportHelpers.cpp
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCCallContext.cpp
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCConvert.cpp
js/xpconnect/src/XPCJSContext.cpp
js/xpconnect/src/XPCJSID.cpp
js/xpconnect/src/XPCJSWeakReference.cpp
js/xpconnect/src/XPCVariant.cpp
js/xpconnect/src/XPCWrappedNativeJSOps.cpp
js/xpconnect/src/nsXPConnect.cpp
js/xpconnect/src/xpcpublic.h
js/xpconnect/wrappers/XrayWrapper.cpp
toolkit/components/telemetry/core/Stopwatch.cpp
--- a/dom/base/StructuredCloneHolder.cpp
+++ b/dom/base/StructuredCloneHolder.cpp
@@ -463,18 +463,19 @@ void StructuredCloneHolder::ReadFromBuff
         xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
         return false;
       }
 
       return sct->WriteStructuredClone(aWriter);
     }
   }
 
-  if (NS_IsMainThread() && xpc::IsReflector(obj)) {
-    nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(obj);
+  if (NS_IsMainThread() && xpc::IsReflector(obj, aCx)) {
+    // We only care about principals, so ReflectorToISupportsStatic is fine.
+    nsCOMPtr<nsISupports> base = xpc::ReflectorToISupportsStatic(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
@@ -1037,17 +1038,18 @@ bool StructuredCloneHolder::CustomWriteH
       JS::IsWasmModuleObject(obj)) {
     RefPtr<JS::WasmModule> module = JS::GetWasmModule(obj);
     MOZ_ASSERT(module);
 
     return WriteWasmModule(aWriter, module, this);
   }
 
   {
-    nsCOMPtr<nsISupports> base = xpc::UnwrapReflectorToISupports(aObj);
+    // We only care about streams, so ReflectorToISupportsStatic is fine.
+    nsCOMPtr<nsISupports> base = xpc::ReflectorToISupportsStatic(aObj);
     nsCOMPtr<nsIInputStream> inputStream = do_QueryInterface(base);
     if (inputStream) {
       return WriteInputStream(aWriter, inputStream, this);
     }
   }
 
   return WriteFullySerializableObjects(aCx, aWriter, aObj);
 }
--- a/dom/bindings/BindingUtils.cpp
+++ b/dom/bindings/BindingUtils.cpp
@@ -2298,17 +2298,20 @@ nsISupports* GlobalObject::GetAsSupports
   // 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.
-  nsCOMPtr<nsISupports> supp = xpc::UnwrapReflectorToISupports(mGlobalJSObject);
+  //
+  // It's OK to use ReflectorToISupportsStatic, because we know we don't have a
+  // cross-compartment wrapper.
+  nsCOMPtr<nsISupports> supp = xpc::ReflectorToISupportsStatic(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,
@@ -3261,17 +3264,19 @@ bool CallerSubsumes(JSObject* aObject) {
 }
 
 nsresult UnwrapArgImpl(JSContext* cx, JS::Handle<JSObject*> src,
                        const nsIID& iid, void** ppArg) {
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  nsCOMPtr<nsISupports> iface = xpc::UnwrapReflectorToISupports(src);
+  // The JSContext represents the "who is unwrapping" realm, so we want to use
+  // it for ReflectorToISupportsDynamic here.
+  nsCOMPtr<nsISupports> iface = xpc::ReflectorToISupportsDynamic(src, cx);
   if (iface) {
     if (NS_FAILED(iface->QueryInterface(iid, ppArg))) {
       return NS_ERROR_XPC_BAD_CONVERT_JS;
     }
 
     return NS_OK;
   }
 
--- a/dom/xbl/nsXBLProtoImplField.cpp
+++ b/dom/xbl/nsXBLProtoImplField.cpp
@@ -136,17 +136,20 @@ static bool InstallXBLField(JSContext* c
   //
   // 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.
-  nsCOMPtr<nsISupports> native = xpc::UnwrapReflectorToISupports(thisObj);
+  //
+  // OK to use ReflectorToISupportsStatic, because we only care about nodes
+  // here.
+  nsCOMPtr<nsISupports> native = xpc::ReflectorToISupportsStatic(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;
--- a/js/ipc/WrapperOwner.cpp
+++ b/js/ipc/WrapperOwner.cpp
@@ -1029,17 +1029,19 @@ bool WrapperOwner::ok(JSContext* cx, con
   }
   return result.succeed();
 }
 
 // CPOWs can have a tag string attached to them, originating in the local
 // process from this function.  It's sent with the CPOW to the remote process,
 // where it can be fetched with Components.utils.getCrossProcessWrapperTag.
 static nsCString GetRemoteObjectTag(JS::Handle<JSObject*> obj) {
-  if (nsCOMPtr<nsISupports> supports = xpc::UnwrapReflectorToISupports(obj)) {
+  // OK to use ReflectorToISupportsStatic, because we only care about docshells
+  // and documents here.
+  if (nsCOMPtr<nsISupports> supports = xpc::ReflectorToISupportsStatic(obj)) {
     nsCOMPtr<nsIDocShellTreeItem> treeItem(do_QueryInterface(supports));
     if (treeItem) {
       return NS_LITERAL_CSTRING("ContentDocShellTreeItem");
     }
 
     nsCOMPtr<dom::Document> doc(do_QueryInterface(supports));
     if (doc) {
       return NS_LITERAL_CSTRING("ContentDocument");
--- a/js/xpconnect/src/ExportHelpers.cpp
+++ b/js/xpconnect/src/ExportHelpers.cpp
@@ -19,18 +19,18 @@
 #include "nsJSUtils.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace JS;
 
 namespace xpc {
 
-bool IsReflector(JSObject* obj) {
-  obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
+bool IsReflector(JSObject* obj, JSContext* cx) {
+  obj = js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false);
   if (!obj) {
     return false;
   }
   return IS_WN_REFLECTOR(obj) || dom::IsDOMObject(obj);
 }
 
 enum StackScopedCloneTags {
   SCTAG_BASE = JS_SCTAG_USER_MIN,
@@ -65,17 +65,18 @@ class MOZ_STACK_CLASS StackScopedCloneDa
 
       size_t idx;
       if (!JS_ReadBytes(aReader, &idx, sizeof(size_t))) {
         return nullptr;
       }
 
       RootedObject reflector(aCx, mReflectors[idx]);
       MOZ_ASSERT(reflector, "No object pointer?");
-      MOZ_ASSERT(IsReflector(reflector), "Object pointer must be a reflector!");
+      MOZ_ASSERT(IsReflector(reflector, aCx),
+                 "Object pointer must be a reflector!");
 
       if (!JS_WrapObject(aCx, &reflector)) {
         return nullptr;
       }
 
       return reflector;
     }
 
@@ -141,17 +142,18 @@ class MOZ_STACK_CLASS StackScopedCloneDa
         }
 
         size_t idx = mBlobImpls.Length() - 1;
         return JS_WriteUint32Pair(aWriter, SCTAG_BLOB, 0) &&
                JS_WriteBytes(aWriter, &idx, sizeof(size_t));
       }
     }
 
-    if ((mOptions->wrapReflectors && IsReflector(aObj)) || IsFileList(aObj)) {
+    if ((mOptions->wrapReflectors && IsReflector(aObj, aCx)) ||
+        IsFileList(aObj)) {
       if (!mReflectors.append(aObj)) {
         return false;
       }
 
       size_t idx = mReflectors.length() - 1;
       if (!JS_WriteUint32Pair(aWriter, SCTAG_REFLECTOR, 0)) {
         return false;
       }
@@ -389,18 +391,20 @@ bool ExportFunction(JSContext* cx, Handl
   if (hasOptions && !options.Parse()) {
     return false;
   }
 
   // Restrictions:
   // * We must subsume the scope we are exporting to.
   // * We must subsume the function being exported, because the function
   //   forwarder manually circumvents security wrapper CALL restrictions.
-  targetScope = js::CheckedUnwrap(targetScope);
-  funObj = js::CheckedUnwrap(funObj);
+  targetScope = js::CheckedUnwrapDynamic(targetScope, cx);
+  // For the function we can just CheckedUnwrapStatic, because if it's
+  // not callable we're going to fail out anyway.
+  funObj = js::CheckedUnwrapStatic(funObj);
   if (!targetScope || !funObj) {
     JS_ReportErrorASCII(cx, "Permission denied to export function into scope");
     return false;
   }
 
   if (js::IsScriptedProxy(targetScope)) {
     JS_ReportErrorASCII(cx, "Defining property on proxy object is not allowed");
     return false;
@@ -475,17 +479,18 @@ bool ExportFunction(JSContext* cx, Handl
 
 bool CreateObjectIn(JSContext* cx, HandleValue vobj,
                     CreateObjectInOptions& options, MutableHandleValue rval) {
   if (!vobj.isObject()) {
     JS_ReportErrorASCII(cx, "Expected an object as the target scope");
     return false;
   }
 
-  RootedObject scope(cx, js::CheckedUnwrap(&vobj.toObject()));
+  // cx represents the caller Realm.
+  RootedObject scope(cx, js::CheckedUnwrapDynamic(&vobj.toObject(), cx));
   if (!scope) {
     JS_ReportErrorASCII(
         cx, "Permission denied to create object in the target scope");
     return false;
   }
 
   bool define = !JSID_IS_VOID(options.defineAs);
 
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -340,18 +340,24 @@ static bool SandboxIsProxy(JSContext* cx
     return false;
   }
   if (!args[0].isObject()) {
     args.rval().setBoolean(false);
     return true;
   }
 
   RootedObject obj(cx, &args[0].toObject());
-  obj = js::CheckedUnwrap(obj);
-  NS_ENSURE_TRUE(obj, false);
+  // CheckedUnwrapStatic is OK here, since we only care about whether
+  // it's a scripted proxy and the things CheckedUnwrapStatic fails on
+  // are not.
+  obj = js::CheckedUnwrapStatic(obj);
+  if (!obj) {
+    args.rval().setBoolean(false);
+    return true;
+  }
 
   args.rval().setBoolean(js::IsScriptedProxy(obj));
   return true;
 }
 
 /*
  * Expected type of the arguments and the return value:
  * function exportFunction(function funToExport,
@@ -659,19 +665,20 @@ bool WrapAccessorFunction(JSContext* cx,
   if (!func) {
     return false;
   }
   op = JS_DATA_TO_FUNC_PTR(Op, func.get());
   return true;
 }
 
 static bool IsMaybeWrappedDOMConstructor(JSObject* obj) {
-  // We really care about the underlying object here, which might be wrapped
-  // in cross-compartment wrappers.
-  obj = js::CheckedUnwrap(obj);
+  // We really care about the underlying object here, which might be wrapped in
+  // cross-compartment wrappers.  CheckedUnwrapStatic is fine, since we just
+  // care whether it's a DOM constructor.
+  obj = js::CheckedUnwrapStatic(obj);
   if (!obj) {
     return false;
   }
 
   return dom::IsDOMConstructor(obj);
 }
 
 bool SandboxProxyHandler::getPropertyDescriptorImpl(
@@ -1130,17 +1137,21 @@ nsresult xpc::CreateSandboxObject(JSCont
       //
       // Note that, in the case of a window, we can't require that the
       // Sandbox subsumes the prototype, because we have to hold our
       // reference to it via an outer window, and the window may navigate
       // at any time. So we have to handle that case separately.
       bool useSandboxProxy =
           !!WindowOrNull(js::UncheckedUnwrap(options.proto, false));
       if (!useSandboxProxy) {
-        JSObject* unwrappedProto = js::CheckedUnwrap(options.proto, false);
+        // We just wrapped options.proto into the compartment of whatever Realm
+        // is on the cx, so use that same realm for the CheckedUnwrapDynamic
+        // call.
+        JSObject* unwrappedProto =
+            js::CheckedUnwrapDynamic(options.proto, cx, false);
         if (!unwrappedProto) {
           JS_ReportErrorASCII(cx, "Sandbox must subsume sandboxPrototype");
           return NS_ERROR_INVALID_ARG;
         }
         const js::Class* unwrappedClass = js::GetObjectClass(unwrappedProto);
         useSandboxProxy = IS_WN_CLASS(unwrappedClass) ||
                           mozilla::dom::IsDOMClass(Jsvalify(unwrappedClass));
       }
@@ -1259,17 +1270,18 @@ bool ParsePrincipal(JSContext* cx, Handl
  * For sandbox constructor the first argument can be a principal object or
  * a script object principal (Document, Window).
  */
 static bool GetPrincipalOrSOP(JSContext* cx, HandleObject from,
                               nsISupports** out) {
   MOZ_ASSERT(out);
   *out = nullptr;
 
-  nsCOMPtr<nsISupports> native = xpc::UnwrapReflectorToISupports(from);
+  // We might have a Window here, so need ReflectorToISupportsDynamic
+  nsCOMPtr<nsISupports> native = ReflectorToISupportsDynamic(from, cx);
 
   if (nsCOMPtr<nsIScriptObjectPrincipal> sop = do_QueryInterface(native)) {
     sop.forget(out);
     return true;
   }
 
   nsCOMPtr<nsIPrincipal> principal = do_QueryInterface(native);
   principal.forget(out);
@@ -1796,17 +1808,19 @@ nsresult nsXPCComponents_utils_Sandbox::
 
 nsresult xpc::EvalInSandbox(JSContext* cx, HandleObject sandboxArg,
                             const nsAString& source, const nsACString& filename,
                             int32_t lineNo, MutableHandleValue rval) {
   JS_AbortIfWrongThread(cx);
   rval.set(UndefinedValue());
 
   bool waiveXray = xpc::WrapperFactory::HasWaiveXrayFlag(sandboxArg);
-  RootedObject sandbox(cx, js::CheckedUnwrap(sandboxArg));
+  // CheckedUnwrapStatic is fine here, since we're checking for "is it a
+  // sandbox".
+  RootedObject sandbox(cx, js::CheckedUnwrapStatic(sandboxArg));
   if (!sandbox || !IsSandbox(sandbox)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   nsIScriptObjectPrincipal* sop =
       static_cast<nsIScriptObjectPrincipal*>(xpc_GetJSPrivate(sandbox));
   MOZ_ASSERT(sop, "Invalid sandbox passed");
   SandboxPrivate* priv = static_cast<SandboxPrivate*>(sop);
--- a/js/xpconnect/src/XPCCallContext.cpp
+++ b/js/xpconnect/src/XPCCallContext.cpp
@@ -56,17 +56,18 @@ XPCCallContext::XPCCallContext(
   }
 
   mMethodIndex = 0xDEAD;
 
   mState = HAVE_OBJECT;
 
   mTearOff = nullptr;
 
-  JSObject* unwrapped = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
+  JSObject* unwrapped =
+      js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false);
   if (!unwrapped) {
     JS_ReportErrorASCII(mJSContext,
                         "Permission denied to call method on |this|");
     mState = INIT_FAILED;
     return;
   }
   const js::Class* clasp = js::GetObjectClass(unwrapped);
   if (IS_WN_CLASS(clasp)) {
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -1524,34 +1524,36 @@ nsXPCComponents_Utils::GetUAWidgetScope(
 NS_IMETHODIMP
 nsXPCComponents_Utils::GetSandboxMetadata(HandleValue sandboxVal, JSContext* cx,
                                           MutableHandleValue rval) {
   if (!sandboxVal.isObject()) {
     return NS_ERROR_INVALID_ARG;
   }
 
   RootedObject sandbox(cx, &sandboxVal.toObject());
-  sandbox = js::CheckedUnwrap(sandbox);
+  // We only care about sandboxes here, so CheckedUnwrapStatic is fine.
+  sandbox = js::CheckedUnwrapStatic(sandbox);
   if (!sandbox || !xpc::IsSandbox(sandbox)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   return xpc::GetSandboxMetadata(cx, sandbox, rval);
 }
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::SetSandboxMetadata(HandleValue sandboxVal,
                                           HandleValue metadataVal,
                                           JSContext* cx) {
   if (!sandboxVal.isObject()) {
     return NS_ERROR_INVALID_ARG;
   }
 
   RootedObject sandbox(cx, &sandboxVal.toObject());
-  sandbox = js::CheckedUnwrap(sandbox);
+  // We only care about sandboxes here, so CheckedUnwrapStatic is fine.
+  sandbox = js::CheckedUnwrapStatic(sandbox);
   if (!sandbox || !xpc::IsSandbox(sandbox)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   nsresult rv = xpc::SetSandboxMetadata(cx, sandbox, metadataVal);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
@@ -1833,17 +1835,20 @@ nsXPCComponents_Utils::GetGlobalForObjec
 NS_IMETHODIMP
 nsXPCComponents_Utils::IsProxy(HandleValue vobj, JSContext* cx, bool* rval) {
   if (!vobj.isObject()) {
     *rval = false;
     return NS_OK;
   }
 
   RootedObject obj(cx, &vobj.toObject());
-  obj = js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
+  // We need to do a dynamic unwrap, because we apparently want to treat
+  // "failure to unwrap" differently from "not a proxy" (throw for the former,
+  // return false for the latter).
+  obj = js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false);
   NS_ENSURE_TRUE(obj, NS_ERROR_FAILURE);
 
   *rval = js::IsScriptedProxy(obj);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::ExportFunction(HandleValue vfunction, HandleValue vscope,
@@ -2312,17 +2317,18 @@ nsXPCComponents_Utils::GetJSEngineTeleme
 
 bool xpc::CloneInto(JSContext* aCx, HandleValue aValue, HandleValue aScope,
                     HandleValue aOptions, MutableHandleValue aCloned) {
   if (!aScope.isObject()) {
     return false;
   }
 
   RootedObject scope(aCx, &aScope.toObject());
-  scope = js::CheckedUnwrap(scope);
+  // The scope could be a Window, so we need to CheckedUnwrapDynamic.
+  scope = js::CheckedUnwrapDynamic(scope, aCx);
   if (!scope) {
     JS_ReportErrorASCII(aCx, "Permission denied to clone object into scope");
     return false;
   }
 
   if (!aOptions.isUndefined() && !aOptions.isObject()) {
     JS_ReportErrorASCII(aCx, "Invalid argument");
     return false;
@@ -2373,32 +2379,36 @@ nsXPCComponents_Utils::GetWebIDLCallerPr
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::GetObjectPrincipal(HandleValue val, JSContext* cx,
                                           nsIPrincipal** result) {
   if (!val.isObject()) {
     return NS_ERROR_INVALID_ARG;
   }
   RootedObject obj(cx, &val.toObject());
-  obj = js::CheckedUnwrap(obj);
+  // We need to be able to unwrap to WindowProxy or Location here, so
+  // use CheckedUnwrapDynamic.
+  obj = js::CheckedUnwrapDynamic(obj, cx);
   MOZ_ASSERT(obj);
 
   nsCOMPtr<nsIPrincipal> prin = nsContentUtils::ObjectPrincipal(obj);
   prin.forget(result);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::GetRealmLocation(HandleValue val, JSContext* cx,
                                         nsACString& result) {
   if (!val.isObject()) {
     return NS_ERROR_INVALID_ARG;
   }
   RootedObject obj(cx, &val.toObject());
-  obj = js::CheckedUnwrap(obj);
+  // We need to be able to unwrap to WindowProxy or Location here, so
+  // use CheckedUnwrapDynamic.
+  obj = js::CheckedUnwrapDynamic(obj, cx);
   MOZ_ASSERT(obj);
 
   result = xpc::RealmPrivate::Get(obj)->GetLocation();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsXPCComponents_Utils::ReadUTF8File(nsIFile* aFile, nsACString& aResult) {
--- a/js/xpconnect/src/XPCConvert.cpp
+++ b/js/xpconnect/src/XPCConvert.cpp
@@ -1064,19 +1064,19 @@ bool XPCConvert::JSObject2NativeInterfac
     // 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.
     //
     // It's also very important that "inner" be rooted here.
-    RootedObject inner(RootingCx(),
-                       js::CheckedUnwrap(src,
-                                         /* stopAtWindowProxy = */ false));
+    RootedObject inner(
+        cx, js::CheckedUnwrapDynamic(src, cx,
+                                     /* 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?
@@ -1273,22 +1273,24 @@ nsresult XPCConvert::JSValToXPCException
 
     if (!obj) {
       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);
+        js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false);
     if (!unwrapped) {
       return NS_ERROR_XPC_SECURITY_MANAGER_VETO;
     }
+    // It's OK to use ReflectorToISupportsStatic, because we have already
+    // stripped off wrappers.
     if (nsCOMPtr<nsISupports> supports =
-            UnwrapReflectorToISupports(unwrapped)) {
+            ReflectorToISupportsStatic(unwrapped)) {
       nsCOMPtr<Exception> iface = do_QueryInterface(supports);
       if (iface) {
         // just pass through the exception (with extra ref and all)
         iface.forget(exceptn);
         return NS_OK;
       }
 
       // it is a wrapped native, but not an exception!
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -654,17 +654,18 @@ bool XPCJSContext::InterruptCallback(JSC
     // If this is a sandbox associated with a DOMWindow via a
     // sandboxPrototype, use that DOMWindow. This supports GreaseMonkey
     // and JetPack content scripts.
     JS::Rooted<JSObject*> proto(cx);
     if (!JS_GetPrototype(cx, global, &proto)) {
       return false;
     }
     if (proto && xpc::IsSandboxPrototypeProxy(proto) &&
-        (proto = js::CheckedUnwrap(proto, /* stopAtWindowProxy = */ false))) {
+        (proto = js::CheckedUnwrapDynamic(proto, cx,
+                                          /* stopAtWindowProxy = */ false))) {
       win = WindowGlobalOrNull(proto);
     }
   }
 
   if (!win) {
     NS_WARNING("No active window");
     return true;
   }
--- a/js/xpconnect/src/XPCJSID.cpp
+++ b/js/xpconnect/src/XPCJSID.cpp
@@ -142,17 +142,18 @@ static JSObject* GetIDPrototype(JSContex
   }
 
   MOZ_CRASH("Unrecognized ID Object Class");
 }
 
 // Unwrap the given value to an object with the correct class, or nullptr.
 static JSObject* GetIDObject(HandleValue aVal, const Class* aClass) {
   if (aVal.isObject()) {
-    JSObject* obj = js::CheckedUnwrap(&aVal.toObject());
+    // We care only about IID/CID objects here, so CheckedUnwrapStatic is fine.
+    JSObject* obj = js::CheckedUnwrapStatic(&aVal.toObject());
     if (obj && js::GetObjectClass(obj) == aClass) {
       return obj;
     }
   }
   return nullptr;
 }
 
 /**
@@ -163,18 +164,23 @@ static JSObject* GetIDObject(HandleValue
  * and for ContractID objects, the ContractID's corresponding CID will be looked
  * up.
  */
 Maybe<nsID> JSValue2ID(JSContext* aCx, HandleValue aVal) {
   if (!aVal.isObject()) {
     return Nothing();
   }
 
+  // We only care about ID objects here, so CheckedUnwrapStatic is fine.
+  RootedObject obj(aCx, js::CheckedUnwrapStatic(&aVal.toObject()));
+  if (!obj) {
+    return Nothing();
+  }
+
   mozilla::Maybe<nsID> id;
-  RootedObject obj(aCx, js::CheckedUnwrap(&aVal.toObject()));
   if (js::GetObjectClass(obj) == &sID_Class) {
     // Extract the raw bytes of the nsID from reserved slots.
     uint32_t rawid[] = {js::GetReservedSlot(obj, kID_Slot0).toPrivateUint32(),
                         js::GetReservedSlot(obj, kID_Slot1).toPrivateUint32(),
                         js::GetReservedSlot(obj, kID_Slot2).toPrivateUint32(),
                         js::GetReservedSlot(obj, kID_Slot3).toPrivateUint32()};
 
     // Construct a nsID inside the Maybe, and copy the rawid into it.
@@ -362,18 +368,21 @@ static bool ID_Equals(JSContext* aCx, un
  * This static method handles both complexities, returning either an XPCWN, a
  * DOM object, or null. The object may well be cross-compartment from |cx|.
  */
 static nsresult FindObjectForHasInstance(JSContext* cx, HandleObject objArg,
                                          MutableHandleObject target) {
   using namespace mozilla::jsipc;
   RootedObject obj(cx, objArg), proto(cx);
   while (true) {
-    // Try the object, or the wrappee if allowed.
-    JSObject* o = js::IsWrapper(obj) ? js::CheckedUnwrap(obj, false) : obj;
+    // Try the object, or the wrappee if allowed.  We want CheckedUnwrapDynamic
+    // here, because we might in fact be looking for a Window.  "cx" represents
+    // our current global.
+    JSObject* o =
+        js::IsWrapper(obj) ? js::CheckedUnwrapDynamic(obj, cx, false) : obj;
     if (o && (IS_WN_REFLECTOR(o) || IsDOMObject(o) || IsCPOW(o))) {
       target.set(o);
       return NS_OK;
     }
 
     // Walk the prototype chain from the perspective of the callee (i.e.
     // respecting Xrays if they exist).
     if (!js::GetObjectProto(cx, obj, &proto)) {
@@ -400,17 +409,18 @@ nsresult HasInstance(JSContext* cx, Hand
   if (!obj) {
     return NS_OK;
   }
 
   if (mozilla::jsipc::IsCPOW(obj)) {
     return mozilla::jsipc::InstanceOf(obj, iid, bp);
   }
 
-  nsCOMPtr<nsISupports> identity = UnwrapReflectorToISupports(obj);
+  // Need to unwrap Window correctly here, so use ReflectorToISupportsDynamic.
+  nsCOMPtr<nsISupports> identity = ReflectorToISupportsDynamic(obj, cx);
   if (!identity) {
     return NS_OK;
   }
 
   nsCOMPtr<nsISupports> supp;
   identity->QueryInterface(*iid, getter_AddRefs(supp));
   *bp = supp;
 
--- a/js/xpconnect/src/XPCJSWeakReference.cpp
+++ b/js/xpconnect/src/XPCJSWeakReference.cpp
@@ -20,17 +20,17 @@ nsresult xpcJSWeakReference::Init(JSCont
     return NS_OK;
   }
 
   JS::RootedObject obj(cx, &object.toObject());
 
   XPCCallContext ccx(cx);
 
   // See if the object is a wrapped native that supports weak references.
-  nsCOMPtr<nsISupports> supports = xpc::UnwrapReflectorToISupports(obj);
+  nsCOMPtr<nsISupports> supports = xpc::ReflectorToISupportsDynamic(obj, cx);
   nsCOMPtr<nsISupportsWeakReference> supportsWeakRef =
       do_QueryInterface(supports);
   if (supportsWeakRef) {
     supportsWeakRef->GetWeakReference(getter_AddRefs(mReferent));
     if (mReferent) {
       return NS_OK;
     }
   }
--- a/js/xpconnect/src/XPCVariant.cpp
+++ b/js/xpconnect/src/XPCVariant.cpp
@@ -39,20 +39,21 @@ XPCVariant::XPCVariant(JSContext* cx, co
     // be a problem anymore because SetParentToWindow will do the right
     // thing, but I'm saving the cleanup here for another day. Blake thinks
     // that we should just not store the WN if we're creating a variant for
     // an outer window.
     JSObject* obj = js::ToWindowIfWindowProxy(&mJSVal.toObject());
     mJSVal = JS::ObjectValue(*obj);
 
     JSObject* unwrapped =
-        js::CheckedUnwrap(obj, /* stopAtWindowProxy = */ false);
+        js::CheckedUnwrapDynamic(obj, cx, /* stopAtWindowProxy = */ false);
     mReturnRawObject = !(unwrapped && IS_WN_REFLECTOR(unwrapped));
-  } else
+  } else {
     mReturnRawObject = false;
+  }
 }
 
 XPCTraceableVariant::~XPCTraceableVariant() {
   Value val = GetJSValPreserveColor();
 
   MOZ_ASSERT(val.isGCThing() || val.isNull(), "Must be traceable or unlinked");
 
   mData.Cleanup();
--- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp
@@ -698,17 +698,17 @@ bool XPC_WN_MaybeResolvingDeleteProperty
     return result.succeed();
   }
   return Throw(NS_ERROR_XPC_CANT_MODIFY_PROP_ON_WN, cx);
 }
 
 // macro fun!
 #define PRE_HELPER_STUB                                                 \
   /* It's very important for "unwrapped" to be rooted here.  */         \
-  RootedObject unwrapped(cx, js::CheckedUnwrap(obj, false));            \
+  RootedObject unwrapped(cx, js::CheckedUnwrapDynamic(obj, cx, 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
@@ -677,31 +677,29 @@ NS_IMETHODIMP
 nsXPConnect::GetWrappedNativeOfJSObject(JSContext* aJSContext,
                                         JSObject* aJSObjArg,
                                         nsIXPConnectWrappedNative** _retval) {
   MOZ_ASSERT(aJSContext, "bad param");
   MOZ_ASSERT(aJSObjArg, "bad param");
   MOZ_ASSERT(_retval, "bad param");
 
   RootedObject aJSObj(aJSContext, aJSObjArg);
-  aJSObj = js::CheckedUnwrap(aJSObj, /* stopAtWindowProxy = */ false);
+  aJSObj = js::CheckedUnwrapDynamic(aJSObj, aJSContext,
+                                    /* stopAtWindowProxy = */ false);
   if (!aJSObj || !IS_WN_REFLECTOR(aJSObj)) {
     *_retval = nullptr;
     return NS_ERROR_FAILURE;
   }
 
   RefPtr<XPCWrappedNative> temp = XPCWrappedNative::Get(aJSObj);
   temp.forget(_retval);
   return NS_OK;
 }
 
-already_AddRefed<nsISupports> xpc::UnwrapReflectorToISupports(
-    JSObject* reflector) {
-  // Unwrap security wrappers, if allowed.
-  reflector = js::CheckedUnwrap(reflector, /* stopAtWindowProxy = */ false);
+static already_AddRefed<nsISupports> ReflectorToISupports(JSObject* reflector) {
   if (!reflector) {
     return nullptr;
   }
 
   // Try XPCWrappedNatives.
   if (IS_WN_REFLECTOR(reflector)) {
     XPCWrappedNative* wn = XPCWrappedNative::Get(reflector);
     if (!wn) {
@@ -714,16 +712,30 @@ already_AddRefed<nsISupports> xpc::Unwra
   // 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.forget();
 }
 
+already_AddRefed<nsISupports> xpc::ReflectorToISupportsStatic(
+    JSObject* reflector) {
+  // Unwrap security wrappers, if allowed.
+  return ReflectorToISupports(js::CheckedUnwrapStatic(reflector));
+}
+
+already_AddRefed<nsISupports> xpc::ReflectorToISupportsDynamic(
+    JSObject* reflector, JSContext* cx) {
+  // Unwrap security wrappers, if allowed.
+  return ReflectorToISupports(
+      js::CheckedUnwrapDynamic(reflector, cx,
+                               /* stopAtWindowProxy = */ false));
+}
+
 NS_IMETHODIMP
 nsXPConnect::CreateSandbox(JSContext* cx, nsIPrincipal* principal,
                            JSObject** _retval) {
   *_retval = nullptr;
 
   RootedValue rval(cx);
   SandboxOptions options;
   nsresult rv = CreateSandboxObject(cx, &rval, principal, options);
--- a/js/xpconnect/src/xpcpublic.h
+++ b/js/xpconnect/src/xpcpublic.h
@@ -134,17 +134,21 @@ bool AllowContentXBLScope(JS::Realm* rea
 bool UseContentXBLScope(JS::Realm* realm);
 
 // Clear out the content XBL scope (if any) on the given global.  This will
 // force creation of a new one if one is needed again.
 void ClearContentXBLScope(JSObject* global);
 
 bool IsSandboxPrototypeProxy(JSObject* obj);
 
-bool IsReflector(JSObject* obj);
+// The JSContext argument represents the Realm that's asking the question.  This
+// is needed to properly answer without exposing information unnecessarily
+// from behind security wrappers.  There will be no exceptions thrown on this
+// JSContext.
+bool IsReflector(JSObject* obj, JSContext* cx);
 
 bool IsXrayWrapper(JSObject* obj);
 
 // If this function was created for a given XrayWrapper, returns the global of
 // the Xrayed object. Otherwise, returns the global of the function.
 //
 // To emphasize the obvious: the return value here is not necessarily same-
 // compartment with the argument.
@@ -455,19 +459,34 @@ void ReportJSRuntimeExplicitTreeStats(co
 
 /**
  * Throws an exception on cx and returns false.
  */
 bool Throw(JSContext* cx, nsresult rv);
 
 /**
  * Returns the nsISupports native behind a given reflector (either DOM or
- * XPCWN).
+ * XPCWN).  If a non-reflector object is passed in, null will be returned.
+ *
+ * This function will not correctly handle Window or Location objects behind
+ * cross-compartment wrappers: it will return null.  If you care about getting
+ * non-null for Window or Location, use ReflectorToISupportsDynamic.
  */
-already_AddRefed<nsISupports> UnwrapReflectorToISupports(JSObject* reflector);
+already_AddRefed<nsISupports> ReflectorToISupportsStatic(JSObject* reflector);
+
+/**
+ * Returns the nsISupports native behind a given reflector (either DOM or
+ * XPCWN).  If a non-reflector object is passed in, null will be returned.
+ *
+ * The JSContext argument represents the Realm that's asking for the
+ * nsISupports.  This is needed to properly handle Window and Location objects,
+ * which do dynamic security checks.
+ */
+already_AddRefed<nsISupports> ReflectorToISupportsDynamic(JSObject* reflector,
+                                                          JSContext* cx);
 
 /**
  * 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.
  */
--- a/js/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/xpconnect/wrappers/XrayWrapper.cpp
@@ -28,17 +28,17 @@
 #include "mozilla/dom/XrayExpandoClass.h"
 #include "nsGlobalWindow.h"
 
 using namespace mozilla::dom;
 using namespace JS;
 using namespace mozilla;
 
 using js::BaseProxyHandler;
-using js::CheckedUnwrap;
+using js::CheckedUnwrapStatic;
 using js::IsCrossCompartmentWrapper;
 using js::UncheckedUnwrap;
 using js::Wrapper;
 
 namespace xpc {
 
 using namespace XrayUtils;
 
@@ -1929,23 +1929,25 @@ static bool RecreateLostWaivers(JSContex
   if (valueWasWaived &&
       !IsCrossCompartmentWrapper(&wrapped.value().toObject())) {
     rewaived = &wrapped.value().toObject();
     rewaived = WrapperFactory::WaiveXray(cx, UncheckedUnwrap(rewaived));
     NS_ENSURE_TRUE(rewaived, false);
     wrapped.value().set(ObjectValue(*rewaived));
   }
   if (getterWasWaived && !IsCrossCompartmentWrapper(wrapped.getterObject())) {
-    MOZ_ASSERT(CheckedUnwrap(wrapped.getterObject()));
+    // We can't end up with WindowProxy or Location as getters.
+    MOZ_ASSERT(CheckedUnwrapStatic(wrapped.getterObject()));
     rewaived = WrapperFactory::WaiveXray(cx, wrapped.getterObject());
     NS_ENSURE_TRUE(rewaived, false);
     wrapped.setGetterObject(rewaived);
   }
   if (setterWasWaived && !IsCrossCompartmentWrapper(wrapped.setterObject())) {
-    MOZ_ASSERT(CheckedUnwrap(wrapped.setterObject()));
+    // We can't end up with WindowProxy or Location as setters.
+    MOZ_ASSERT(CheckedUnwrapStatic(wrapped.setterObject()));
     rewaived = WrapperFactory::WaiveXray(cx, wrapped.setterObject());
     NS_ENSURE_TRUE(rewaived, false);
     wrapped.setSetterObject(rewaived);
   }
 
   return true;
 }
 
--- a/toolkit/components/telemetry/core/Stopwatch.cpp
+++ b/toolkit/components/telemetry/core/Stopwatch.cpp
@@ -20,17 +20,18 @@
 #include "nsRefPtrHashtable.h"
 #include "nsString.h"
 #include "xpcpublic.h"
 
 using mozilla::dom::AutoJSAPI;
 
 static inline nsQueryObject<nsISupports> do_QueryReflector(
     JSObject* aReflector) {
-  nsCOMPtr<nsISupports> reflector = xpc::UnwrapReflectorToISupports(aReflector);
+  // None of the types we query to are implemented by Window or Location.
+  nsCOMPtr<nsISupports> reflector = xpc::ReflectorToISupportsStatic(aReflector);
   return do_QueryObject(reflector);
 }
 
 static inline nsQueryObject<nsISupports> do_QueryReflector(
     const JS::Value& aReflector) {
   return do_QueryReflector(&aReflector.toObject());
 }