Bug 658909 - Remove GWNOJO from XPCVariant. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 21 Mar 2013 08:20:44 -0700
changeset 125814 d8ac57d4452f8e0e77dbd6deba4b9607200a3c7e
parent 125813 ddd198e0e288a9ed7f0c12cb009f4a3a0e2af09c
child 125815 10c92bbb5f74535e4dc0f42c586ed9c33bb3e6a1
push id25109
push userryanvm@gmail.com
push dateThu, 21 Mar 2013 19:52:05 +0000
treeherdermozilla-inbound@a83cbe4e0576 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs658909
milestone22.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 658909 - Remove GWNOJO from XPCVariant. r=mrbkap The old code seems to be deciding whether we have a double-wrapped object by checking _either_ the rv of GWNOJO _or_ the potential slim wrapper. This is nonsensical, because double-wrapped objects are never slim wrappers. Furthermore, that variable here is named 'proto', which further suggests that this code is nonsensical. So let's just check for WNs. Also, it seems pretty wack to be innerizing here before storing the jsval, but I'm going to leave that for now.
js/xpconnect/src/XPCVariant.cpp
--- a/js/xpconnect/src/XPCVariant.cpp
+++ b/js/xpconnect/src/XPCVariant.cpp
@@ -23,31 +23,30 @@ NS_IMPL_CI_INTERFACE_GETTER2(XPCVariant,
 NS_IMPL_CYCLE_COLLECTING_ADDREF(XPCVariant)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(XPCVariant)
 
 XPCVariant::XPCVariant(JSContext* cx, jsval aJSVal)
     : mJSVal(aJSVal), mCCGeneration(0)
 {
     nsVariant::Initialize(&mData);
     if (!JSVAL_IS_PRIMITIVE(mJSVal)) {
+        // XXXbholley - The innerization here was from bug 638026. Blake says
+        // the basic problem was that we were storing the C++ inner but the JS
+        // outer, which meant that, after navigation, the JS inner could be
+        // collected, which would cause us to try to recreate the JS inner at
+        // some later point after teardown, which would crash. This is shouldn't
+        // be a problem anymore because SetParentToWindow will do the right
+        // thing, but I'm saving the cleanup here for another day. Blake thinks
+        // that we should just not store the WN if we're creating a variant for
+        // an outer window.
         JSObject *obj = JS_ObjectToInnerObject(cx, JSVAL_TO_OBJECT(mJSVal));
-
         mJSVal = OBJECT_TO_JSVAL(obj);
 
-        // If the incoming object is an XPCWrappedNative, then it could be a
-        // double-wrapped object, and we should return the double-wrapped
-        // object back out to script.
-
-        JSObject* proto;
-        XPCWrappedNative* wn =
-            XPCWrappedNative::GetWrappedNativeOfJSObject(cx,
-                                                         JSVAL_TO_OBJECT(mJSVal),
-                                                         nullptr,
-                                                         &proto);
-        mReturnRawObject = !wn && !proto;
+        JSObject *unwrapped = js::UnwrapObjectChecked(obj, /* stopAtOuter = */ false);
+        mReturnRawObject = !(unwrapped && IS_WN_WRAPPER(unwrapped));
     } else
         mReturnRawObject = false;
 }
 
 XPCTraceableVariant::~XPCTraceableVariant()
 {
     jsval val = GetJSValPreserveColor();