Bug 752764 - Nuke wrapped reflector during transplant. r=bholley a=lsblakk
authorAndrew McCreight <amccreight@mozilla.com>
Thu, 09 Aug 2012 13:18:47 -0700
changeset 100464 bc0b53d05e68fd338401ab48dbce7edf469b9a86
parent 100463 8c91d2848d80f263e92d1cd0952511f1ec984c1f
child 100465 d5fe000f51bf2b70141d9c546fb08abad5d802a1
push id1252
push useramccreight@mozilla.com
push dateThu, 09 Aug 2012 20:19:28 +0000
treeherdermozilla-beta@757d9c0a8d0a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, lsblakk
bugs752764
milestone15.0
Bug 752764 - Nuke wrapped reflector during transplant. r=bholley a=lsblakk
js/src/jsapi.cpp
js/src/jswrapper.cpp
js/src/jswrapper.h
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1649,22 +1649,21 @@ RemapWrappers(JSContext *cx, JSObject *o
             return false;
         pmap.put(targetv, ObjectValue(*wobj));
     }
 
     return true;
 }
 
 /*
- * The location object is special. There is the location object itself and
- * then the location object wrapper. Because there are no direct references to
- * the location object itself, we don't want the old obj (|origobj| here) to
- * become the new wrapper but the wrapper itself instead. This leads to very
- * subtle differences between js_TransplantObjectWithWrapper and
- * JS_TransplantObject.
+ * Some C++ objects (such as the location object and XBL) require both an XPConnect
+ * reflector and a security wrapper for that reflector. We expect that there are
+ * no live references to the reflector, so when we perform the transplant we turn
+ * the security wrapper into a cross-compartment wrapper. Just in case there
+ * happen to be live references to the reflector, we swap it out to limit the harm.
  */
 JS_FRIEND_API(JSObject *)
 js_TransplantObjectWithWrapper(JSContext *cx,
                                JSObject *origobj,
                                JSObject *origwrapper,
                                JSObject *targetobj,
                                JSObject *targetwrapper)
 {
@@ -1705,22 +1704,35 @@ js_TransplantObjectWithWrapper(JSContext
     // object. Note that the entries in the maps are for |origobj| and not
     // |origwrapper|. They need to be updated to point at the new object.
     if (!RemapWrappers(cx, origobj, targetobj))
         return NULL;
 
     // Lastly, update things in the original compartment. Our invariants dictate
     // that the original compartment can only have one cross-compartment wrapper
     // to the new object. So we choose to update |origwrapper|, not |origobj|,
-    // since theoretically there should have been no direct intra-compartment
-    // references to |origobj|.
+    // since there are probably no live direct intra-compartment references to
+    // |origobj|.
     {
         AutoCompartment ac(cx, origobj);
+        if (!ac.enter())
+            return NULL;
+
+        // We can't be sure that the reflector is completely dead. This is bad,
+        // because it is in a weird state. To minimize potential harm we create
+        // a new unreachable dummy object and swap it with the reflector.
+        // After the swap we have a possibly-live object that isn't dangerous,
+        // and a possibly-dangerous object that isn't live.
+        JSObject *reflectorGuts = NewDeadProxyObject(cx, JS_GetGlobalForObject(cx, origobj));
+        if (!reflectorGuts || !origobj->swap(cx, reflectorGuts))
+            return NULL;
+
+        // Turn origwrapper into a CCW to the new object.
         JSObject *wrapperGuts = targetobj;
-        if (!ac.enter() || !JS_WrapObject(cx, &wrapperGuts))
+        if (!JS_WrapObject(cx, &wrapperGuts))
             return NULL;
         if (!origwrapper->swap(cx, wrapperGuts))
             return NULL;
         origwrapper->compartment()->crossCompartmentWrappers.put(ObjectValue(*targetobj),
                                                                  ObjectValue(*origwrapper));
     }
 
     return newWrapper;
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -963,16 +963,23 @@ DeadObjectProxy::getElementIfPresent(JSC
     return false;
 }
 
 DeadObjectProxy DeadObjectProxy::singleton;
 int DeadObjectProxy::sDeadObjectFamily;
 
 } // namespace js
 
+JSObject *
+js::NewDeadProxyObject(JSContext *cx, JSObject *parent)
+{
+    return NewProxyObject(cx, &DeadObjectProxy::singleton, NullValue(),
+                          NULL, parent, NULL, NULL);
+}
+
 void
 js::NukeCrossCompartmentWrapper(JSObject *wrapper)
 {
     JS_ASSERT(IsCrossCompartmentWrapper(wrapper));
 
     SetProxyPrivate(wrapper, NullValue());
     SetProxyHandler(wrapper, &DeadObjectProxy::singleton);
 
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -273,14 +273,17 @@ UnwrapObject(JSObject *obj, bool stopAtO
 // code should never be unwrapping outer window wrappers, we always stop at
 // outer windows.
 JS_FRIEND_API(JSObject *)
 UnwrapObjectChecked(JSContext *cx, JSObject *obj);
 
 JS_FRIEND_API(bool)
 IsCrossCompartmentWrapper(const JSObject *obj);
 
+JSObject *
+NewDeadProxyObject(JSContext *cx, JSObject *parent);
+
 void
 NukeCrossCompartmentWrapper(JSObject *wrapper);
 
 } /* namespace js */
 
 #endif