Bug 614347 - 'XPConnect-wrapped JSObjects must clear their gray bit when they are handed out'. r=peterv+gal, a=blocking.

content/base/public/nsContentUtils.h

@@ -1280,7 +1280,8 @@ public:
                            void *aClosure)
   {
     if (aCache->PreservingWrapper()) {
-      aCallback(nsIProgrammingLanguage::JAVASCRIPT, aCache->GetWrapper(),
+      aCallback(nsIProgrammingLanguage::JAVASCRIPT,
+                aCache->GetWrapperPreserveColor(),
                 aClosure);
     }
   }

content/base/src/nsDOMDocumentType.cpp

@@ -51,6 +51,7 @@
 #include "nsIDocument.h"
 #include "nsIXPConnect.h"
 #include "nsIDOMDocument.h"
+#include "xpcpublic.h"
 
 nsresult
 NS_NewDOMDocumentType(nsIDOMDocumentType** aDocType,

content/base/src/nsDocument.cpp

@@ -3802,7 +3802,7 @@ nsDocument::SetScriptGlobalObject(nsIScriptGlobalObject *aScriptGlobalObject)
     if (!mWillReparent) {
       // We really shouldn't have a wrapper here but if we do we need to make sure
       // it has the correct parent.
-      JSObject *obj = GetWrapper();
+      JSObject *obj = GetWrapperPreserveColor();
       if (obj) {
         JSObject *newScope = aScriptGlobalObject->GetGlobalJSObject();
         nsIScriptContext *scx = aScriptGlobalObject->GetContext();

dom/base/nsDOMClassInfo.cpp

@@ -71,6 +71,7 @@
 #include "nsIRunnable.h"
 #include "nsThreadUtils.h"
 #include "nsDOMEventTargetWrapperCache.h"
+#include "xpcpublic.h"
 
 // General helper includes
 #include "nsGlobalWindow.h"
@@ -4246,6 +4247,31 @@ nsDOMClassInfo::GetArrayIndexFromId(JSContext *cx, jsid id, PRBool *aIsNumber)
   return i;
 }
 
+// static
+nsresult
+nsDOMClassInfo::WrapNative(JSContext *cx, JSObject *scope,
+                           nsISupports *native, nsWrapperCache *cache,
+                           const nsIID* aIID, jsval *vp,
+                           nsIXPConnectJSObjectHolder** aHolder,
+                           PRBool aAllowWrapping)
+{
+  if (!native) {
+    NS_ASSERTION(!aHolder || !*aHolder, "*aHolder should be null!");
+
+    *vp = JSVAL_NULL;
+
+    return NS_OK;
+  }
+
+  JSObject *wrapper = xpc_GetCachedSlimWrapper(cache, scope, vp);
+  if (wrapper) {
+    return NS_OK;
+  }
+
+  return sXPConnect->WrapNativeToJSVal(cx, scope, native, cache, aIID,
+                                       aAllowWrapping, vp, aHolder);
+}
+
 NS_IMETHODIMP
 nsDOMClassInfo::GetInterfaces(PRUint32 *aCount, nsIID ***aArray)
 {

dom/base/nsDOMClassInfo.h

@@ -48,7 +48,6 @@
 #include "nsDOMJSUtils.h" // for GetScriptContextFromJSContext
 #include "nsIScriptGlobalObject.h"
 #include "nsContentUtils.h"
-#include "xpcpublic.h"
 
 class nsIDOMWindow;
 class nsIDOMNSHTMLOptionCollection;
@@ -249,28 +248,11 @@ protected:
             id == sName_id);
   }
 
-  static nsresult WrapNative(JSContext *cx, JSObject *scope,
+  inline static nsresult WrapNative(JSContext *cx, JSObject *scope,
-                             nsISupports *native, nsWrapperCache *cache,
+                                    nsISupports *native, nsWrapperCache *cache,
-                             const nsIID* aIID, jsval *vp,
+                                    const nsIID* aIID, jsval *vp,
-                             nsIXPConnectJSObjectHolder** aHolder,
+                                    nsIXPConnectJSObjectHolder** aHolder,
-                             PRBool aAllowWrapping)
+                                    PRBool aAllowWrapping);
-  {
-    if (!native) {
-      NS_ASSERTION(!aHolder || !*aHolder, "*aHolder should be null!");
-
-      *vp = JSVAL_NULL;
-
-      return NS_OK;
-    }
-
-    JSObject *wrapper = xpc_GetCachedSlimWrapper(cache, scope, vp);
-    if (wrapper) {
-      return NS_OK;
-    }
-
-    return sXPConnect->WrapNativeToJSVal(cx, scope, native, cache, aIID,
-                                         aAllowWrapping, vp, aHolder);
-  }
 
   static nsIXPConnect *sXPConnect;
   static nsIScriptSecurityManager *sSecMan;

dom/base/nsJSEnvironment.cpp

@@ -85,6 +85,7 @@
 #include "nsIXULRuntime.h"
 
 #include "nsDOMClassInfo.h"
+#include "xpcpublic.h"
 
 #include "jsdbgapi.h"           // for JS_ClearWatchPointsForObject
 #include "jsxdrapi.h"

dom/base/nsWrapperCache.h

@@ -42,6 +42,7 @@
 #include "nsCycleCollectionParticipant.h"
 
 struct JSObject;
+class nsContentUtils;
 
 typedef PRUptrdiff PtrBits;
 
@@ -58,6 +59,8 @@ typedef PRUptrdiff PtrBits;
  */
 class nsWrapperCache
 {
+  friend class nsContentUtils;
+
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_WRAPPERCACHE_IID)
 
@@ -70,7 +73,25 @@ public:
                  "Destroying cache with a preserved wrapper!");
   }
 
