Preserve XPCNativeWrappers when "expando" properties are set on them. Bug

295937, r=shaver, sr+a=brendan

content/xbl/src/nsXBLBinding.cpp

@@ -102,8 +102,7 @@
 
 #include "prprf.h"
 
-nsresult NS_DOMClassInfo_PreserveWrapper(nsIDOMNode *aDOMNode,
-                                         nsIXPConnectWrappedNative *aWrapper);
+nsresult NS_DOMClassInfo_PreserveWrapper(nsIXPConnectWrappedNative *aWrapper);
 
 // Helper classes
 
@@ -1142,8 +1141,7 @@ nsXBLBinding::InitClass(const nsCString& aClassName,
       do_QueryInterface(wrapper);
 
     if (native_wrapper) {
-      nsCOMPtr<nsIDOMNode> node(do_QueryInterface(mBoundElement));
-      NS_DOMClassInfo_PreserveWrapper(node, native_wrapper);
+      NS_DOMClassInfo_PreserveWrapper(native_wrapper);
     }
   }
 

content/xbl/src/nsXBLProtoImpl.cpp

@@ -48,8 +48,7 @@
 #include "nsIXBLDocumentInfo.h"
 #include "nsIDOMNode.h"
 
-nsresult NS_DOMClassInfo_PreserveWrapper(nsIDOMNode *aDOMNode,
-                                         nsIXPConnectWrappedNative *aWrapper);
+nsresult NS_DOMClassInfo_PreserveWrapper(nsIXPConnectWrappedNative *aWrapper);
 
 nsresult
 nsXBLProtoImpl::InstallImplementation(nsXBLPrototypeBinding* aBinding, nsIContent* aBoundElement)
