Fix for bug 412491 (function objects cloned by XPConnect still keep hidden window alive late into shutdown). r=igor, sr=jst.

js/src/xpconnect/src/XPCWrapper.cpp

@@ -448,26 +448,20 @@ XPCWrapper::ResolveNativeProperty(JSContext *cx, JSObject *wrapperObj,
     return MaybePreserveWrapper(cx, wn, flags);
   }
 
-  // Get (and perhaps lazily create) the member's value (commonly a
-  // cloneable function).
-  jsval memberval;
-  if (!member->GetValue(ccx, iface, &memberval)) {
-    return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
-  }
-
-  // Make sure memberval doesn't go away while we mess with it.
-  AUTO_MARK_JSVAL(ccx, memberval);
-
   JSString *str = JSVAL_TO_STRING(id);
   if (!str) {
     return ThrowException(NS_ERROR_UNEXPECTED, cx);
   }
 
+  // Get (and perhaps lazily create) the member's value (commonly a
+  // cloneable function).
   jsval v;
   uintN attrs = JSPROP_ENUMERATE;
 
   if (member->IsConstant()) {
-    v = memberval;
+    if (!member->GetConstantValue(ccx, iface, &v)) {
+      return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
+    }
   } else if (member->IsAttribute()) {
     // An attribute is being resolved. Define the property, the value
     // will be dealt with in the get/set hooks.  Use JSPROP_SHARED to
@@ -481,24 +475,28 @@ XPCWrapper::ResolveNativeProperty(JSContext *cx, JSObject *wrapperObj,
     // use for this object.  NB: cx's newborn roots will protect funobj
     // and funWrapper and its object from GC.
 
-    JSObject* funobj = xpc_CloneJSFunction(ccx, JSVAL_TO_OBJECT(memberval),
+    jsval funval;
-                                           wrapper->GetFlatJSObject());
+    if (!member->NewFunctionObject(ccx, iface, wrapper->GetFlatJSObject(),
-    if (!funobj) {
+                                   &funval)) {
-      return JS_FALSE;
+      return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
     }
 
-    AUTO_MARK_JSVAL(ccx, OBJECT_TO_JSVAL(funobj));
+    AUTO_MARK_JSVAL(ccx, funval);
 
 #ifdef DEBUG_XPCNativeWrapper
     printf("Wrapping function object for %s\n",
            ::JS_GetStringBytes(JSVAL_TO_STRING(id)));
 #endif
 
-    if (!WrapFunction(cx, wrapperObj, funobj, &v, isNativeWrapper)) {
+    if (!WrapFunction(cx, wrapperObj, JSVAL_TO_OBJECT(funval), &v,
+                      isNativeWrapper)) {
       return JS_FALSE;
     }
   }
 
+  // Make sure v doesn't go away while we mess with it.
+  AUTO_MARK_JSVAL(ccx, v);
+
   // XPCNativeWrapper doesn't need to do this.
   jsval oldFlags;
   if (!isNativeWrapper &&
@@ -604,14 +602,12 @@ XPCWrapper::GetOrSetNativeProperty(JSContext *cx, JSObject *obj,
     return JS_TRUE;
   }
 
-  // Get (and perhaps lazily create) the member's value (commonly a
-  // cloneable function).
-  jsval memberval;
-  if (!member->GetValue(ccx, iface, &memberval)) {
-    return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
-  }
-
   if (member->IsConstant()) {
+    jsval memberval;
+    if (!member->GetConstantValue(ccx, iface, &memberval)) {
+      return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
+    }
+
     // Getting the value of constants is easy, just return the
     // value. Setting is not supported (obviously).
     if (aIsSet) {
@@ -630,17 +626,14 @@ XPCWrapper::GetOrSetNativeProperty(JSContext *cx, JSObject *obj,
     return JS_TRUE;
   }
 
-  // Make sure the function we're cloning doesn't go away while
+  jsval funval;
-  // we're cloning it.
+  if (!member->NewFunctionObject(ccx, iface, wrapper->GetFlatJSObject(),
-  AUTO_MARK_JSVAL(ccx, memberval);
+                                 &funval)) {
-
+    return ThrowException(NS_ERROR_XPC_BAD_CONVERT_JS, cx);
-  // clone a function we can use for this object
-  JSObject* funobj = xpc_CloneJSFunction(ccx, JSVAL_TO_OBJECT(memberval),
-                                         wrapper->GetFlatJSObject());
-  if (!funobj) {
-    return JS_FALSE;
   }
 
+  AUTO_MARK_JSVAL(ccx, funval);
+
   jsval *argv = nsnull;
   uintN argc = 0;
 
@@ -666,8 +659,8 @@ XPCWrapper::GetOrSetNativeProperty(JSContext *cx, JSObject *obj,
 
   // Call the getter
   jsval v;
-  if (!::JS_CallFunctionValue(cx, wrapper->GetFlatJSObject(),
+  if (!::JS_CallFunctionValue(cx, wrapper->GetFlatJSObject(), funval, argc,
-                              OBJECT_TO_JSVAL(funobj), argc, argv, &v)) {
+                              argv, &v)) {
     return JS_FALSE;
   }
 
@@ -711,34 +704,22 @@ XPCWrapper::NativeToString(JSContext *cx, XPCWrappedNative *wrappedNative,
 
   XPCNativeInterface *iface = ccx.GetInterface();
   XPCNativeMember *member = ccx.GetMember();
-  JSBool overridden = JS_FALSE;
+  JSString* str = nsnull;
-  jsval toStringVal;
 
   // First, try to see if the object declares a toString in its IDL. If it does,
   // then we need to defer to that.
-  if (iface && member) {
+  if (iface && member && member->IsMethod()) {
-    if (!member->GetValue(ccx, iface, &toStringVal)) {
+    jsval toStringVal;
+    if (!member->NewFunctionObject(ccx, iface, wn_obj, &toStringVal)) {
       return JS_FALSE;
     }
 
-    overridden = member->IsMethod();
-  }
-
-  JSString* str = nsnull;
-  if (overridden) {
     // Defer to the IDL-declared toString.
 
     AUTO_MARK_JSVAL(ccx, toStringVal);
 
-    JSObject *funobj = xpc_CloneJSFunction(ccx, JSVAL_TO_OBJECT(toStringVal),
-                                           wn_obj);
-    if (!funobj) {
-      return JS_FALSE;
-    }
-
     jsval v;
-    if (!::JS_CallFunctionValue(cx, wn_obj, OBJECT_TO_JSVAL(funobj), argc, argv,
+    if (!::JS_CallFunctionValue(cx, wn_obj, toStringVal, argc, argv, &v)) {
-                                &v)) {
       return JS_FALSE;
     }
 

js/src/xpconnect/src/xpccomponents.cpp

@@ -2825,23 +2825,14 @@ nsXPCComponents_Utils::LookupMethod()
     if(!iface)
         return NS_ERROR_XPC_BAD_CONVERT_JS;
 
-    // get (and perhaps lazily create) the member's cloneable function
+    // get (and perhaps lazily create) the member's cloned function
     jsval funval;
-    if(!member->GetValue(inner_cc, iface, &funval))
+    if(!member->NewFunctionObject(inner_cc, iface, wrapper->GetFlatJSObject(),
-        return NS_ERROR_XPC_BAD_CONVERT_JS;
+                                  &funval))
-
-    // Make sure the function we're cloning doesn't go away while
-    // we're cloning it.
-    AUTO_MARK_JSVAL(inner_cc, funval);
-
-    // clone a function we can use for this object
-    JSObject* funobj = xpc_CloneJSFunction(inner_cc, JSVAL_TO_OBJECT(funval),
-                                           wrapper->GetFlatJSObject());
-    if(!funobj)
         return NS_ERROR_XPC_BAD_CONVERT_JS;
 
     // return the function and let xpconnect know we did so
-    *retval = OBJECT_TO_JSVAL(funobj);
+    *retval = funval;
     cc->SetReturnValueWasSet(PR_TRUE);
 
     return NS_OK;

js/src/xpconnect/src/xpcjsid.cpp

@@ -508,7 +508,7 @@ nsJSIID::NewResolve(nsIXPConnectWrappedNative *wrapper,
     if(member && member->IsConstant())
     {
         jsval val;
-        if(!member->GetValue(ccx, iface, &val))
+        if(!member->GetConstantValue(ccx, iface, &val))
             return NS_ERROR_OUT_OF_MEMORY;
 
         jsid idid;

js/src/xpconnect/src/xpcprivate.h

@@ -1343,6 +1343,9 @@ private:
 #endif
 };
 
+JSObject* xpc_CloneJSFunction(XPCCallContext &ccx, JSObject *funobj,
+                              JSObject *parent);
+
 /***************************************************************************/
 // XPCNativeMember represents a single idl declared method, attribute or
 // constant.
@@ -1361,10 +1364,23 @@ public:
 
     PRUint16 GetIndex() const {return mIndex;}
 
-    JSBool GetValue(XPCCallContext& ccx, XPCNativeInterface* iface, jsval* pval)
+    JSBool GetConstantValue(XPCCallContext& ccx, XPCNativeInterface* iface,
-        {if(!IsResolved() && !Resolve(ccx, iface)) return JS_FALSE;
+                            jsval* pval)
+        {NS_ASSERTION(IsConstant(),
+                      "Only call this if you're sure this is a constant!");
+         if(!IsResolved() && !Resolve(ccx, iface)) return JS_FALSE;
          *pval = mVal; return JS_TRUE;}
 
+    JSBool NewFunctionObject(XPCCallContext& ccx, XPCNativeInterface* iface,
+                             JSObject *parent, jsval* pval)
+        {NS_ASSERTION(!IsConstant(),
+                      "Only call this if you're sure this is not a constant!");
+         if(!IsResolved() && !Resolve(ccx, iface)) return JS_FALSE;
+         JSObject* funobj =
+            xpc_CloneJSFunction(ccx, JSVAL_TO_OBJECT(mVal), parent);
+         if(!funobj) return JS_FALSE;
+         *pval = OBJECT_TO_JSVAL(funobj); return JS_TRUE;}
+
     JSBool IsMethod() const
         {return 0 != (mFlags & METHOD);}
 
@@ -3852,9 +3868,6 @@ NS_DEFINE_STATIC_IID_ACCESSOR(PrincipalHolder, PRINCIPALHOLDER_IID)
 
 JSBool xpc_IsReportableErrorCode(nsresult code);
 
-JSObject* xpc_CloneJSFunction(XPCCallContext &ccx, JSObject *funobj,
-                              JSObject *parent);
-
 #ifndef XPCONNECT_STANDALONE
 
 // Helper for creating a sandbox object to use for evaluating

js/src/xpconnect/src/xpcwrappednativeinfo.cpp

@@ -193,6 +193,9 @@ XPCNativeMember::Resolve(XPCCallContext& ccx, XPCNativeInterface* iface)
 
     AUTO_MARK_JSVAL(ccx, OBJECT_TO_JSVAL(funobj));
 
+    STOBJ_SET_PARENT(funobj, nsnull);
+    STOBJ_SET_PROTO(funobj, nsnull);
+
     if(!JS_SetReservedSlot(ccx, funobj, 0, PRIVATE_TO_JSVAL(iface))||
        !JS_SetReservedSlot(ccx, funobj, 1, PRIVATE_TO_JSVAL(this)))
         return JS_FALSE;

js/src/xpconnect/src/xpcwrappednativejsops.cpp

@@ -427,7 +427,7 @@ DefinePropertyIfFound(XPCCallContext& ccx,
         AutoResolveName arn(ccx, idval);
         if(resolved)
             *resolved = JS_TRUE;
-        return member->GetValue(ccx, iface, &val) &&
+        return member->GetConstantValue(ccx, iface, &val) &&
                JS_ValueToId(ccx, idval, &id) &&
                OBJ_DEFINE_PROPERTY(ccx, obj, id, val, nsnull, nsnull,
                                    propFlags, nsnull);
@@ -440,24 +440,12 @@ DefinePropertyIfFound(XPCCallContext& ccx,
         idval == rt->GetStringJSVal(XPCJSRuntime::IDX_QUERY_INTERFACE)))
         propFlags &= ~JSPROP_ENUMERATE;
 
-    JSObject* funobj;
+    jsval funval;
-    
+    if(!member->NewFunctionObject(ccx, iface, obj, &funval))
-    {
+        return JS_FALSE;
-        // scoped gc protection of funval
-        jsval funval;
-
-        if(!member->GetValue(ccx, iface, &funval))
-            return JS_FALSE;
-    
-        AUTO_MARK_JSVAL(ccx, funval);
-
-        funobj = xpc_CloneJSFunction(ccx, JSVAL_TO_OBJECT(funval), obj);
-        if(!funobj)
-            return JS_FALSE;
-    }
 
     // protect funobj until it is actually attached
-    AUTO_MARK_JSVAL(ccx, OBJECT_TO_JSVAL(funobj));
+    AUTO_MARK_JSVAL(ccx, funval);
 
 #ifdef off_DEBUG_jband
     {
@@ -473,8 +461,8 @@ DefinePropertyIfFound(XPCCallContext& ccx,
         if(resolved)
             *resolved = JS_TRUE;
         return JS_ValueToId(ccx, idval, &id) &&
-               OBJ_DEFINE_PROPERTY(ccx, obj, id, OBJECT_TO_JSVAL(funobj),
+               OBJ_DEFINE_PROPERTY(ccx, obj, id, funval, nsnull, nsnull,
-                                   nsnull, nsnull, propFlags, nsnull);
+                                   propFlags, nsnull);
     }
 
     // else...
@@ -491,6 +479,8 @@ DefinePropertyIfFound(XPCCallContext& ccx,
     AutoResolveName arn(ccx, idval);
     if(resolved)
         *resolved = JS_TRUE;
+
+    JSObject* funobj = JSVAL_TO_OBJECT(funval);
     return JS_ValueToId(ccx, idval, &id) &&
            OBJ_DEFINE_PROPERTY(ccx, obj, id, JSVAL_VOID,
                                (JSPropertyOp) funobj,