Bug 1534902 - Move more of XPConnect's PreWrap code into the JS engine. r=kmag
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 29 Mar 2019 09:06:31 +0000
changeset 466729 116b9cd070f29167d65c90ceb1ac4e7ac60f9db9
parent 466728 b81febb43f21add6b7bd6ae8b3127db075af282d
child 466730 cec67adbdbb6d5b2948a68e4f6fb13612f949368
push id35780
push useropoprus@mozilla.com
push dateFri, 29 Mar 2019 21:53:01 +0000
treeherdermozilla-central@414f37afbe07 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1534902
milestone68.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 1534902 - Move more of XPConnect's PreWrap code into the JS engine. r=kmag This ensures the JS shell and browser behave the same way and it's nice for fuzzing. Differential Revision: https://phabricator.services.mozilla.com/D25204
js/src/jit-test/tests/basic/wrapping-dead-wrapper.js
js/src/vm/Compartment.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/wrapping-dead-wrapper.js
@@ -0,0 +1,9 @@
+// Make sure the wrapper machinery returns a dead wrapper instead of a CCW when
+// attempting to wrap a dead wrapper.
+var g1 = newGlobal({newCompartment: true});
+var g2 = newGlobal({newCompartment: true});
+var wrapper = g1.Math;
+nukeCCW(wrapper);
+assertEq(isSameCompartment(wrapper, this), true);
+g2.wrapper = wrapper;
+g2.eval("assertEq(isSameCompartment(wrapper, this), true)");
--- a/js/src/vm/Compartment.cpp
+++ b/js/src/vm/Compartment.cpp
@@ -212,23 +212,47 @@ bool Compartment::getNonWrapperObjectFor
   if (obj->compartment() == this) {
     MOZ_ASSERT(!IsWindow(obj));
     return true;
   }
 
   // Disallow creating new wrappers if we nuked the object's realm or the
   // current compartment.
   if (!AllowNewWrapper(this, obj)) {
-    JSObject* res = NewDeadProxyObject(cx, IsCallableFlag(obj->isCallable()),
-                                       IsConstructorFlag(obj->isConstructor()));
-    if (!res) {
-      return false;
+    obj.set(NewDeadProxyObject(cx, IsCallableFlag(obj->isCallable()),
+                               IsConstructorFlag(obj->isConstructor())));
+    return !!obj;
+  }
+
+  // Use the WindowProxy instead of the Window here, so that we don't have to
+  // deal with this in the rest of the wrapping code.
+  if (IsWindow(obj)) {
+    obj.set(ToWindowProxyIfWindow(obj));
+
+    // ToWindowProxyIfWindow can return a CCW if |obj| was a navigated-away-from
+    // Window. Strip any CCWs.
+    obj.set(UncheckedUnwrap(obj));
+
+    if (JS_IsDeadWrapper(obj)) {
+      obj.set(NewDeadProxyObject(cx, obj));
+      return !!obj;
     }
-    obj.set(res);
-    return true;
+
+    MOZ_ASSERT(IsWindowProxy(obj));
+
+    // We crossed a compartment boundary there, so may now have a gray object.
+    // This function is not allowed to return gray objects, so don't do that.
+    ExposeObjectToActiveJS(obj);
+  }
+
+  // If the object is a dead wrapper, return a new dead wrapper rather than
+  // trying to wrap it for a different compartment.
+  if (JS_IsDeadWrapper(obj)) {
+    obj.set(NewDeadProxyObject(cx, obj));
+    return !!obj;
   }
 
   // Invoke the prewrap callback. The prewrap callback is responsible for
   // doing similar reification as above, but can account for any additional
   // embedder requirements.
   //
   // We're a bit worried about infinite recursion here, so we do a check -
   // see bug 809295.
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -151,44 +151,23 @@ inline bool ShouldWaiveXray(JSContext* c
   }
   return sameOrigin;
 }
 
 void WrapperFactory::PrepareForWrapping(JSContext* cx, HandleObject scope,
                                         HandleObject objArg,
                                         HandleObject objectPassedToWrap,
                                         MutableHandleObject retObj) {
+  // The JS engine calls ToWindowProxyIfWindow and deals with dead wrappers.
+  MOZ_ASSERT(!js::IsWindow(objArg));
+  MOZ_ASSERT(!JS_IsDeadWrapper(objArg));
+
   bool waive = ShouldWaiveXray(cx, objectPassedToWrap);
   RootedObject obj(cx, objArg);
   retObj.set(nullptr);
-  // Outerize any raw inner objects at the entry point here, so that we don't
-  // have to worry about them for the rest of the wrapping code.
-  if (js::IsWindow(obj)) {
-    obj = js::ToWindowProxyIfWindow(obj);
-    MOZ_ASSERT(obj);
-    // ToWindowProxyIfWindow can return a CCW if |obj| was a
-    // navigated-away-from Window. Strip any CCWs.
-    obj = js::UncheckedUnwrap(obj);
-    if (JS_IsDeadWrapper(obj)) {
-      retObj.set(JS_NewDeadWrapper(cx, obj));
-      return;
-    }
-    MOZ_ASSERT(js::IsWindowProxy(obj));
-    // We crossed a compartment boundary there, so may now have a gray
-    // object.  This function is not allowed to return gray objects, so
-    // don't do that.
-    ExposeObjectToActiveJS(obj);
-  }
-
-  // If the object is a dead wrapper, return a new dead wrapper rather than
-  // trying to wrap it for a different compartment.
-  if (JS_IsDeadWrapper(obj)) {
-    retObj.set(JS_NewDeadWrapper(cx, obj));
-    return;
-  }
 
   // If we've got a WindowProxy, there's nothing special that needs to be
   // done here, and we can move on to the next phase of wrapping. We handle
   // this case first to allow us to assert against wrappers below.
   if (js::IsWindowProxy(obj)) {
     retObj.set(waive ? WaiveXray(cx, obj) : obj);
     return;
   }