Fix bug 505988. r=bzbarsky sr=jst

js/src/xpconnect/src/xpcprivate.h

@@ -4007,7 +4007,7 @@ public:
 
     jsval GetJSVal() const {return mJSVal;}
 
-    XPCVariant(jsval aJSVal);
+    XPCVariant(XPCCallContext& ccx, jsval aJSVal);
 
     /**
      * Convert a variant into a jsval.
@@ -4032,6 +4032,7 @@ protected:
 protected:
     nsDiscriminatedUnion mData;
     jsval                mJSVal;
+    JSBool               mReturnRawObject;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(XPCVariant, XPCVARIANT_IID)
@@ -4040,10 +4041,10 @@ class XPCTraceableVariant: public XPCVariant,
                            public XPCRootSetElem
 {
 public:
-    XPCTraceableVariant(XPCJSRuntime *runtime, jsval aJSVal)
-        : XPCVariant(aJSVal)
+    XPCTraceableVariant(XPCCallContext& ccx, jsval aJSVal)
+        : XPCVariant(ccx, aJSVal)
     {
-        runtime->AddVariantRoot(this);
+        ccx.GetRuntime()->AddVariantRoot(this);
     }
 
     virtual ~XPCTraceableVariant();

js/src/xpconnect/src/xpcvariant.cpp

@@ -55,10 +55,26 @@ NS_IMPL_CI_INTERFACE_GETTER2(XPCVariant, XPCVariant, nsIVariant)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(XPCVariant)
 NS_IMPL_CYCLE_COLLECTING_RELEASE(XPCVariant)
 
-XPCVariant::XPCVariant(jsval aJSVal)
+XPCVariant::XPCVariant(XPCCallContext& ccx, jsval aJSVal)
     : mJSVal(aJSVal)
 {
     nsVariant::Initialize(&mData);
+    if(!JSVAL_IS_PRIMITIVE(mJSVal))
+    {
+        // 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(ccx,
+                                                         JSVAL_TO_OBJECT(mJSVal),
+                                                         nsnull,
+                                                         &proto);
+        mReturnRawObject = !wn && !proto;
+    }
+    else
+        mReturnRawObject = JS_FALSE;
 }
 
 XPCTraceableVariant::~XPCTraceableVariant()
@@ -119,9 +135,9 @@ XPCVariant* XPCVariant::newVariant(XPCCallContext& ccx, jsval aJSVal)
     XPCVariant* variant;
 
     if(!JSVAL_IS_TRACEABLE(aJSVal))
-        variant = new XPCVariant(aJSVal);
+        variant = new XPCVariant(ccx, aJSVal);
     else
-        variant = new XPCTraceableVariant(ccx.GetRuntime(), aJSVal);
+        variant = new XPCTraceableVariant(ccx, aJSVal);
 
     if(!variant)
         return nsnull;
@@ -401,6 +417,15 @@ XPCVariant::VariantDataToJS(XPCCallContext& ccx,
             return JS_TRUE;
         }
 
+        if(xpcvariant->mReturnRawObject)
+        {
+            NS_ASSERTION(type == nsIDataType::VTYPE_INTERFACE ||
+                         type == nsIDataType::VTYPE_INTERFACE_IS,
+                         "Weird variant");
+            *pJSVal = realVal;
+            return JS_TRUE;
+        }
+
         // else, it's an object and we really need to double wrap it if we've 
         // already decided that its 'natural' type is as some sort of interface.
         
@@ -641,40 +666,10 @@ VARIANT_DONE:
     }
     else
     {
-        // Last ditch check to prevent us from double-wrapping a regular JS
-        // object. This allows us to unwrap regular JS objects (since we
-        // normally can't double wrap them). See bug 384632.
-        *pJSVal = JSVAL_VOID;
-        if(type == nsIDataType::VTYPE_INTERFACE ||
-           type == nsIDataType::VTYPE_INTERFACE_IS)
-        {
-            nsISupports *src = reinterpret_cast<nsISupports *>(xpctvar.val.p);
-            if(nsXPCWrappedJSClass::IsWrappedJS(src))
-            {
-                // First QI the wrapper to the right interface.
-                nsCOMPtr<nsISupports> wrapper;
-                nsresult rv = src->QueryInterface(iid, getter_AddRefs(wrapper));
-                NS_ENSURE_SUCCESS(rv, JS_FALSE);
-
-                // Now, get the actual JS object out of the wrapper.
-                nsCOMPtr<nsIXPConnectJSObjectHolder> holder =
-                    do_QueryInterface(wrapper);
-                NS_ENSURE_TRUE(holder, JS_FALSE);
-
-                JSObject *obj;
-                holder->GetJSObject(&obj);
-                NS_ASSERTION(obj, "No JS object but the QIs above succeeded?");
-                *pJSVal = OBJECT_TO_JSVAL(obj);
-                success = JS_TRUE;
-            }
-        }
-        if(!JSVAL_IS_OBJECT(*pJSVal))
-        {
-            success = XPCConvert::NativeData2JS(ccx, pJSVal,
-                                                (const void*)&xpctvar.val,
-                                                xpctvar.type,
-                                                &iid, scope, pErr);
-        }
+        success = XPCConvert::NativeData2JS(ccx, pJSVal,
+                                            (const void*)&xpctvar.val,
+                                            xpctvar.type,
+                                            &iid, scope, pErr);
     }
 
     if(xpctvar.IsValAllocated())

js/src/xpconnect/tests/mochitest/Makefile.in

@@ -47,6 +47,7 @@ include $(topsrcdir)/config/rules.mk
 _TEST_FILES =	bug500931_helper.html \
 		inner.html \
 		test_bug361111.xul \
+		test_bug384632.html \
 		test_bug390488.html \
 		test_bug393269.html \
 		test_bug396851.html \

js/src/xpconnect/tests/mochitest/test_bug384632.html

@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=384632
+-->
+<head>
+  <title>Test for Bug 384632</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=384632">Mozilla Bug 384632</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 384632 **/
+var div = $('content');
+var obj = {};
+div.setUserData("foopy", obj, function() {});
+ok(div.getUserData("foopy") === obj, "getUserData works with regular objects");
+div.setUserData("foopy1", sidebar, function() {});
+ok(div.getUserData("foopy1") === sidebar, "getUserData works with bizarre objects");
+
+</script>
+</pre>
+</body>
+</html>