Bug 958326 - Remove same-compartment security wrapper machinery. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Wed, 26 Mar 2014 10:59:20 -0300
changeset 175521 15ceb48e2bf68611fe89d6efc3c4f8bfd5db6b6e
parent 175520 edc54a1c9a9be720a1061b400d7758493d4e30ab
child 175522 08c58214f01f409d1136d85cf37a778fc5a89f28
push id5951
push userryanvm@gmail.com
push dateThu, 27 Mar 2014 03:07:34 +0000
treeherderfx-team@47dcbe2b1fd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs958326
milestone31.0a1
Bug 958326 - Remove same-compartment security wrapper machinery. r=mrbkap
dom/bindings/BindingUtils.h
js/src/jsapi-tests/testBug604087.cpp
js/src/jsapi.h
js/src/jscompartment.cpp
js/src/vm/Runtime.cpp
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
js/xpconnect/wrappers/WrapperFactory.h
--- a/dom/bindings/BindingUtils.h
+++ b/dom/bindings/BindingUtils.h
@@ -507,22 +507,16 @@ CouldBeDOMBinding(void*)
 }
 
 MOZ_ALWAYS_INLINE bool
 CouldBeDOMBinding(nsWrapperCache* aCache)
 {
   return aCache->IsDOMBinding();
 }
 
