Bug 1521907 part 4. Start using CheckedUnwrapStatic/Dynamic in non-binding DOM code. r=peterv
authorBoris Zbarsky <bzbarsky@mit.edu>
Sat, 02 Feb 2019 03:24:22 +0000
changeset 456532 f41215bdded6614b78f6b371a23f842a07c20bfa
parent 456531 64af12d24e9defd3823252921ea0d51cbf2bfef1
child 456533 a0b9977daa361c63bae40a7caf6dbfd2130558fa
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 4. Start using CheckedUnwrapStatic/Dynamic in non-binding DOM code. r=peterv Differential Revision: https://phabricator.services.mozilla.com/D17884
dom/base/ChromeUtils.cpp
dom/base/CustomElementRegistry.cpp
dom/base/StructuredCloneBlob.cpp
dom/base/nsContentPermissionHelper.cpp
dom/base/nsGlobalWindowOuter.cpp
dom/base/nsJSUtils.cpp
dom/media/webaudio/AudioContext.cpp
dom/media/webaudio/AudioWorkletGlobalScope.cpp
dom/plugins/base/nsJSNPRuntime.cpp
dom/promise/PromiseDebugging.cpp
dom/workers/WorkerScope.cpp
--- a/dom/base/ChromeUtils.cpp
+++ b/dom/base/ChromeUtils.cpp
@@ -216,17 +216,20 @@ namespace dom {
 
   auto cleanup = MakeScopeExit([&]() { aRv.NoteJSContextException(cx); });
 
   JS::Rooted<JS::IdVector> ids(cx, JS::IdVector(cx));
   JS::AutoValueVector values(cx);
   JS::AutoIdVector valuesIds(cx);
 
   {
-    JS::RootedObject obj(cx, js::CheckedUnwrap(aObj));
+    // cx represents our current Realm, so it makes sense to use it for the
+    // CheckedUnwrapDynamic call.  We do want CheckedUnwrapDynamic, in case
+    // someone is shallow-cloning a Window.
+    JS::RootedObject obj(cx, js::CheckedUnwrapDynamic(aObj, cx));
     if (!obj) {
       js::ReportAccessDenied(cx);
       return;
     }
 
     if (js::IsScriptedProxy(obj)) {
       JS_ReportErrorASCII(cx, "Shallow cloning a proxy object is not allowed");
       return;
@@ -253,17 +256,20 @@ namespace dom {
       values.infallibleAppend(desc.value());
     }
   }
 
   JS::RootedObject obj(cx);
   {
     Maybe<JSAutoRealm> ar;
     if (aTarget) {
-      JS::RootedObject target(cx, js::CheckedUnwrap(aTarget));
+      // Our target could be anything, so we want CheckedUnwrapDynamic here.
+      // "cx" represents the current Realm when we were called from bindings, so
+      // we can just use that.
+      JS::RootedObject target(cx, js::CheckedUnwrapDynamic(aTarget, cx));
       if (!target) {
         js::ReportAccessDenied(cx);
         return;
       }
       ar.emplace(cx, target);
     }
 
     obj = JS_NewPlainObject(cx);
--- a/dom/base/CustomElementRegistry.cpp
+++ b/dom/base/CustomElementRegistry.cpp
@@ -375,17 +375,19 @@ CustomElementDefinition* CustomElementRe
     return data;
   }
 
   return nullptr;
 }
 
 CustomElementDefinition* CustomElementRegistry::LookupCustomElementDefinition(
     JSContext* aCx, JSObject* aConstructor) const {
-  JS::Rooted<JSObject*> constructor(aCx, js::CheckedUnwrap(aConstructor));
+  // We're looking up things that tested true for JS::IsConstructor,
+  // so doing a CheckedUnwrapStatic is fine here.
+  JS::Rooted<JSObject*> constructor(aCx, js::CheckedUnwrapStatic(aConstructor));
 
   const auto& ptr = mConstructors.lookup(constructor);
   if (!ptr) {
     return nullptr;
   }
 
   CustomElementDefinition* definition =
       mCustomDefinitions.GetWeak(ptr->value());
@@ -660,18 +662,24 @@ int32_t CustomElementRegistry::InferName
 
 // https://html.spec.whatwg.org/multipage/scripting.html#element-definition
 void CustomElementRegistry::Define(JSContext* aCx, const nsAString& aName,
                                    Function& aFunctionConstructor,
                                    const ElementDefinitionOptions& aOptions,
                                    ErrorResult& aRv) {
   JS::Rooted<JSObject*> constructor(aCx, aFunctionConstructor.CallableOrNull());
 
-  JS::Rooted<JSObject*> constructorUnwrapped(aCx,
-                                             js::CheckedUnwrap(constructor));
+  // We need to do a dynamic unwrap in order to throw the right exception.  We
+  // could probably avoid that if we just threw MSG_NOT_CONSTRUCTOR if unwrap
+  // fails.
+  //
+  // In any case, aCx represents the global we want to be using for the unwrap
+  // here.
+  JS::Rooted<JSObject*> constructorUnwrapped(
+      aCx, js::CheckedUnwrapDynamic(constructor, aCx));
   if (!constructorUnwrapped) {
     // If the caller's compartment does not have permission to access the
     // unwrapped constructor then throw.
     aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return;
   }
 
   /**
--- a/dom/base/StructuredCloneBlob.cpp
+++ b/dom/base/StructuredCloneBlob.cpp
@@ -35,31 +35,34 @@ StructuredCloneBlob::Constructor(GlobalO
   JSContext* cx = aGlobal.Context();
 
   RefPtr<StructuredCloneBlob> holder = StructuredCloneBlob::Create();
 
   Maybe<JSAutoRealm> ar;
   JS::RootedValue value(cx, aValue);
 
   if (aTargetGlobal) {
-    JS::RootedObject targetGlobal(cx, js::CheckedUnwrap(aTargetGlobal));
+    // OK to unwrap if our caller (represented by cx's Realm) can do it.
+    JS::RootedObject targetGlobal(cx,
+                                  js::CheckedUnwrapDynamic(aTargetGlobal, cx));
     if (!targetGlobal) {
       js::ReportAccessDenied(cx);
       aRv.NoteJSContextException(cx);
       return nullptr;
     }
 
     ar.emplace(cx, targetGlobal);
 
     if (!JS_WrapValue(cx, &value)) {
       aRv.NoteJSContextException(cx);
       return nullptr;
     }
   } else if (value.isObject()) {
-    JS::RootedObject obj(cx, js::CheckedUnwrap(&value.toObject()));
+    // OK to unwrap if our caller (represented by cx's Realm) can do it.
+    JS::RootedObject obj(cx, js::CheckedUnwrapDynamic(&value.toObject(), cx));
     if (!obj) {
       js::ReportAccessDenied(cx);
       aRv.NoteJSContextException(cx);
       return nullptr;
     }
 
     ar.emplace(cx, obj);
     value = JS::ObjectValue(*obj);
@@ -73,17 +76,18 @@ StructuredCloneBlob::Constructor(GlobalO
   return holder.forget();
 }
 
 void StructuredCloneBlob::Deserialize(JSContext* aCx,
                                       JS::HandleObject aTargetScope,
                                       bool aKeepData,
                                       JS::MutableHandleValue aResult,
                                       ErrorResult& aRv) {
-  JS::RootedObject scope(aCx, js::CheckedUnwrap(aTargetScope));
+  // OK to unwrap if our caller (represented by aCx's Realm) can do it.
+  JS::RootedObject scope(aCx, js::CheckedUnwrapDynamic(aTargetScope, aCx));
   if (!scope) {
     js::ReportAccessDenied(aCx);
     aRv.NoteJSContextException(aCx);
     return;
   }
 
   if (!mHolder.isSome()) {
     aRv.Throw(NS_ERROR_NOT_INITIALIZED);
--- a/dom/base/nsContentPermissionHelper.cpp
+++ b/dom/base/nsContentPermissionHelper.cpp
@@ -665,17 +665,19 @@ nsresult TranslateChoices(
   if (aChoices.isNullOrUndefined()) {
     // No choice is specified.
   } else if (aChoices.isObject()) {
     // Iterate through all permission types.
     for (uint32_t i = 0; i < aPermissionRequests.Length(); ++i) {
       nsCString type = aPermissionRequests[i].type();
 
       JS::Rooted<JSObject*> obj(RootingCx(), &aChoices.toObject());
-      obj = CheckedUnwrap(obj);
+      // People really shouldn't be passing WindowProxy or Location
+      // objects for the choices here.
+      obj = js::CheckedUnwrapStatic(obj);
       if (!obj) {
         return NS_ERROR_FAILURE;
       }
 
       AutoJSAPI jsapi;
       jsapi.Init();
 
       JSContext* cx = jsapi.cx();
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -5775,18 +5775,22 @@ nsGlobalWindowInner* nsGlobalWindowOuter
   // sandboxPrototype. This used to work incidentally for unrelated reasons, but
   // now we need to do some special handling to support it.
   if (xpc::IsSandbox(scope)) {
     JSAutoRealm ar(aCx, scope);
     JS::Rooted<JSObject*> scopeProto(aCx);
     bool ok = JS_GetPrototype(aCx, scope, &scopeProto);
     NS_ENSURE_TRUE(ok, nullptr);
     if (scopeProto && xpc::IsSandboxPrototypeProxy(scopeProto) &&
-        (scopeProto =
-             js::CheckedUnwrap(scopeProto, /* stopAtWindowProxy = */ false))) {
+        // Our current Realm on aCx is the sandbox.  Using that for the
+        // CheckedUnwrapDynamic call makes sense: if the sandbox can unwrap the
+        // window, we can use it.  And we do want CheckedUnwrapDynamic, because
+        // the whole point is to unwrap windows.
+        (scopeProto = js::CheckedUnwrapDynamic(
+             scopeProto, aCx, /* stopAtWindowProxy = */ false))) {
       global = xpc::NativeGlobal(scopeProto);
       NS_ENSURE_TRUE(global, nullptr);
     }
   }
 
   // The calling window must be holding a reference, so we can return a weak
   // pointer.
   nsCOMPtr<nsPIDOMWindowInner> win = do_QueryInterface(global);