-  JSObject* GetWrapper() const
+  /**
+   * This getter clears the gray bit before handing out the JSObject which means
+   * that the object is guaranteed to be kept alive past the next CC.
+   *
+   * Implemented in xpcpublic.h because we have to include some JS headers that
+   * don't play nicely with the rest of the codebase. Include xpcpublic.h if you
+   * need to call this method.
+   */
+  inline JSObject* GetWrapper() const;
+
+  /**
+   * This getter does not change the color of the JSObject meaning that the
+   * object returned is not guaranteed to be kept alive past the next CC.
+   *
+   * This should only be called if you are certain that the return value won't
+   * be passed into a JS API function and that it won't be stored without being
+   * rooted (or otherwise signaling the stored value to the CC).
+   */
+  JSObject* GetWrapperPreserveColor() const
   {
     return reinterpret_cast<JSObject*>(mWrapperPtrBits & ~kWrapperBitMask);
   }
@@ -88,6 +109,23 @@ public:
     mWrapperPtrBits = 0;
   }
 
+  PRBool PreservingWrapper()
+  {
+    return (mWrapperPtrBits & WRAPPER_BIT_PRESERVED) != 0;
+  }
+
+  void SetIsProxy()
+  {
+    mWrapperPtrBits |= WRAPPER_IS_PROXY;
+  }
+
+  PRBool IsProxy()
+  {
+    return (mWrapperPtrBits & WRAPPER_IS_PROXY) != 0;
+  }
+
+private:
+  // Only meant to be called by nsContentUtils.
   void SetPreservingWrapper(PRBool aPreserve)
   {
     if(aPreserve) {
@@ -98,22 +136,6 @@ public:
     }
   }
 
-  PRBool PreservingWrapper()
-  {
-    return (mWrapperPtrBits & WRAPPER_BIT_PRESERVED) != 0;
-  }
-
-  void SetIsProxy()
-  {
-      mWrapperPtrBits |= WRAPPER_IS_PROXY;
-  }
-
-  PRBool IsProxy()
-  {
-      return (mWrapperPtrBits & WRAPPER_IS_PROXY) != 0;
-  }
-
-private:
   enum { WRAPPER_BIT_PRESERVED = 1 << 0 };
   enum { WRAPPER_IS_PROXY = 1 << 1 };
   enum { kWrapperBitMask = (WRAPPER_BIT_PRESERVED | WRAPPER_IS_PROXY) };

js/src/jscell.h

@@ -71,6 +71,7 @@ struct Cell {
 
     JS_ALWAYS_INLINE bool isMarked(uint32 color) const;
     JS_ALWAYS_INLINE bool markIfUnmarked(uint32 color) const;
+    JS_ALWAYS_INLINE void unmark(uint32 color) const;
 
     inline JSCompartment *compartment() const;
 

js/src/jsgc.cpp

@@ -505,7 +505,7 @@ IsAboutToBeFinalized(JSContext *cx, void *thing)
 }
 
 JS_FRIEND_API(bool)
