Bug 613515 - JS properties set from chrome are lost. r=mrbkap@gmail.com, a=blocker
authorAndreas Gal <gal@uci.edu>
Tue, 18 Jan 2011 16:47:22 -0800
changeset 61214 5a6e9e7e487a04c6bcdcca3220c7d55644e11e0a
parent 61213 c22d4fa6edee89763dd08d25b7d2a2b8ccf42153
child 61215 a4b72ddb9e6e8af2fca372d1f5546d3330a24daa
push id18277
push usercleary@mozilla.com
push dateTue, 25 Jan 2011 03:52:51 +0000
treeherdermozilla-central@7ee91bd90e7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap, blocker
bugs613515
milestone2.0b10pre
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 613515 - JS properties set from chrome are lost. r=mrbkap@gmail.com, a=blocker
js/src/xpconnect/src/xpcjsruntime.cpp
js/src/xpconnect/src/xpcprivate.h
js/src/xpconnect/wrappers/XrayWrapper.cpp
--- a/js/src/xpconnect/src/xpcjsruntime.cpp
+++ b/js/src/xpconnect/src/xpcjsruntime.cpp
@@ -239,16 +239,18 @@ ContextCallback(JSContext *cx, uintN ope
     }
     return JS_TRUE;
 }
 
 xpc::CompartmentPrivate::~CompartmentPrivate()
 {
     if (waiverWrapperMap)
         delete waiverWrapperMap;
+    if (expandoMap)
+        delete expandoMap;
 }
 
 static JSBool
 CompartmentCallback(JSContext *cx, JSCompartment *compartment, uintN op)
 {
     if(op == JSCOMPARTMENT_NEW)
         return JS_TRUE;
 
@@ -397,16 +399,34 @@ TraceJSHolder(JSDHashTable *table, JSDHa
 }
 
 struct ClearedGlobalObject : public JSDHashEntryHdr
 {
     JSContext* mContext;
     JSObject* mGlobalObject;
 };
 
+static PLDHashOperator
+TraceExpandos(XPCWrappedNative *wn, JSObject *expando, void *aClosure)
+{
+    JS_CALL_OBJECT_TRACER(static_cast<JSTracer *>(aClosure), expando, "expando object");
+    return PL_DHASH_NEXT;
+}
+
+
+static PLDHashOperator
+TraceCompartment(nsCStringHashKey& aKey, JSCompartment *compartment, void *aClosure)
+{
+    xpc::CompartmentPrivate *priv = (xpc::CompartmentPrivate *)
+        JS_GetCompartmentPrivate(static_cast<JSTracer *>(aClosure)->context, compartment);
+    if (priv->expandoMap)
+        priv->expandoMap->EnumerateRead(TraceExpandos, (JSContext *)aClosure);
+    return PL_DHASH_NEXT;
+}
+
 void XPCJSRuntime::TraceXPConnectRoots(JSTracer *trc)
 {
     JSContext *iter = nsnull, *acx;
     while ((acx = JS_ContextIterator(GetJSRuntime(), &iter))) {
         JS_ASSERT(JS_HAS_OPTION(acx, JSOPTION_UNROOTED_GLOBAL));
         if (acx->globalObject)
             JS_CALL_OBJECT_TRACER(trc, acx->globalObject, "global object");
     }
@@ -418,16 +438,20 @@ void XPCJSRuntime::TraceXPConnectRoots(J
     for(XPCRootSetElem *e = mVariantRoots; e ; e = e->GetNextRoot())
         static_cast<XPCTraceableVariant*>(e)->TraceJS(trc);
 
     for(XPCRootSetElem *e = mWrappedJSRoots; e ; e = e->GetNextRoot())
         static_cast<nsXPCWrappedJS*>(e)->TraceJS(trc);
 
     if(mJSHolders.ops)
         JS_DHashTableEnumerate(&mJSHolders, TraceJSHolder, trc);
+
+    // Trace compartments.
+    GetCompartmentMap().EnumerateRead((XPCCompartmentMap::EnumReadFunction)
+                                      TraceCompartment, trc);
 }
 
 struct Closure
 {
     JSContext *cx;
     bool cycleCollectionEnabled;
     nsCycleCollectionTraversalCallback *cb;
 };
@@ -559,22 +583,33 @@ SweepWaiverWrappers(JSDHashTable *table,
     JSObject *key = ((JSObject2JSObjectMap::Entry *)hdr)->key;
     JSObject *value = ((JSObject2JSObjectMap::Entry *)hdr)->value;
     if(IsAboutToBeFinalized(cx, key) || IsAboutToBeFinalized(cx, value))
         return JS_DHASH_REMOVE;
     return JS_DHASH_NEXT;
 }
 
 static PLDHashOperator
+SweepExpandos(XPCWrappedNative *wn, JSObject *&expando, void *arg)
+{
+    JSContext *cx = (JSContext *)arg;
+    return IsAboutToBeFinalized(cx, wn->GetFlatJSObjectNoMark())
+           ? PL_DHASH_REMOVE
+           : PL_DHASH_NEXT;
+}
+
+static PLDHashOperator
 SweepCompartment(nsCStringHashKey& aKey, JSCompartment *compartment, void *aClosure)
 {
     xpc::CompartmentPrivate *priv = (xpc::CompartmentPrivate *)
         JS_GetCompartmentPrivate((JSContext *)aClosure, compartment);
     if (priv->waiverWrapperMap)
         priv->waiverWrapperMap->Enumerate(SweepWaiverWrappers, (JSContext *)aClosure);
+    if (priv->expandoMap)
+        priv->expandoMap->Enumerate(SweepExpandos, (JSContext *)aClosure);
     return PL_DHASH_NEXT;
 }
 
 // static
 JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status)
 {
     XPCJSRuntime* self = nsXPConnect::GetRuntimeInstance();
     if(self)
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -4444,58 +4444,77 @@ xpc_SameScope(XPCWrappedNativeScope *obj
 
 nsISupports *
 XPC_GetIdentityObject(JSContext *cx, JSObject *obj);
 
 namespace xpc {
 
 struct CompartmentPrivate
 {
-  CompartmentPrivate(PtrAndPrincipalHashKey *key, bool wantXrays, bool cycleCollectionEnabled)
-    : key(key),
-      ptr(nsnull),
-      wantXrays(wantXrays),
-      cycleCollectionEnabled(cycleCollectionEnabled),
-      waiverWrapperMap(nsnull)
-  {
-  }
-
-  CompartmentPrivate(nsISupports *ptr, bool wantXrays, bool cycleCollectionEnabled)
-    : key(nsnull),
-      ptr(ptr),
-      wantXrays(wantXrays),
-      cycleCollectionEnabled(cycleCollectionEnabled),
-      waiverWrapperMap(nsnull)
-  {
-  }
-
-  ~CompartmentPrivate();
-
-  // NB: key and ptr are mutually exclusive.
-  nsAutoPtr<PtrAndPrincipalHashKey> key;
-  nsCOMPtr<nsISupports> ptr;
-  bool wantXrays;
-  bool cycleCollectionEnabled;
-  JSObject2JSObjectMap *waiverWrapperMap;
+    CompartmentPrivate(PtrAndPrincipalHashKey *key, bool wantXrays, bool cycleCollectionEnabled)
+        : key(key),
+          ptr(nsnull),
+          wantXrays(wantXrays),
+          cycleCollectionEnabled(cycleCollectionEnabled),
+          waiverWrapperMap(nsnull),
+          expandoMap(nsnull)
+    {
+    }
+
+    CompartmentPrivate(nsISupports *ptr, bool wantXrays, bool cycleCollectionEnabled)
+        : key(nsnull),
+          ptr(ptr),
+          wantXrays(wantXrays),
+          cycleCollectionEnabled(cycleCollectionEnabled),
+          waiverWrapperMap(nsnull),
+          expandoMap(nsnull)
+    {
+    }
+
+    ~CompartmentPrivate();
+
+    // NB: key and ptr are mutually exclusive.
+    nsAutoPtr<PtrAndPrincipalHashKey> key;
+    nsCOMPtr<nsISupports> ptr;
+    bool wantXrays;
+    bool cycleCollectionEnabled;
+    JSObject2JSObjectMap *waiverWrapperMap;
+    // NB: we don't want this map to hold a strong reference to the wrapper.
+    nsDataHashtable<nsPtrHashKey<XPCWrappedNative>, JSObject *> *expandoMap;
+
+    bool RegisterExpandoObject(XPCWrappedNative *wn, JSObject *expando) {
+        if (!expandoMap) {
+            expandoMap = new nsDataHashtable<nsPtrHashKey<XPCWrappedNative>, JSObject *>();
+            if (!expandoMap->Init(8))
+                return false;
+        }
+        return expandoMap->Put(wn, expando);
+    }
+
+    JSObject *LookupExpandoObject(XPCWrappedNative *wn) {
+        if (!expandoMap)
+            return nsnull;
+        return expandoMap->Get(wn);
+    }
 };
 
 inline bool
 CompartmentParticipatesInCycleCollection(JSContext *cx, JSCompartment *compartment)
 {
-   CompartmentPrivate *priv =
-       static_cast<CompartmentPrivate *>(JS_GetCompartmentPrivate(cx, compartment));
-   NS_ASSERTION(priv, "This should never be null!");
-
-   return priv->cycleCollectionEnabled;
+    CompartmentPrivate *priv =
+        static_cast<CompartmentPrivate *>(JS_GetCompartmentPrivate(cx, compartment));
+    NS_ASSERTION(priv, "This should never be null!");
+
+    return priv->cycleCollectionEnabled;
 }
 
 inline bool
 ParticipatesInCycleCollection(JSContext *cx, js::gc::Cell *cell)
 {
-   return CompartmentParticipatesInCycleCollection(cx, cell->compartment());
+    return CompartmentParticipatesInCycleCollection(cx, cell->compartment());
 }
 
 }
 
 #ifdef XPC_IDISPATCH_SUPPORT
 
 #ifdef WINCE
 /* defined static near the top here */
--- a/js/src/xpconnect/wrappers/XrayWrapper.cpp
+++ b/js/src/xpconnect/wrappers/XrayWrapper.cpp
@@ -134,25 +134,49 @@ GetWrappedNativeObjectFromHolder(JSConte
 {
     NS_ASSERTION(holder->getJSClass() == &HolderClass, "expected a native property holder object");
     JSObject *wrappedObj = &holder->getSlot(JSSLOT_WN_OBJ).toObject();
     OBJ_TO_INNER_OBJECT(cx, wrappedObj);
     return wrappedObj;
 }
 
 static JSObject *
-GetExpandoObject(JSContext *cx, JSObject *holder)
+GetExpandoObject(JSObject *holder)
+{
+    NS_ASSERTION(holder->getJSClass() == &HolderClass, "expected a native property holder object");
+    return holder->getSlot(JSSLOT_EXPANDO).toObjectOrNull();
+}
+
+static JSObject *
+EnsureExpandoObject(JSContext *cx, JSObject *holder)
 {
-    JSObject *expando = holder->getSlot(JSSLOT_EXPANDO).toObjectOrNull();
+    NS_ASSERTION(holder->getJSClass() == &HolderClass, "expected a native property holder object");
+    JSObject *expando = GetExpandoObject(holder);
+    if (expando)
+        return expando;
+    CompartmentPrivate *priv =
+        (CompartmentPrivate *)JS_GetCompartmentPrivate(cx, holder->compartment());
+    XPCWrappedNative *wn = GetWrappedNative(GetWrappedNativeObjectFromHolder(cx, holder));
+    expando = priv->LookupExpandoObject(wn);
     if (!expando) {
-        expando =  JS_NewObjectWithGivenProto(cx, nsnull, nsnull, holder->getParent());
+        expando = JS_NewObjectWithGivenProto(cx, nsnull, nsnull, holder->getParent());
         if (!expando)
             return NULL;
-        holder->setSlot(JSSLOT_EXPANDO, ObjectValue(*expando));
+        // Add the expando object to the expando map to keep it alive.
+        if (!priv->RegisterExpandoObject(wn, expando)) {
+            JS_ReportOutOfMemory(cx);
+            return NULL;
+        }
+        // Make sure the wn stays alive so it keeps the expando object alive.
+        nsRefPtr<nsXPCClassInfo> ci;
+        CallQueryInterface(wn->Native(), getter_AddRefs(ci));
+        if (ci)
+            ci->PreserveWrapper(wn->Native());
     }
+    holder->setSlot(JSSLOT_EXPANDO, ObjectValue(*expando));
     return expando;
 }
 
 // Some DOM objects have shared properties that don't have an explicit
 // getter/setter and rely on the class getter/setter. We install a
 // class getter/setter on the holder object to trigger them.
 static JSBool
 holder_get(JSContext *cx, JSObject *wrapper, jsid id, jsval *vp)
@@ -420,29 +444,28 @@ XrayWrapper<Base, Policy>::resolveOwnPro
         desc->getter = wrappedJSObject_getter;
         desc->setter = NULL;
         desc->shortid = 0;
         desc->value = JSVAL_VOID;
         return true;
     }
 
     JSObject *holder = GetHolder(wrapper);
-    JSObject *expando = GetExpandoObject(cx, holder);
-    if (!expando)
-        return false;
+    JSObject *expando = GetExpandoObject(holder);
+    if (expando) {
+        if (!JS_GetPropertyDescriptorById(cx, expando, id,
+                                          (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED,
+                                          desc)) {
+            return false;
+        }
 
-    if (!JS_GetPropertyDescriptorById(cx, expando, id,
-                                      (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED,
-                                      desc)) {
-        return false;
+        if (desc->obj)
+            return true;
     }
 
-    if (desc->obj)
-        return true;
-
     JSObject *wnObject = GetWrappedNativeObjectFromHolder(cx, holder);
     XPCWrappedNative *wn = GetWrappedNative(wnObject);
 
     // Run the resolve hook of the wrapped native.
     if (NATIVE_HAS_FLAG(wn, WantNewResolve)) {
         JSBool retval = true;
         JSObject *pobj = NULL;
         uintN flags = (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED;
@@ -622,17 +645,17 @@ XrayWrapper<Base, Policy>::definePropert
             if (!desc->setter)
                 jsdesc->setter = holder_set;
         }
 
         return JS_DefinePropertyById(cx, holder, id, jsdesc->value, jsdesc->getter, jsdesc->setter,
                                      jsdesc->attrs);
     }
 
-    JSObject *expando = GetExpandoObject(cx, holder);
+    JSObject *expando = EnsureExpandoObject(cx, holder);
     if (!expando)
         return false;
 
     return JS_DefinePropertyById(cx, expando, id, jsdesc->value, jsdesc->getter, jsdesc->setter,
                                  jsdesc->attrs);
 }
 
 static bool
@@ -652,21 +675,18 @@ EnumerateNames(JSContext *cx, JSObject *
     }
 
     if (WrapperFactory::IsPartiallyTransparent(wrapper)) {
         JS_ReportError(cx, "Not allowed to enumerate cross origin objects");
         return false;
     }
 
     // Enumerate expando properties first.
-    JSObject *expando = GetExpandoObject(cx, holder);
-    if (!expando)
-        return false;
-
-    if (!js::GetPropertyNames(cx, expando, flags, &props))
+    JSObject *expando = GetExpandoObject(holder);
+    if (expando && !js::GetPropertyNames(cx, expando, flags, &props))
         return false;
 
     // Force all native properties to be materialized onto the wrapped native.
     js::AutoIdVector wnProps(cx);
     {
         JSAutoEnterCompartment ac;
         if (!ac.enter(cx, wnObject))
             return false;
@@ -711,22 +731,24 @@ XrayWrapper<Base, Policy>::delete_(JSCon
             return false;
 
         if (!JS_DeletePropertyById2(cx, wnObject, id, &v) || !JS_ValueToBoolean(cx, v, &b))
             return false;
         *bp = !!b;
         return true;
     }
 
-    JSObject *expando = GetExpandoObject(cx, holder);
-    if (!expando)
+    JSObject *expando = GetExpandoObject(holder);
+    b = true;
+    if (expando &&
+        (!JS_DeletePropertyById2(cx, expando, id, &v) ||
+         !JS_ValueToBoolean(cx, v, &b))) {
         return false;
+    }
 
-    if (!JS_DeletePropertyById2(cx, expando, id, &v) || !JS_ValueToBoolean(cx, v, &b))
-        return false;
     *bp = !!b;
     return true;
 }
 
 template <typename Base, typename Policy>
 bool
 XrayWrapper<Base, Policy>::enumerate(JSContext *cx, JSObject *wrapper, js::AutoIdVector &props)
 {
@@ -798,21 +820,28 @@ XrayWrapper<Base, Policy>::iterate(JSCon
 template <typename Base, typename Policy>
 JSObject *
 XrayWrapper<Base, Policy>::createHolder(JSContext *cx, JSObject *wrappedNative, JSObject *parent)
 {
     JSObject *holder = JS_NewObjectWithGivenProto(cx, &HolderClass, nsnull, parent);
     if (!holder)
         return nsnull;
 
+    CompartmentPrivate *priv =
+        (CompartmentPrivate *)JS_GetCompartmentPrivate(cx, holder->compartment());
+    JSObject *inner = wrappedNative;
+    OBJ_TO_INNER_OBJECT(cx, inner);
+    XPCWrappedNative *wn = GetWrappedNative(inner);
+    Value expando = ObjectOrNullValue(priv->LookupExpandoObject(wn));
+
     JS_ASSERT(IS_WN_WRAPPER(wrappedNative) ||
               wrappedNative->getClass()->ext.innerObject);
     holder->setSlot(JSSLOT_WN_OBJ, ObjectValue(*wrappedNative));
     holder->setSlot(JSSLOT_RESOLVING, PrivateValue(NULL));
-    holder->setSlot(JSSLOT_EXPANDO, NullValue());
+    holder->setSlot(JSSLOT_EXPANDO, expando);
     return holder;
 }
 
 bool
 CrossCompartmentXray::enter(JSContext *cx, JSObject *wrapper, jsid *idp,
                             JSWrapper::Action act, void **priv)
 {
     JSObject *target = wrapper->unwrap();