--- a/dom/base/nsJSUtils.cpp
+++ b/dom/base/nsJSUtils.cpp
@@ -395,17 +395,18 @@ nsresult nsJSUtils::ExecutionContext::Ex
   return NS_OK;
 }
 
 static bool IsPromiseValue(JSContext* aCx, JS::Handle<JS::Value> aValue) {
   if (!aValue.isObject()) {
     return false;
   }
 
-  JS::Rooted<JSObject*> obj(aCx, js::CheckedUnwrap(&aValue.toObject()));
+  // We only care about Promise here, so CheckedUnwrapStatic is fine.
+  JS::Rooted<JSObject*> obj(aCx, js::CheckedUnwrapStatic(&aValue.toObject()));
   if (!obj) {
     return false;
   }
 
   return JS::IsPromiseObject(obj);
 }
 
 nsresult nsJSUtils::ExecutionContext::ExecScript(
--- a/dom/media/webaudio/AudioContext.cpp
+++ b/dom/media/webaudio/AudioContext.cpp
@@ -540,17 +540,18 @@ already_AddRefed<Promise> AudioContext::
     const Optional<OwningNonNull<DecodeErrorCallback> >& aFailureCallback,
     ErrorResult& aRv) {
   nsCOMPtr<nsIGlobalObject> parentObject = do_QueryInterface(GetParentObject());
   RefPtr<Promise> promise;
   AutoJSAPI jsapi;
   jsapi.Init();
   JSContext* cx = jsapi.cx();
 
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aBuffer.Obj()));
+  // CheckedUnwrapStatic is OK, since we know we have an ArrayBuffer.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aBuffer.Obj()));
   if (!obj) {
     aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return nullptr;
   }
 
   JSAutoRealm ar(cx, obj);
 
   promise = Promise::Create(parentObject, aRv);