-js_GCThingIsMarked(void *thing, uint32 color = BLACK)
+js_GCThingIsMarked(void *thing, uintN color = BLACK)
 {
     JS_ASSERT(thing);
     AssertValidColor(thing, color);

js/src/jsgc.h

@@ -213,6 +213,13 @@ struct ArenaBitmap {
         return true;
     }
 
+    JS_ALWAYS_INLINE void unmark(size_t bit, uint32 color) {
+        bit += color;
+        JS_ASSERT(bit < BitCount);
+        uintptr_t *word = &bitmap[bit / JS_BITS_PER_WORD];
+        *word &= ~(uintptr_t(1) << (bit % JS_BITS_PER_WORD));
+    }
+
 #ifdef DEBUG
     bool noBitsSet() {
         for (unsigned i = 0; i < BitWords; i++) {
@@ -461,6 +468,14 @@ Cell::markIfUnmarked(uint32 color = BLACK) const
     return bitmap()->markIfUnmarked(cellIndex(), color);
 }
 
+void
+Cell::unmark(uint32 color) const
+{
+    JS_ASSERT(color != BLACK);
+    AssertValidColor(this, color);
+    bitmap()->unmark(cellIndex(), color);
+}
+
 JSCompartment *
 Cell::compartment() const
 {
@@ -800,7 +815,7 @@ extern JS_FRIEND_API(bool)
 IsAboutToBeFinalized(JSContext *cx, void *thing);
 
 extern JS_FRIEND_API(bool)
-js_GCThingIsMarked(void *thing, uint32 color);
+js_GCThingIsMarked(void *thing, uintN color);
 
 extern void
 js_TraceStackFrame(JSTracer *trc, JSStackFrame *fp);

js/src/xpconnect/idl/nsIXPConnect.idl

@@ -55,7 +55,8 @@
 #include "jspubtd.h"
 #include "xptinfo.h"
 #include "nsAXPCNativeCallContext.h"
-#include "nsWrapperCache.h"
+
+class nsWrapperCache;
 %}
 
 /***************************************************************************/

js/src/xpconnect/src/nsXPConnect.cpp

@@ -49,6 +49,7 @@
 #include "jsatom.h"
 #include "jsobj.h"
 #include "jsfun.h"
+#include "jsgc.h"
 #include "jsscript.h"
 #include "nsThreadUtilsInternal.h"
 #include "dom_quickstubs.h"
@@ -558,6 +559,43 @@ nsXPConnect::Unroot(void *p)
     return NS_OK;
 }
 
+static void
+UnmarkGrayChildren(JSTracer *trc, void *thing, uint32 kind)
+{
+    // If this thing is not a CC-kind or already non-gray then we're done.
+    if(!ADD_TO_CC(kind) || !xpc_IsGrayGCThing(thing))
+        return;
+
+    // Unmark.
+    static_cast<js::gc::Cell *>(thing)->unmark(XPC_GC_COLOR_GRAY);
+
+    // Trace children.
+    JS_TraceChildren(trc, thing, kind);
+}
+
+void
+xpc_UnmarkGrayObjectRecursive(JSObject *obj)
+{
+    NS_ASSERTION(obj, "Don't pass me null!");
+
+    // Unmark.
+    obj->unmark(XPC_GC_COLOR_GRAY);
+
+    // Tracing requires a JSContext...
+    JSContext *cx;
+    nsXPConnect* xpc = nsXPConnect::GetXPConnect();
+    if(!xpc || NS_FAILED(xpc->GetSafeJSContext(&cx)) || !cx)
+    {
+        NS_ERROR("Failed to get safe JSContext!");
+        return;
+    }
+
+    // Trace children.
+    JSTracer trc;
+    JS_TRACER_INIT(&trc, cx, UnmarkGrayChildren);
+    JS_TraceChildren(&trc, obj, JSTRACE_OBJECT);
+}
+
 struct TraversalTracer : public JSTracer
 {
     TraversalTracer(nsCycleCollectionTraversalCallback &aCb) : cb(aCb)
@@ -572,6 +610,12 @@ NoteJSChild(JSTracer *trc, void *thing, uint32 kind)
     if(ADD_TO_CC(kind))
     {
         TraversalTracer *tracer = static_cast<TraversalTracer*>(trc);
+
+        // There's no point in further traversing a non-gray object here unless
+        // we explicitly want to see all traces.
+        if(!xpc_IsGrayGCThing(thing) && !tracer->cb.WantAllTraces())
+            return;
+
 #if defined(DEBUG)
         if (NS_UNLIKELY(tracer->cb.WantDebugInfo())) {
             // based on DumpNotify in jsapi.c
@@ -613,12 +657,6 @@ WrapperIsNotMainThreadOnly(XPCWrappedNative *wrapper)
     return NS_FAILED(CallQueryInterface(wrapper->Native(), &participant));
 }
 
-JSBool
-nsXPConnect::IsGray(void *thing)
-{
-    return js_GCThingIsMarked(thing, XPC_GC_COLOR_GRAY);
-}
-
 NS_IMETHODIMP
 nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
 {
@@ -680,7 +718,7 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
 #endif
     {
         // Normal codepath (matches non-DEBUG_CC codepath).
-        type = !markJSObject && IsGray(p) ? GCUnmarked : GCMarked;
+        type = !markJSObject && xpc_IsGrayGCThing(p) ? GCUnmarked : GCMarked;
     }
 
     if (cb.WantDebugInfo()) {
@@ -775,7 +813,7 @@ nsXPConnect::Traverse(void *p, nsCycleCollectionTraversalCallback &cb)
 
     if(traceKind != JSTRACE_OBJECT || dontTraverse)
         return NS_OK;
-    
+
     if(clazz == &XPC_WN_Tearoff_JSClass)
     {
         // A tearoff holds a strong reference to its native object
@@ -1594,7 +1632,7 @@ MoveWrapper(XPCCallContext& ccx, XPCWrappedNative *wrapper,
 
         NS_ENSURE_SUCCESS(rv, rv);
 
-        newParent = parentWrapper->GetFlatJSObjectNoMark();
+        newParent = parentWrapper->GetFlatJSObject();
     }
     else
         NS_ASSERTION(betterScope == newScope, "Weird scope returned");

js/src/xpconnect/src/xpccallcontext.cpp

@@ -198,7 +198,7 @@ XPCCallContext::Init(XPCContext::LangType callerLanguage,
         {
             DEBUG_CheckWrapperThreadSafety(mWrapper);
 
-            mFlattenedJSObject = mWrapper->GetFlatJSObjectAndMark();
+            mFlattenedJSObject = mWrapper->GetFlatJSObject();
 
             if(mTearOff)
                 mScriptableInfo = nsnull;

js/src/xpconnect/src/xpcconvert.cpp

@@ -1310,7 +1310,7 @@ XPCConvert::NativeInterface2JSObject(XPCLazyCallContext& lccx,
 
     // If we're not creating security wrappers, we can return the
     // XPCWrappedNative as-is here.
-    flat = wrapper->GetFlatJSObjectAndMark();
+    flat = wrapper->GetFlatJSObject();
     jsval v = OBJECT_TO_JSVAL(flat);
     if(!XPCPerThreadData::IsMainThread(lccx.GetJSContext()) ||
        !allowNativeWrapper)

js/src/xpconnect/src/xpcinlines.h

@@ -791,7 +791,7 @@ XPCLazyCallContext::SetWrapper(XPCWrappedNative* wrapper,
     if(mTearOff)
         mFlattenedJSObject = mTearOff->GetJSObject();
     else
-        mFlattenedJSObject = mWrapper->GetFlatJSObjectAndMark();
+        mFlattenedJSObject = mWrapper->GetFlatJSObject();
 }
 inline void
 XPCLazyCallContext::SetWrapper(JSObject* flattenedJSObject)

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -94,8 +94,10 @@ WrappedJSDyingJSObjectFinder(JSDHashTable *table, JSDHashEntryHdr *hdr,
     {
         if(wrapper->IsSubjectToFinalization())
         {
-            js::SwitchToCompartment sc(data->cx, wrapper->GetJSObject());
+            js::SwitchToCompartment sc(data->cx,
-            if(JS_IsAboutToBeFinalized(data->cx, wrapper->GetJSObject()))
+                                       wrapper->GetJSObjectPreserveColor());
+            if(JS_IsAboutToBeFinalized(data->cx,
+                                       wrapper->GetJSObjectPreserveColor()))
                 data->array->AppendElement(wrapper);
         }
         wrapper = wrapper->GetNextWrapper();
@@ -503,16 +505,13 @@ XPCJSRuntime::SuspectWrappedNative(JSContext *cx, XPCWrappedNative *wrapper,
 
     // Only suspect wrappedJSObjects that are in a compartment that
     // participates in cycle collection.
-    JSObject* obj = wrapper->GetFlatJSObjectAndMark();
+    JSObject* obj = wrapper->GetFlatJSObjectPreserveColor();
     if(!xpc::ParticipatesInCycleCollection(cx, obj))
         return;
 
-    NS_ASSERTION(!JS_IsAboutToBeFinalized(cx, obj),
-                 "SuspectWrappedNative attempting to touch dead object");
-
     // Only record objects that might be part of a cycle as roots, unless
     // the callback wants all traces (a debug feature).
-    if(nsXPConnect::IsGray(obj) || cb.WantAllTraces())
+    if(xpc_IsGrayGCThing(obj) || cb.WantAllTraces())
         cb.NoteRoot(nsIProgrammingLanguage::JAVASCRIPT, obj,
                     nsXPConnect::GetXPConnect());
 }
@@ -572,7 +571,7 @@ XPCJSRuntime::AddXPConnectRoots(JSContext* cx,
     for(XPCRootSetElem *e = mWrappedJSRoots; e ; e = e->GetNextRoot())
     {
         nsXPCWrappedJS *wrappedJS = static_cast<nsXPCWrappedJS*>(e);
-        JSObject *obj = wrappedJS->GetJSObject();
+        JSObject *obj = wrappedJS->GetJSObjectPreserveColor();
 
         // Only suspect wrappedJSObjects that are in a compartment that
         // participates in cycle collection.
@@ -640,7 +639,7 @@ static PLDHashOperator
 SweepExpandos(XPCWrappedNative *wn, JSObject *&expando, void *arg)
 {
     JSContext *cx = (JSContext *)arg;
-    return IsAboutToBeFinalized(cx, wn->GetFlatJSObjectNoMark())
+    return IsAboutToBeFinalized(cx, wn->GetFlatJSObjectPreserveColor())
            ? PL_DHASH_REMOVE
            : PL_DHASH_NEXT;
 }

js/src/xpconnect/src/xpcmaps.h

@@ -79,7 +79,7 @@ public:
     inline nsXPCWrappedJS* Add(nsXPCWrappedJS* wrapper)
     {
         NS_PRECONDITION(wrapper,"bad param");
-        JSObject* obj = wrapper->GetJSObject();
+        JSObject* obj = wrapper->GetJSObjectPreserveColor();
         Entry* entry = (Entry*)
             JS_DHashTableOperate(mTable, obj, JS_DHASH_ADD);
         if(!entry)
@@ -94,7 +94,8 @@ public:
     inline void Remove(nsXPCWrappedJS* wrapper)
     {
         NS_PRECONDITION(wrapper,"bad param");
-        JS_DHashTableOperate(mTable, wrapper->GetJSObject(), JS_DHASH_REMOVE);
+        JS_DHashTableOperate(mTable, wrapper->GetJSObjectPreserveColor(),
+                             JS_DHASH_REMOVE);
     }
 
     inline uint32 Count() {return mTable->entryCount;}

js/src/xpconnect/src/xpcprivate.h

@@ -60,7 +60,6 @@
 #include "jsdbgapi.h"
 #include "jsgc.h"
 #include "jscompartment.h"
-#include "xpcpublic.h"
 #include "nscore.h"
 #include "nsXPCOM.h"
 #include "nsAutoPtr.h"
@@ -489,9 +488,6 @@ private:
 ****************************************************************************
 ***************************************************************************/
 
-static const uint32 XPC_GC_COLOR_BLACK = 0;
-static const uint32 XPC_GC_COLOR_GRAY = 1;
-
 // We have a general rule internally that getters that return addref'd interface
 // pointer generally do so using an 'out' parm. When interface pointers are
 // returned as function call result values they are not addref'd. Exceptions
@@ -557,11 +553,6 @@ public:
 
     JSBool IsShuttingDown() const {return mShuttingDown;}
 
-    // The JS GC marks objects gray that are held alive directly or indirectly
-    // by an XPConnect root. The cycle collector explores only this subset
-    // of the JS heap.
-    static JSBool IsGray(void *thing);
-
     nsresult GetInfoForIID(const nsIID * aIID, nsIInterfaceInfo** info);
     nsresult GetInfoForName(const char * name, nsIInterfaceInfo** info);
 
@@ -2550,14 +2541,26 @@ public:
     nsISupports*
     GetIdentityObject() const {return mIdentity;}
 
+    /**
+     * This getter clears the gray bit before handing out the JSObject which
+     * means that the object is guaranteed to be kept alive past the next CC.
+     */
     JSObject*
-    GetFlatJSObjectAndMark() const
+    GetFlatJSObject() const
-        {if(mFlatJSObject && mFlatJSObject != INVALID_OBJECT)
+        {if(mFlatJSObject != INVALID_OBJECT)
-             mFlatJSObject->markIfUnmarked();
+             xpc_UnmarkGrayObject(mFlatJSObject);
          return mFlatJSObject;}
 
+    /**
+     * This getter does not change the color of the JSObject meaning that the
+     * object returned is not guaranteed to be kept alive past the next CC.
+     *
+     * This should only be called if you are certain that the return value won't
+     * be passed into a JS API function and that it won't be stored without
+     * being rooted (or otherwise signaling the stored value to the CC).
+     */
     JSObject*
-    GetFlatJSObjectNoMark() const {return mFlatJSObject;}
+    GetFlatJSObjectPreserveColor() const {return mFlatJSObject;}
 
     XPCLock*
     GetLock() const {return IsValid() && HasProto() ?
@@ -2695,7 +2698,7 @@ public:
         if(mScriptableInfo && JS_IsGCMarkingTracer(trc))
             mScriptableInfo->Mark();
         if(HasProto()) GetProto()->TraceJS(trc);
-        JSObject* wrapper = GetWrapper();
+        JSObject* wrapper = GetWrapperPreserveColor();
         if(wrapper)
             JS_CALL_OBJECT_TRACER(trc, wrapper, "XPCWrappedNative::mWrapper");
     }
@@ -2741,9 +2744,19 @@ public:
     JSBool NeedsCOW() { return !!(mWrapperWord & NEEDS_COW); }
     void SetNeedsCOW() { mWrapperWord |= NEEDS_COW; }
 
+    JSObject* GetWrapperPreserveColor() const
+        {return (JSObject*)(mWrapperWord & (size_t)~(size_t)FLAG_MASK);}
+
     JSObject* GetWrapper()
     {
-        return (JSObject *) (mWrapperWord & (size_t)~(size_t)FLAG_MASK);
+        JSObject* wrapper = GetWrapperPreserveColor();
+        if(wrapper)
+        {
+            xpc_UnmarkGrayObject(wrapper);
+            // Call this to unmark mFlatJSObject.
+            GetFlatJSObject();
+        }
+        return wrapper;
     }
     void SetWrapper(JSObject *obj)
     {
@@ -3005,7 +3018,24 @@ public:
                  nsXPCWrappedJS** wrapper);
 
     nsISomeInterface* GetXPTCStub() { return mXPTCStub; }
-    JSObject* GetJSObject() const {return mJSObj;}
+
+    /**
+     * This getter clears the gray bit before handing out the JSObject which
+     * means that the object is guaranteed to be kept alive past the next CC.
+     */
+    JSObject* GetJSObject() const {xpc_UnmarkGrayObject(mJSObj);
+                                   return mJSObj;}
+
+    /**
+     * This getter does not change the color of the JSObject meaning that the
+     * object returned is not guaranteed to be kept alive past the next CC.
+     *
+     * This should only be called if you are certain that the return value won't
+     * be passed into a JS API function and that it won't be stored without
+     * being rooted (or otherwise signaling the stored value to the CC).
+     */
+    JSObject* GetJSObjectPreserveColor() const {return mJSObj;}
+
     nsXPCWrappedJSClass*  GetClass() const {return mClass;}
     REFNSIID GetIID() const {return GetClass()->GetIID();}
     nsXPCWrappedJS* GetRootWrapper() const {return mRoot;}
@@ -4321,10 +4351,25 @@ public:
     static XPCVariant* newVariant(XPCCallContext& ccx, jsval aJSVal);
 
     /**
-     * nsIVariant exposes a GetAsJSVal() method, which also returns mJSVal.
+     * This getter clears the gray bit before handing out the jsval if the jsval
-     * But if you can, you should call this one, since it can be inlined.
+     * represents a JSObject. That means that the object is guaranteed to be
+     * kept alive past the next CC.
      */
-    jsval GetJSVal() const {return mJSVal;}
+    jsval GetJSVal() const
+        {if(!JSVAL_IS_PRIMITIVE(mJSVal))
+             xpc_UnmarkGrayObject(JSVAL_TO_OBJECT(mJSVal));
+         return mJSVal;}
+
+    /**
+     * This getter does not change the color of the jsval (if it represents a
+     * JSObject) meaning that the value returned is not guaranteed to be kept
+     * alive past the next CC.
+     *
+     * This should only be called if you are certain that the return value won't
+     * be passed into a JS API function and that it won't be stored without
+     * being rooted (or otherwise signaling the stored value to the CC).
+     */
+    jsval GetJSValPreserveColor() const {return mJSVal;}
 
     XPCVariant(XPCCallContext& ccx, jsval aJSVal);
 
@@ -4515,7 +4560,9 @@ struct CompartmentPrivate
     JSObject *LookupExpandoObject(XPCWrappedNative *wn) {
         if (!expandoMap)
             return nsnull;
-        return expandoMap->Get(wn);
+        JSObject *obj = expandoMap->Get(wn);
+        xpc_UnmarkGrayObject(obj);
+        return obj;
     }
 };
 

js/src/xpconnect/src/xpcpublic.h

@@ -50,6 +50,9 @@
 
 class nsIPrincipal;
 
+static const uint32 XPC_GC_COLOR_BLACK = 0;
+static const uint32 XPC_GC_COLOR_GRAY = 1;
+
 nsresult
 xpc_CreateGlobalObject(JSContext *cx, JSClass *clasp,
                        nsIPrincipal *principal, nsISupports *ptr,
@@ -136,4 +139,34 @@ xpc_GetCachedSlimWrapper(nsWrapperCache *cache, JSObject *scope)
     return xpc_GetCachedSlimWrapper(cache, scope, &dummy);
 }
 
+// The JS GC marks objects gray that are held alive directly or indirectly
+// by an XPConnect root. The cycle collector explores only this subset
+// of the JS heap.
+inline JSBool
+xpc_IsGrayGCThing(void *thing)
+{
+    return js_GCThingIsMarked(thing, XPC_GC_COLOR_GRAY);
+}
+
+// Implemented in nsXPConnect.cpp.
+extern void
+xpc_UnmarkGrayObjectRecursive(JSObject* obj);
+
+// Remove the gray color from the given JSObject and any other objects that can
+// be reached through it.
+inline void
+xpc_UnmarkGrayObject(JSObject *obj)
+{
+    if(obj && xpc_IsGrayGCThing(obj))
+        xpc_UnmarkGrayObjectRecursive(obj);
+}
+
+inline JSObject*
+nsWrapperCache::GetWrapper() const
+{
+  JSObject* obj = GetWrapperPreserveColor();
+  xpc_UnmarkGrayObject(obj);
+  return obj;
+}
+
 #endif

js/src/xpconnect/src/xpcquickstubs.cpp

@@ -816,8 +816,7 @@ getNativeFromWrapper(JSContext *cx,
                      jsval *vp)
 {
     return getNative(wrapper->GetIdentityObject(), wrapper->GetOffsets(),
-                     wrapper->GetFlatJSObjectAndMark(), iid, ppThis, pThisRef,
+                     wrapper->GetFlatJSObject(), iid, ppThis, pThisRef, vp);
-                     vp);
 }
 
 

js/src/xpconnect/src/xpcquickstubs.h

@@ -581,7 +581,7 @@ castNativeFromWrapper(JSContext *cx,
     if(wrapper)
     {
         native = wrapper->GetIdentityObject();
-        cur = wrapper->GetFlatJSObjectAndMark();
+        cur = wrapper->GetFlatJSObject();
     }
     else
     {

js/src/xpconnect/src/xpcvariant.cpp

@@ -81,22 +81,26 @@ XPCVariant::XPCVariant(XPCCallContext& ccx, jsval aJSVal)
 
 XPCTraceableVariant::~XPCTraceableVariant()
 {
-    NS_ASSERTION(JSVAL_IS_GCTHING(mJSVal), "Must be traceable or unlinked");
+    jsval val = GetJSValPreserveColor();
 
-    // If mJSVal is JSVAL_STRING, we don't need to clean anything up;
+    NS_ASSERTION(JSVAL_IS_GCTHING(val), "Must be traceable or unlinked");
-    // simply removing the string from the root set is good.
+
-    if(!JSVAL_IS_STRING(mJSVal))
+    // If val is JSVAL_STRING, we don't need to clean anything up; simply
+    // removing the string from the root set is good.
+    if(!JSVAL_IS_STRING(val))
         nsVariant::Cleanup(&mData);
 
-    if (!JSVAL_IS_NULL(mJSVal))
+    if (!JSVAL_IS_NULL(val))
         RemoveFromRootSet(nsXPConnect::GetRuntimeInstance()->GetMapLock());
 }
 
 void XPCTraceableVariant::TraceJS(JSTracer* trc)
 {
-    NS_ASSERTION(JSVAL_IS_TRACEABLE(mJSVal), "Must be traceable");
+    jsval val = GetJSValPreserveColor();
+
+    NS_ASSERTION(JSVAL_IS_TRACEABLE(val), "Must be traceable");
     JS_SET_TRACING_DETAILS(trc, PrintTraceName, this, 0);
-    JS_CallTracer(trc, JSVAL_TO_TRACEABLE(mJSVal), JSVAL_TRACE_KIND(mJSVal));
+    JS_CallTracer(trc, JSVAL_TO_TRACEABLE(val), JSVAL_TRACE_KIND(val));
 }
 
 #ifdef DEBUG
@@ -109,21 +113,24 @@ XPCTraceableVariant::PrintTraceName(JSTracer* trc, char *buf, size_t bufsize)
 #endif
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(XPCVariant)
-    if(JSVAL_IS_OBJECT(tmp->mJSVal))
+    jsval val = tmp->GetJSValPreserveColor();
+    if(JSVAL_IS_OBJECT(val))
         cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT,
-                           JSVAL_TO_OBJECT(tmp->mJSVal));
+                           JSVAL_TO_OBJECT(val));
 
     nsVariant::Traverse(tmp->mData, cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(XPCVariant)
-    // We're sharing mJSVal's buffer, clear the pointer to it
+    jsval val = tmp->GetJSValPreserveColor();
-    // so Cleanup() won't try to delete it
+
-    if(JSVAL_IS_STRING(tmp->mJSVal))
+    // We're sharing val's buffer, clear the pointer to it so Cleanup() won't
+    // try to delete it
+    if(JSVAL_IS_STRING(val))
         tmp->mData.u.wstr.mWStringValue = nsnull;
     nsVariant::Cleanup(&tmp->mData);
 
-    if(JSVAL_IS_TRACEABLE(tmp->mJSVal))
+    if(JSVAL_IS_TRACEABLE(val))
     {
         XPCTraceableVariant *v = static_cast<XPCTraceableVariant*>(tmp);
         v->RemoveFromRootSet(nsXPConnect::GetRuntimeInstance()->GetMapLock());
@@ -300,24 +307,25 @@ JSBool XPCVariant::InitializeData(XPCCallContext& ccx)
 {
     JS_CHECK_RECURSION(ccx.GetJSContext(), return JS_FALSE);
 
-    if(JSVAL_IS_INT(mJSVal))
+    jsval val = GetJSVal();
-        return NS_SUCCEEDED(nsVariant::SetFromInt32(&mData, 
+
-                                                    JSVAL_TO_INT(mJSVal)));
+    if(JSVAL_IS_INT(val))
-    if(JSVAL_IS_DOUBLE(mJSVal))
+        return NS_SUCCEEDED(nsVariant::SetFromInt32(&mData, JSVAL_TO_INT(val)));
+    if(JSVAL_IS_DOUBLE(val))
         return NS_SUCCEEDED(nsVariant::SetFromDouble(&mData, 
-                                                     JSVAL_TO_DOUBLE(mJSVal)));
+                                                     JSVAL_TO_DOUBLE(val)));
-    if(JSVAL_IS_BOOLEAN(mJSVal))
+    if(JSVAL_IS_BOOLEAN(val))
         return NS_SUCCEEDED(nsVariant::SetFromBool(&mData, 
-                                                   JSVAL_TO_BOOLEAN(mJSVal)));
+                                                   JSVAL_TO_BOOLEAN(val)));
-    if(JSVAL_IS_VOID(mJSVal))
+    if(JSVAL_IS_VOID(val))
         return NS_SUCCEEDED(nsVariant::SetToVoid(&mData));
-    if(JSVAL_IS_NULL(mJSVal))
+    if(JSVAL_IS_NULL(val))
         return NS_SUCCEEDED(nsVariant::SetToEmpty(&mData));
-    if(JSVAL_IS_STRING(mJSVal))
+    if(JSVAL_IS_STRING(val))
     {
         // Make our string immutable.  This will also ensure null-termination,
         // which nsVariant assumes for its PRUnichar* stuff.
-        JSString* str = JSVAL_TO_STRING(mJSVal);
+        JSString* str = JSVAL_TO_STRING(val);
         if(!JS_MakeStringImmutable(ccx, str))
             return JS_FALSE;
 
@@ -344,9 +352,9 @@ JSBool XPCVariant::InitializeData(XPCCallContext& ccx)
     }
 
     // leaving only JSObject...
-    NS_ASSERTION(JSVAL_IS_OBJECT(mJSVal), "invalid type of jsval!");
+    NS_ASSERTION(JSVAL_IS_OBJECT(val), "invalid type of jsval!");
     
-    JSObject* jsobj = JSVAL_TO_OBJECT(mJSVal);
+    JSObject* jsobj = JSVAL_TO_OBJECT(val);
 
     // Let's see if it is a xpcJSID.
 
@@ -374,7 +382,7 @@ JSBool XPCVariant::InitializeData(XPCCallContext& ccx)
             return JS_FALSE; 
 
         if(!XPCConvert::JSArray2Native(ccx, &mData.u.array.mArrayValue, 
-                                       mJSVal, len, len,
+                                       val, len, len,
                                        type, type.IsPointer(),
                                        &id, nsnull))
             return JS_FALSE;

js/src/xpconnect/src/xpcwrappedjs.cpp

@@ -81,7 +81,7 @@ NS_CYCLE_COLLECTION_CLASSNAME(nsXPCWrappedJS)::Traverse
         // nsXPCWrappedJS roots its mJSObj when its refcount is > 1, see
         // the comment above nsXPCWrappedJS::AddRef.
         cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT,
-                           tmp->GetJSObject());
+                           tmp->GetJSObjectPreserveColor());
 
     nsXPCWrappedJS* root = tmp->GetRootWrapper();
     if(root == tmp)
@@ -262,7 +262,7 @@ nsXPCWrappedJS::TraceJS(JSTracer* trc)
 {
     NS_ASSERTION(mRefCnt >= 2 && IsValid(), "must be strongly referenced");
     JS_SET_TRACING_DETAILS(trc, PrintTraceName, this, 0);
-    JS_CallTracer(trc, mJSObj, JSTRACE_OBJECT);
+    JS_CallTracer(trc, GetJSObjectPreserveColor(), JSTRACE_OBJECT);
 }
 
 #ifdef DEBUG
@@ -290,9 +290,9 @@ NS_IMETHODIMP
 nsXPCWrappedJS::GetJSObject(JSObject** aJSObj)
 {
     NS_PRECONDITION(aJSObj, "bad param");
-    NS_PRECONDITION(mJSObj, "bad wrapper");
+    NS_PRECONDITION(IsValid(), "bad wrapper");
 
-    if(!(*aJSObj = mJSObj))
+    if(!(*aJSObj = GetJSObject()))
         return NS_ERROR_OUT_OF_MEMORY;
     return NS_OK;
 }
@@ -627,7 +627,8 @@ nsXPCWrappedJS::GetEnumerator(nsISimpleEnumerator * *aEnumerate)
     if(!ccx.IsValid())
         return NS_ERROR_UNEXPECTED;
 
-    return nsXPCWrappedJSClass::BuildPropertyEnumerator(ccx, mJSObj, aEnumerate);
+    return nsXPCWrappedJSClass::BuildPropertyEnumerator(ccx, GetJSObject(),
+                                                        aEnumerate);
 }
 
 /* nsIVariant getProperty (in AString name); */
@@ -646,7 +647,7 @@ nsXPCWrappedJS::GetProperty(const nsAString & name, nsIVariant **_retval)
         buf->AddRef();
 
     return nsXPCWrappedJSClass::
-        GetNamedPropertyAsVariant(ccx, mJSObj, jsstr, _retval);
+        GetNamedPropertyAsVariant(ccx, GetJSObject(), jsstr, _retval);
 }
 
 /***************************************************************************/

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -122,10 +122,8 @@ NS_CYCLE_COLLECTION_CLASSNAME(XPCWrappedNative)::Traverse(void *p,
         // for XPCWrappedNatives, described in a larger comment below and also
         // on our wiki at http://wiki.mozilla.org/XPConnect_object_wrapping 
 
-        JSObject *obj = nsnull;
+        JSObject *obj = tmp->GetFlatJSObjectPreserveColor();
-        nsresult rv = tmp->GetJSObject(&obj);
+        cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT, obj);
-        if(NS_SUCCEEDED(rv))
-            cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT, obj);
     }
 
     XPCJSRuntime *rt = tmp->GetRuntime();
@@ -334,7 +332,7 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx,
 {
     nsWrapperCache *cache = helper.GetWrapperCache();
 
-    NS_ASSERTION(!cache || !cache->GetWrapper(),
+    NS_ASSERTION(!cache || !cache->GetWrapperPreserveColor(),
                  "We assume the caller already checked if it could get the "
                  "wrapper from the cache.");
 
@@ -397,7 +395,7 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx,
         }
     }
 #ifdef DEBUG
-    else if(!cache->GetWrapper())
+    else if(!cache->GetWrapperPreserveColor())
     {   // scoped lock
         XPCAutoLock lock(mapLock);
         NS_ASSERTION(!map->Find(identity),
@@ -675,13 +673,13 @@ FinishCreate(XPCCallContext& ccx,
     }
     else if(wrapper)
     {
-        JSObject *flat = wrapper->GetFlatJSObjectAndMark();
+        JSObject *flat = wrapper->GetFlatJSObject();
-        NS_ASSERTION(!cache || !cache->GetWrapper() ||
+        NS_ASSERTION(!cache || !cache->GetWrapperPreserveColor() ||
-                     flat == cache->GetWrapper(),
+                     flat == cache->GetWrapperPreserveColor(),
                      "This object has a cached wrapper that's different from "
                      "the JSObject held by its native wrapper?");
 
-        if(cache && !cache->GetWrapper())
+        if(cache && !cache->GetWrapperPreserveColor())
             cache->SetWrapper(flat);
 
         // Our newly created wrapper is the one that we just added to the table.
@@ -1511,7 +1509,7 @@ XPCWrappedNative::ReparentWrapperIfFound(XPCCallContext& ccx,
         if(NS_FAILED(rv))
             return rv;
 
-        flat = wrapper->GetFlatJSObjectAndMark();
+        flat = wrapper->GetFlatJSObject();
     }
 
     if(!flat)
@@ -3106,7 +3104,7 @@ CallMethodHelper::Invoke()
 /* readonly attribute JSObjectPtr JSObject; */
 NS_IMETHODIMP XPCWrappedNative::GetJSObject(JSObject * *aJSObject)
 {
-    *aJSObject = GetFlatJSObjectAndMark();
+    *aJSObject = GetFlatJSObject();
     return NS_OK;
 }
 
@@ -3124,7 +3122,7 @@ NS_IMETHODIMP XPCWrappedNative::GetNative(nsISupports * *aNative)
 NS_IMETHODIMP XPCWrappedNative::GetJSObjectPrototype(JSObject * *aJSObjectPrototype)
 {
     *aJSObjectPrototype = HasProto() ?
-                GetProto()->GetJSProtoObject() : GetFlatJSObjectAndMark();
+                GetProto()->GetJSProtoObject() : GetFlatJSObject();
     return NS_OK;
 }
 
@@ -3208,7 +3206,7 @@ NS_IMETHODIMP XPCWrappedNative::RefreshPrototype()
         return UnexpectedFailure(NS_ERROR_FAILURE);
 
     JSAutoEnterCompartment ac;
-    if(!ac.enter(ccx, GetFlatJSObjectAndMark()))
+    if(!ac.enter(ccx, GetFlatJSObject()))
         return UnexpectedFailure(NS_ERROR_FAILURE);
 
     AutoMarkingWrappedNativeProtoPtr oldProto(ccx);
@@ -3232,7 +3230,7 @@ NS_IMETHODIMP XPCWrappedNative::RefreshPrototype()
     if(newProto.get() == oldProto.get())
         return NS_OK;
 
-    if(!JS_SetPrototype(ccx, GetFlatJSObjectAndMark(),
+    if(!JS_SetPrototype(ccx, GetFlatJSObject(),
                         newProto->GetJSProtoObject()))
         return UnexpectedFailure(NS_ERROR_FAILURE);
 

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -363,7 +363,7 @@ WrappedNativeJSGCThingTracer(JSDHashTable *table, JSDHashEntryHdr *hdr,
     if(wrapper->HasExternalReference() && !wrapper->IsWrapperExpired())
     {
         JSTracer* trc = (JSTracer *)arg;
-        JS_CALL_OBJECT_TRACER(trc, wrapper->GetFlatJSObjectNoMark(),
+        JS_CALL_OBJECT_TRACER(trc, wrapper->GetFlatJSObjectPreserveColor(),
                               "XPCWrappedNative::mFlatJSObject");
     }
 

modules/plugin/base/src/nsJSNPRuntime.cpp

@@ -56,6 +56,7 @@
 #include "nsIDOMElement.h"
 #include "prmem.h"
 #include "nsIContent.h"
+#include "xpcpublic.h"
 
 using namespace mozilla::plugins::parent;