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

--HG--
extra : moz-landing-system : lando

dom/base/StructuredCloneHolder.cpp

@@ -468,8 +468,9 @@ void StructuredCloneHolder::ReadFromBuffer(nsISupports* aParent, JSContext* aCx,
     }
   }
 
-  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);
@@ -1042,7 +1043,8 @@ bool StructuredCloneHolder::CustomWriteHandler(JSContext* aCx,
   }
 
   {
-    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);

dom/bindings/BindingUtils.cpp

@@ -2303,7 +2303,10 @@ nsISupports* GlobalObject::GetAsSupports() const {
   // 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;
@@ -3266,7 +3269,9 @@ nsresult UnwrapArgImpl(JSContext* cx, JS::Handle<JSObject*> src,
     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;

dom/xbl/nsXBLProtoImplField.cpp

@@ -141,7 +141,10 @@ static bool InstallXBLField(JSContext* cx, JS::Handle<JSObject*> callee,
   // 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

js/ipc/WrapperOwner.cpp

@@ -1034,7 +1034,9 @@ bool WrapperOwner::ok(JSContext* cx, const ReturnStatus& status,
 // 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");

js/xpconnect/src/ExportHelpers.cpp

@@ -24,8 +24,8 @@ 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;
   }
@@ -70,7 +70,8 @@ class MOZ_STACK_CLASS StackScopedCloneData : public StructuredCloneHolderBase {
 
       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;
@@ -146,7 +147,8 @@ class MOZ_STACK_CLASS StackScopedCloneData : public StructuredCloneHolderBase {
       }
     }
 
-    if ((mOptions->wrapReflectors && IsReflector(aObj)) || IsFileList(aObj)) {
+    if ((mOptions->wrapReflectors && IsReflector(aObj, aCx)) ||
+        IsFileList(aObj)) {
       if (!mReflectors.append(aObj)) {
         return false;
       }
@@ -394,8 +396,10 @@ bool ExportFunction(JSContext* cx, HandleValue vfunction, HandleValue vscope,
   // * 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;
@@ -480,7 +484,8 @@ bool CreateObjectIn(JSContext* cx, HandleValue vobj,
     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");

js/xpconnect/src/Sandbox.cpp

@@ -345,8 +345,14 @@ static bool SandboxIsProxy(JSContext* cx, unsigned argc, Value* vp) {
   }
 
   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;
@@ -664,9 +670,10 @@ bool WrapAccessorFunction(JSContext* cx, Op& op, PropertyDescriptor* desc,
 }
 
 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;
   }
@@ -1135,7 +1142,11 @@ nsresult xpc::CreateSandboxObject(JSContext* cx, MutableHandleValue vp,
       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;
@@ -1264,7 +1275,8 @@ static bool GetPrincipalOrSOP(JSContext* cx, HandleObject from,
   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);
@@ -1801,7 +1813,9 @@ nsresult xpc::EvalInSandbox(JSContext* cx, HandleObject sandboxArg,
   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;
   }

js/xpconnect/src/XPCCallContext.cpp

@@ -61,7 +61,8 @@ XPCCallContext::XPCCallContext(
 
   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|");

js/xpconnect/src/XPCComponents.cpp

@@ -1529,7 +1529,8 @@ nsXPCComponents_Utils::GetSandboxMetadata(HandleValue sandboxVal, JSContext* cx,
   }
 
   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;
   }
@@ -1546,7 +1547,8 @@ nsXPCComponents_Utils::SetSandboxMetadata(HandleValue sandboxVal,
   }
 
   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;
   }
@@ -1838,7 +1840,10 @@ nsXPCComponents_Utils::IsProxy(HandleValue vobj, JSContext* cx, bool* rval) {
   }
 
   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);
@@ -2317,7 +2322,8 @@ bool xpc::CloneInto(JSContext* aCx, HandleValue aValue, HandleValue aScope,
   }
 
   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;
@@ -2378,7 +2384,9 @@ nsXPCComponents_Utils::GetObjectPrincipal(HandleValue val, JSContext* cx,
     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);
@@ -2393,7 +2401,9 @@ nsXPCComponents_Utils::GetRealmLocation(HandleValue val, JSContext* cx,
     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();

js/xpconnect/src/XPCConvert.cpp

@@ -1069,9 +1069,9 @@ bool XPCConvert::JSObject2NativeInterface(JSContext* cx, void** dest,
     // 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;
@@ -1278,12 +1278,14 @@ nsresult XPCConvert::JSValToXPCException(MutableHandleValue s,
 
     // 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)

js/xpconnect/src/XPCJSContext.cpp

@@ -659,7 +659,8 @@ bool XPCJSContext::InterruptCallback(JSContext* cx) {
       return false;
     }
     if (proto && xpc::IsSandboxPrototypeProxy(proto) &&
-        (proto = js::CheckedUnwrap(proto, /* stopAtWindowProxy = */ false))) {
+        (proto = js::CheckedUnwrapDynamic(proto, cx,
+                                          /* stopAtWindowProxy = */ false))) {
       win = WindowGlobalOrNull(proto);
     }
   }

js/xpconnect/src/XPCJSID.cpp

@@ -147,7 +147,8 @@ static JSObject* GetIDPrototype(JSContext* aCx, const js::Class* aClass) {
 // 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;
     }
@@ -168,8 +169,13 @@ Maybe<nsID> JSValue2ID(JSContext* aCx, HandleValue aVal) {
     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(),
@@ -367,8 +373,11 @@ static nsresult FindObjectForHasInstance(JSContext* cx, HandleObject objArg,
   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;
@@ -405,7 +414,8 @@ nsresult HasInstance(JSContext* cx, HandleObject objArg, const nsID* iid,
     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;
   }

js/xpconnect/src/XPCJSWeakReference.cpp

@@ -25,7 +25,7 @@ nsresult xpcJSWeakReference::Init(JSContext* cx, const JS::Value& object) {
   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) {

js/xpconnect/src/XPCVariant.cpp

@@ -44,10 +44,11 @@ XPCVariant::XPCVariant(JSContext* cx, const Value& aJSVal)
     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() {

js/xpconnect/src/XPCWrappedNativeJSOps.cpp

@@ -703,7 +703,7 @@ bool XPC_WN_MaybeResolvingDeletePropertyStub(JSContext* cx, HandleObject obj,
 // 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;                                                       \

js/xpconnect/src/nsXPConnect.cpp

@@ -682,7 +682,8 @@ nsXPConnect::GetWrappedNativeOfJSObject(JSContext* aJSContext,
   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;
@@ -693,10 +694,7 @@ nsXPConnect::GetWrappedNativeOfJSObject(JSContext* aJSContext,
   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;
   }
@@ -719,6 +717,20 @@ already_AddRefed<nsISupports> xpc::UnwrapReflectorToISupports(
   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) {

js/xpconnect/src/xpcpublic.h

@@ -139,7 +139,11 @@ 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);
 
@@ -460,9 +464,24 @@ 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.

js/xpconnect/wrappers/XrayWrapper.cpp

@@ -33,7 +33,7 @@ using namespace JS;
 using namespace mozilla;
 
 using js::BaseProxyHandler;
-using js::CheckedUnwrap;
+using js::CheckedUnwrapStatic;
 using js::IsCrossCompartmentWrapper;
 using js::UncheckedUnwrap;
 using js::Wrapper;
@@ -1934,13 +1934,15 @@ static bool RecreateLostWaivers(JSContext* cx, const PropertyDescriptor* orig,
     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);

toolkit/components/telemetry/core/Stopwatch.cpp

@@ -25,7 +25,8 @@ 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);
 }