--- a/dom/media/webaudio/AudioWorkletGlobalScope.cpp
+++ b/dom/media/webaudio/AudioWorkletGlobalScope.cpp
@@ -69,18 +69,19 @@ void AudioWorkletGlobalScope::RegisterPr
     aRv.ThrowDOMException(
         NS_ERROR_DOM_NOT_SUPPORTED_ERR,
         NS_LITERAL_CSTRING(
             "Argument 1 of AudioWorkletGlobalScope.registerProcessor "
             "is invalid: a class with the same name is already registered."));
     return;
   }
 
+  // We know processorConstructor is callable, so not a WindowProxy or Location.
   JS::Rooted<JSObject*> constructorUnwrapped(
-      aCx, js::CheckedUnwrap(processorConstructor));
+      aCx, js::CheckedUnwrapStatic(processorConstructor));
   if (!constructorUnwrapped) {
     // If the caller's compartment does not have permission to access the
     // unwrapped constructor then throw.
     aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
     return;
   }
 
   /**
--- a/dom/plugins/base/nsJSNPRuntime.cpp
+++ b/dom/plugins/base/nsJSNPRuntime.cpp
@@ -517,17 +517,19 @@ bool JSValToNPVariant(NPP npp, JSContext
   // element has since been adopted into a new document. We don't bother
   // transplanting the plugin objects, and just do a unwrap with security
   // checks if we encounter one of them as an argument. If the unwrap fails,
   // we run with the original wrapped object, since sometimes there are
   // legitimate cases where a security wrapper ends up here (for example,
   // Location objects, which are _always_ behind security wrappers).
   JS::Rooted<JSObject *> obj(cx, &val.toObject());
   JS::Rooted<JSObject *> global(cx);
-  obj = js::CheckedUnwrap(obj);
+  // CheckedUnwrapStatic is fine here; if we get a Location or WindowProxy,
+  // we'll just use the current global instead.
+  obj = js::CheckedUnwrapStatic(obj);
   if (obj) {
     global = JS::GetNonCCWObjectGlobal(obj);
   } else {
     obj = &val.toObject();
     global = JS::CurrentGlobalOrNull(cx);
   }
 
   NPObject *npobj = nsJSObjWrapper::GetNewOrUsed(npp, obj, global);
@@ -1069,17 +1071,20 @@ NPObject *nsJSObjWrapper::GetNewOrUsed(N
 //
 // Because this function unwraps, its return value must be wrapped for the cx
 // compartment for callers that plan to hold onto the result or do anything
 // substantial with it.
 static JSObject *GetNPObjectWrapper(JSContext *cx, JS::Handle<JSObject *> aObj,
                                     bool wrapResult = true) {
   JS::Rooted<JSObject *> obj(cx, aObj);
 
-  while (obj && (obj = js::CheckedUnwrap(obj))) {
+  // We can't have WindowProxy or Location objects with NP object wrapper
+  // objects on their proto chain, since they have immutable prototypes.  So
+  // CheckedUnwrapStatic is ok here.
+  while (obj && (obj = js::CheckedUnwrapStatic(obj))) {
     if (nsNPObjWrapper::IsWrapper(obj)) {
       if (wrapResult && !JS_WrapObject(cx, &obj)) {
         return nullptr;
       }
       return obj;
     }
 
     JSAutoRealm ar(cx, obj);
--- a/dom/promise/PromiseDebugging.cpp
+++ b/dom/promise/PromiseDebugging.cpp
@@ -68,17 +68,18 @@ class FlushRejections : public Cancelabl
 };
 
 /* static */ MOZ_THREAD_LOCAL(bool) FlushRejections::sDispatched;
 
 /* static */ void PromiseDebugging::GetState(
     GlobalObject& aGlobal, JS::Handle<JSObject*> aPromise,
     PromiseDebuggingStateHolder& aState, ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aPromise));