-inline bool
-GetSameCompartmentWrapperForDOMBinding(JSObject*& obj)
-{
-  return IsDOMObject(obj);
-}
-
 // Make sure to wrap the given string value into the right compartment, as
 // needed.
 MOZ_ALWAYS_INLINE
 bool
 MaybeWrapStringValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
 {
   MOZ_ASSERT(rval.isString());
   JSString* str = rval.toString();
@@ -535,25 +529,24 @@ MaybeWrapStringValue(JSContext* cx, JS::
 // Make sure to wrap the given object value into the right compartment as
 // needed.  This will work correctly, but possibly slowly, on all objects.
 MOZ_ALWAYS_INLINE
 bool
 MaybeWrapObjectValue(JSContext* cx, JS::MutableHandle<JS::Value> rval)
 {
   MOZ_ASSERT(rval.isObject());
 
+  // Cross-compartment always requires wrapping.
   JSObject* obj = &rval.toObject();
   if (js::GetObjectCompartment(obj) != js::GetContextCompartment(cx)) {
     return JS_WrapValue(cx, rval);
   }
 
-  // We're same-compartment, but even then we might need to wrap
-  // objects specially.  Check for that.
-  if (GetSameCompartmentWrapperForDOMBinding(obj)) {
-    // We're a new-binding object, and "obj" now points to the right thing
+  // We're same-compartment. If we're a WebIDL object, we're done.
+  if (IsDOMObject(obj)) {
     rval.set(JS::ObjectValue(*obj));
     return true;
   }
 
   // It's not a WebIDL object.  But it might be an XPConnect one, in which case
   // we may need to outerize here, so make sure to call JS_WrapValue.
   return JS_WrapValue(cx, rval);
 }
--- a/js/src/jsapi-tests/testBug604087.cpp
+++ b/js/src/jsapi-tests/testBug604087.cpp
@@ -34,39 +34,31 @@ wrap(JSContext *cx, JS::HandleObject toW
     JSAutoCompartment ac(cx, target);
     JS::RootedObject wrapper(cx, toWrap);
     if (!JS_WrapObject(cx, &wrapper))
         return nullptr;
     return wrapper;
 }
 
 static JSObject *
-SameCompartmentWrap(JSContext *cx, JS::HandleObject obj)
-{
-    JS_GC(JS_GetRuntime(cx));
-    return obj;
-}
-
-static JSObject *
 PreWrap(JSContext *cx, JS::HandleObject scope, JS::HandleObject obj, unsigned flags)
 {
     JS_GC(JS_GetRuntime(cx));
     return obj;
 }
 
 static JSObject *
 Wrap(JSContext *cx, JS::HandleObject existing, JS::HandleObject obj,
      JS::HandleObject proto, JS::HandleObject parent, unsigned flags)
 {
     return js::Wrapper::New(cx, obj, parent, &js::CrossCompartmentWrapper::singleton);
 }
 
 static const JSWrapObjectCallbacks WrapObjectCallbacks = {
     Wrap,
-    SameCompartmentWrap,
     PreWrap
 };
 
 BEGIN_TEST(testBug604087)
 {
     js::WrapperOptions options;
     options.setClass(&OuterWrapperClass);
     options.setSingleton(true);
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -797,33 +797,19 @@ typedef JSObject *
  * Callback used by the wrap hook to ask the embedding to prepare an object
  * for wrapping in a context. This might include unwrapping other wrappers
  * or even finding a more suitable object for the new compartment.
  */
 typedef JSObject *
 (* JSPreWrapCallback)(JSContext *cx, JS::HandleObject scope, JS::HandleObject obj,
                       unsigned flags);
 
-/*
- * Callback used when wrapping determines that the underlying object is already
- * in the compartment for which it is being wrapped. This allows consumers to
- * maintain same-compartment wrapping invariants.
- *
- * |obj| is guaranteed to be same-compartment as |cx|, but it may (or may not)
- * be a security or cross-compartment wrapper. This is an unfortunate contract,
- * but is important for to avoid unnecessarily recomputing every cross-
- * compartment wrapper that gets passed to wrap.
- */
-typedef JSObject *
-(* JSSameCompartmentWrapObjectCallback)(JSContext *cx, JS::HandleObject obj);
-
 struct JSWrapObjectCallbacks
 {
     JSWrapObjectCallback wrap;
-    JSSameCompartmentWrapObjectCallback sameCompartmentWrap;
     JSPreWrapCallback preWrap;
 };
 
 typedef void
 (* JSDestroyCompartmentCallback)(JSFreeOp *fop, JSCompartment *compartment);
 
 typedef void
 (* JSZoneCallback)(JS::Zone *zone);
--- a/js/src/jscompartment.cpp
+++ b/js/src/jscompartment.cpp
@@ -177,30 +177,16 @@ JSCompartment::ensureJitCompartmentExist
         jitCompartment_ = nullptr;
         return false;
     }
 
     return true;
 }
 #endif
 
-static bool
-WrapForSameCompartment(JSContext *cx, MutableHandleObject obj, const JSWrapObjectCallbacks *cb)
-{
-    JS_ASSERT(cx->compartment() == obj->compartment());
-    if (!cb->sameCompartmentWrap)
-        return true;
-
-    RootedObject wrapped(cx, cb->sameCompartmentWrap(cx, obj));
-    if (!wrapped)
-        return false;
-    obj.set(wrapped);
-    return true;
-}
-
 #ifdef JSGC_GENERATIONAL
 
 /*
  * This class is used to add a post barrier on the crossCompartmentWrappers map,
  * as the key is calculated based on objects which may be moved by generational
  * GC.
  */
 class WrapperMapRef : public BufferableRef
@@ -358,32 +344,36 @@ JSCompartment::wrap(JSContext *cx, Mutab
     // This loses us some transparency, and is generally very cheesy.
     HandleObject global = cx->global();
     RootedObject objGlobal(cx, &obj->global());
     JS_ASSERT(global);
     JS_ASSERT(objGlobal);
 
     const JSWrapObjectCallbacks *cb = cx->runtime()->wrapObjectCallbacks;
 
-    if (obj->compartment() == this)
-        return WrapForSameCompartment(cx, obj, cb);
+    if (obj->compartment() == this) {
+        obj.set(GetOuterObject(cx, obj));
+        return true;
+    }
 
     // If we have a cross-compartment wrapper, make sure that the cx isn't
     // associated with the self-hosting global. We don't want to create
     // wrappers for objects in other runtimes, which may be the case for the
     // self-hosting global.
     JS_ASSERT(!cx->runtime()->isSelfHostingGlobal(global) &&
               !cx->runtime()->isSelfHostingGlobal(objGlobal));
 
     // Unwrap the object, but don't unwrap outer windows.
     unsigned flags = 0;
     obj.set(UncheckedUnwrap(obj, /* stopAtOuter = */ true, &flags));
 
-    if (obj->compartment() == this)
-        return WrapForSameCompartment(cx, obj, cb);
+    if (obj->compartment() == this) {
+        MOZ_ASSERT(obj == GetOuterObject(cx, obj));
+        return true;
+    }
 
     // Translate StopIteration singleton.
     if (obj->is<StopIterationObject>()) {
         // StopIteration isn't a constructor, but it's stored in GlobalObject
         // as one, out of laziness. Hence the GetBuiltinConstructor call here.
         RootedObject stopIteration(cx);
         if (!GetBuiltinConstructor(cx, JSProto_StopIteration, &stopIteration))
             return false;
@@ -394,26 +384,21 @@ JSCompartment::wrap(JSContext *cx, Mutab
     // Invoke the prewrap callback. We're a bit worried about infinite
     // recursion here, so we do a check - see bug 809295.
     JS_CHECK_CHROME_RECURSION(cx, return false);
     if (cb->preWrap) {
         obj.set(cb->preWrap(cx, global, obj, flags));
         if (!obj)
             return false;
     }
+    MOZ_ASSERT(obj == GetOuterObject(cx, obj));
 
     if (obj->compartment() == this)
-        return WrapForSameCompartment(cx, obj, cb);
+        return true;
 
-#ifdef DEBUG
-    {
-        JSObject *outer = GetOuterObject(cx, obj);
-        JS_ASSERT(outer && outer == obj);
-    }
-#endif
 
     // If we already have a wrapper for this value, use it.
     RootedValue key(cx, ObjectValue(*obj));
     if (WrapperMap::Ptr p = crossCompartmentWrappers.lookup(key)) {
         obj.set(&p->value().get().toObject());
         JS_ASSERT(obj->is<CrossCompartmentWrapperObject>());
         JS_ASSERT(obj->getParent() == global);
         return true;
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -100,17 +100,16 @@ PerThreadData::init()
     if (!dtoaState)
         return false;
 
     return true;
 }
 
 static const JSWrapObjectCallbacks DefaultWrapObjectCallbacks = {
     TransparentObjectWrapper,
-    nullptr,
     nullptr
 };
 
 JSRuntime::JSRuntime(JSRuntime *parentRuntime, JSUseHelperThreads useHelperThreads)
   : JS::shadow::Runtime(
 #ifdef JSGC_GENERATIONAL
         &gcStoreBuffer
 #endif
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -2988,17 +2988,16 @@ class XPCJSSourceHook: public js::Source
         }
 
         return true;
     }
 };
 
 static const JSWrapObjectCallbacks WrapObjectCallbacks = {
     xpc::WrapperFactory::Rewrap,
-    xpc::WrapperFactory::WrapForSameCompartment,
     xpc::WrapperFactory::PrepareForWrapping
 };
 
 XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect)
    : CycleCollectedJSRuntime(nullptr, 32L * 1024L * 1024L, JS_USE_HELPER_THREADS),
    mJSContextStack(new XPCJSContextStack(MOZ_THIS_IN_INITIALIZER_LIST())),
    mCallContext(nullptr),
    mAutoRoots(nullptr),
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -484,36 +484,16 @@ WrapperFactory::Rewrap(JSContext *cx, Ha
     DEBUG_CheckUnwrapSafety(obj, wrapper, origin, target);
 
     if (existing)
         return Wrapper::Renew(cx, existing, obj, wrapper);
 
     return Wrapper::New(cx, obj, parent, wrapper);
 }
 
-JSObject *
-WrapperFactory::WrapForSameCompartment(JSContext *cx, HandleObject objArg)
-{
-    RootedObject obj(cx, objArg);
-    MOZ_ASSERT(js::IsObjectInContextCompartment(obj, cx));
-
-    // NB: The contract of WrapForSameCompartment says that |obj| may or may not
-    // be a security wrapper. These checks implicitly handle the security
-    // wrapper case.
-
-    // Outerize if necessary. This, in combination with the check in
-    // PrepareForUnwrapping, means that calling JS_Wrap* always outerizes.
-    obj = JS_ObjectToOuterObject(cx, obj);
-    NS_ENSURE_TRUE(obj, nullptr);
-
-    // The method below is a no-op for non-DOM objects.
-    dom::GetSameCompartmentWrapperForDOMBinding(*obj.address());
-    return obj;
-}
-
 // Call WaiveXrayAndWrap when you have a JS object that you don't want to be
 // wrapped in an Xray wrapper. cx->compartment is the compartment that will be
 // using the returned object. If the object to be wrapped is already in the
 // correct compartment, then this returns the unwrapped object.
 bool
 WrapperFactory::WaiveXrayAndWrap(JSContext *cx, MutableHandleValue vp)
 {
     if (vp.isPrimitive())
--- a/js/xpconnect/wrappers/WrapperFactory.h
+++ b/js/xpconnect/wrappers/WrapperFactory.h
@@ -53,20 +53,16 @@ class WrapperFactory {
     // Rewrap an object that is about to cross compartment boundaries.
     static JSObject *Rewrap(JSContext *cx,
                             JS::HandleObject existing,
                             JS::HandleObject obj,
                             JS::HandleObject wrappedProto,
                             JS::HandleObject parent,
                             unsigned flags);
 
-    // Wrap an object for same-compartment access.
-    static JSObject *WrapForSameCompartment(JSContext *cx,
-                                            JS::HandleObject obj);
-
     // Wrap wrapped object into a waiver wrapper and then re-wrap it.
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleValue vp);
     static bool WaiveXrayAndWrap(JSContext *cx, JS::MutableHandleObject object);
 
     // Returns true if the wrapper is in not shadowing mode for the id.
     static bool XrayWrapperNotShadowing(JSObject *wrapper, jsid id);
 };