@@ -142,8 +141,7 @@ nsXBLProtoImpl::InitTargetObjects(nsXBLPrototypeBinding* aBinding,
   if (doc) {
     nsCOMPtr<nsIXPConnectWrappedNative> nativeWrapper(do_QueryInterface(wrapper));
     if (nativeWrapper) {
-      nsCOMPtr<nsIDOMNode> node(do_QueryInterface(aBoundElement));
-      NS_DOMClassInfo_PreserveWrapper(node, nativeWrapper);
+      NS_DOMClassInfo_PreserveWrapper(nativeWrapper);
     }
   }
 

dom/src/base/nsDOMClassInfo.cpp

@@ -4519,9 +4519,14 @@ static const PLDHashTableOps sWrapperSCCTableOps = {
 
 // static
 nsresult
-nsDOMClassInfo::PreserveWrapper(nsIDOMNode *aDOMNode,
-                                nsIXPConnectWrappedNative *aWrapper)
+nsDOMClassInfo::PreserveWrapper(nsIXPConnectWrappedNative *aWrapper)
 {
+  nsCOMPtr<nsIDOMNode> node = do_QueryWrappedNative(aWrapper);
+  if (!node) {
+    return NS_OK;
+  }
+  nsIDOMNode* nodePtr = node;
+  
   if (!sPreservedWrapperTable.ops &&
       !PL_DHashTableInit(&sPreservedWrapperTable, PL_DHashGetStubOps(), nsnull,
                          sizeof(PreservedWrapperEntry), 16)) {
@@ -4530,11 +4535,11 @@ nsDOMClassInfo::PreserveWrapper(nsIDOMNode *aDOMNode,
   }
 
   PreservedWrapperEntry *entry = NS_STATIC_CAST(PreservedWrapperEntry*,
-    PL_DHashTableOperate(&sPreservedWrapperTable, aDOMNode, PL_DHASH_ADD));
+    PL_DHashTableOperate(&sPreservedWrapperTable, nodePtr, PL_DHASH_ADD));
   if (!entry)
     return NS_ERROR_OUT_OF_MEMORY;
 
-  entry->key = aDOMNode;
+  entry->key = nodePtr;
   entry->wrapper = aWrapper;
 
   return NS_OK;
@@ -4699,10 +4704,9 @@ nsDOMClassInfo::EndGCMark()
 
 // hack to give XBL access to nsDOMClassInfo::PreserveWrapper
 nsresult
-NS_DOMClassInfo_PreserveWrapper(nsIDOMNode *aDOMNode,
-                                nsIXPConnectWrappedNative *aWrapper)
+NS_DOMClassInfo_PreserveWrapper(nsIXPConnectWrappedNative *aWrapper)
 {
-  return nsDOMClassInfo::PreserveWrapper(aDOMNode, aWrapper);
+  return nsDOMClassInfo::PreserveWrapper(aWrapper);
 }
 
 // static
@@ -5543,12 +5547,12 @@ NS_IMETHODIMP
 nsNodeSH::AddProperty(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
                       JSObject *obj, jsval id, jsval *vp, PRBool *_retval)
 {
-  nsISupports *native = wrapper->Native();
-  nsCOMPtr<nsIDOMNode> node(do_QueryInterface(native));
-
   // This can fail on out-of-memory, which should end up throwing a JS
   // exception.
-  return nsDOMClassInfo::PreserveWrapper(node, wrapper);
+  nsresult rv = nsDOMClassInfo::PreserveWrapper(wrapper);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return nsEventReceiverSH::AddProperty(wrapper, cx, obj, id, vp, _retval);
 }
 
 NS_IMETHODIMP

dom/src/base/nsDOMClassInfo.h

@@ -136,10 +136,11 @@ public:
   static JSClass sDOMJSClass;
 
   /**
-   * Note that the XPConnect wrapper for |aDOMNode| should be preserved.
+   * Note that the XPConnect wrapper should be preserved.  This will only
+   * preserve aWrapper if its native QIs to nsIDOMNode; otherwise it'll just
+   * return NS_OK.
    */
-  static nsresult PreserveWrapper(nsIDOMNode *aDOMNode,
-                                  nsIXPConnectWrappedNative *aWrapper);
+  static nsresult PreserveWrapper(nsIXPConnectWrappedNative *aWrapper);
 
   /**
    * Undoes the effects of any prior |PreserveWrapper| calls on

dom/src/base/nsJSEnvironment.cpp

@@ -2037,6 +2037,14 @@ nsJSContext::ScriptExecuted()
   return NS_OK;
 }
 
+nsresult NS_DOMClassInfo_PreserveWrapper(nsIXPConnectWrappedNative *aWrapper);
+
+NS_IMETHODIMP
+nsJSContext::PreserveWrapper(nsIXPConnectWrappedNative *aWrapper)
+{
+  return NS_DOMClassInfo_PreserveWrapper(aWrapper);
+}
+
 NS_IMETHODIMP
 nsJSContext::Notify(nsITimer *timer)
 {

js/src/xpconnect/idl/nsIXPCScriptNotify.idl

@@ -48,12 +48,19 @@
 
 #include "nsISupports.idl"
 
-[uuid(2d45f33d-fd6e-4f1b-90e5-73e38478d836)]
+interface nsIXPConnectWrappedNative;
+
+[uuid(b804504d-0025-4d6b-8ced-d94e41102a7f)]
 interface nsIXPCScriptNotify : nsISupports
 {
     /**
      * Method invoked when a script has been executed by XPConnect
      */
     void ScriptExecuted();
+
+    /**
+     * Method invoked to preserve an nsIXPConnectWrappedNative as needed
+     */
+    void preserveWrapper(in nsIXPConnectWrappedNative wrapper);
 };
 

js/src/xpconnect/src/XPCNativeWrapper.cpp

@@ -556,10 +556,27 @@ XPC_NW_Enumerate(JSContext *cx, JSObject *obj)
   return JS_TRUE;
 }
 
+static
+JSBool MaybePreserveWrapper(JSContext* cx, XPCWrappedNative *wn, uintN flags)
+{
+  if ((flags & JSRESOLVE_ASSIGNING) &&
+      (::JS_GetOptions(cx) & JSOPTION_PRIVATE_IS_NSISUPPORTS)) {
+    nsCOMPtr<nsIXPCScriptNotify> scriptNotify = 
+      do_QueryInterface(NS_STATIC_CAST(nsISupports*,
+                                       JS_GetContextPrivate(cx)));
+    if (scriptNotify) {
+      return NS_SUCCEEDED(scriptNotify->PreserveWrapper(wn));
+    }
+  }
+  return JS_TRUE;
+}
+
 JS_STATIC_DLL_CALLBACK(JSBool)
 XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
                   JSObject **objp)
 {
+  // No need to preserve on sets of wrappedJSObject or toString, since
+  // callers couldn't get at those values anyway.
   if (id == GetStringByIndex(cx, XPCJSRuntime::IDX_WRAPPED_JSOBJECT) ||
       id == GetStringByIndex(cx, XPCJSRuntime::IDX_TO_STRING)) {
     return JS_TRUE;
@@ -619,6 +636,9 @@ XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
     // An index is being resolved. Define the property and deal with
     // the value in the get/set property hooks.
 
+    // Note that we don't have to worry about preserving here, since
+    // numeric ids can't be assigned to.
+
     if (!::JS_DefineElement(cx, obj, JSVAL_TO_INT(id), JSVAL_VOID, nsnull,
                             nsnull, 0)) {
       return JS_FALSE;
@@ -633,7 +653,7 @@ XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
     // A non-int and non-string id is being resolved. Won't be found
     // here, return early.
 
-    return JS_TRUE;
+    return MaybePreserveWrapper(cx, wrappedNative, flags);
   }
 
   JSObject *nativeObj = wrappedNative->GetFlatJSObject();
@@ -654,7 +674,7 @@ XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
   if (!iface) {
     // No interface, nothing to resolve.
 
-    return JS_TRUE;
+    return MaybePreserveWrapper(cx, wrappedNative, flags);
   }
 
   // did we find a method/attribute by that name?
@@ -663,7 +683,7 @@ XPC_NW_NewResolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
   if (!member) {
     // No member, nothing to resolve.
 
-    return JS_TRUE;
+    return MaybePreserveWrapper(cx, wrappedNative, flags);
   }
 
   // Get (and perhaps lazily create) the member's value (commonly a