+  // CheckedUnwrapStatic is fine, since we're looking for promises only.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aPromise));
   if (!obj || !JS::IsPromiseObject(obj)) {
     aRv.ThrowTypeError<MSG_IS_NOT_PROMISE>(
         NS_LITERAL_STRING("Argument of PromiseDebugging.getState"));
     return;
   }
   switch (JS::GetPromiseState(obj)) {
     case JS::PromiseState::Pending:
       aState.mState = PromiseDebuggingState::Pending;
@@ -94,58 +95,62 @@ class FlushRejections : public Cancelabl
   }
 }
 
 /* static */ void PromiseDebugging::GetPromiseID(GlobalObject& aGlobal,
                                                  JS::Handle<JSObject*> aPromise,
                                                  nsString& aID,
                                                  ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aPromise));
+  // CheckedUnwrapStatic is fine, since we're looking for promises only.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aPromise));
   if (!obj || !JS::IsPromiseObject(obj)) {
     aRv.ThrowTypeError<MSG_IS_NOT_PROMISE>(
         NS_LITERAL_STRING("Argument of PromiseDebugging.getState"));
     return;
   }
   uint64_t promiseID = JS::GetPromiseID(obj);
   aID = sIDPrefix;
   aID.AppendInt(promiseID);
 }
 
 /* static */ void PromiseDebugging::GetAllocationStack(
     GlobalObject& aGlobal, JS::Handle<JSObject*> aPromise,
     JS::MutableHandle<JSObject*> aStack, ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aPromise));
+  // CheckedUnwrapStatic is fine, since we're looking for promises only.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aPromise));
   if (!obj || !JS::IsPromiseObject(obj)) {
     aRv.ThrowTypeError<MSG_IS_NOT_PROMISE>(
         NS_LITERAL_STRING("Argument of PromiseDebugging.getAllocationStack"));
     return;
   }
   aStack.set(JS::GetPromiseAllocationSite(obj));
 }
 
 /* static */ void PromiseDebugging::GetRejectionStack(
     GlobalObject& aGlobal, JS::Handle<JSObject*> aPromise,
     JS::MutableHandle<JSObject*> aStack, ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aPromise));
+  // CheckedUnwrapStatic is fine, since we're looking for promises only.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aPromise));
   if (!obj || !JS::IsPromiseObject(obj)) {
     aRv.ThrowTypeError<MSG_IS_NOT_PROMISE>(
         NS_LITERAL_STRING("Argument of PromiseDebugging.getRejectionStack"));
     return;
   }
   aStack.set(JS::GetPromiseResolutionSite(obj));
 }
 
 /* static */ void PromiseDebugging::GetFullfillmentStack(
     GlobalObject& aGlobal, JS::Handle<JSObject*> aPromise,
     JS::MutableHandle<JSObject*> aStack, ErrorResult& aRv) {
   JSContext* cx = aGlobal.Context();
-  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrap(aPromise));
+  // CheckedUnwrapStatic is fine, since we're looking for promises only.
+  JS::Rooted<JSObject*> obj(cx, js::CheckedUnwrapStatic(aPromise));
   if (!obj || !JS::IsPromiseObject(obj)) {
     aRv.ThrowTypeError<MSG_IS_NOT_PROMISE>(
         NS_LITERAL_STRING("Argument of PromiseDebugging.getFulfillmentStack"));
     return;
   }
   aStack.set(JS::GetPromiseResolutionSite(obj));
 }
 
--- a/dom/workers/WorkerScope.cpp
+++ b/dom/workers/WorkerScope.cpp
@@ -871,18 +871,21 @@ void WorkerDebuggerGlobalScope::CreateSa
 
 void WorkerDebuggerGlobalScope::LoadSubScript(
     JSContext* aCx, const nsAString& aURL,
     const Optional<JS::Handle<JSObject*>>& aSandbox, ErrorResult& aRv) {
   mWorkerPrivate->AssertIsOnWorkerThread();
 
   Maybe<JSAutoRealm> ar;
   if (aSandbox.WasPassed()) {
-    JS::Rooted<JSObject*> sandbox(aCx, js::CheckedUnwrap(aSandbox.Value()));
-    if (!IsWorkerDebuggerSandbox(sandbox)) {
+    // We only care about worker debugger sandbox objects here, so
+    // CheckedUnwrapStatic is fine.
+    JS::Rooted<JSObject*> sandbox(aCx,
+                                  js::CheckedUnwrapStatic(aSandbox.Value()));
+    if (!sandbox || !IsWorkerDebuggerSandbox(sandbox)) {
       aRv.Throw(NS_ERROR_INVALID_ARG);
       return;
     }
 
     ar.emplace(aCx, sandbox);
   }
 
   nsTArray<nsString> urls;