bug 458099 - DOMOperationCallback can get a JSContext with no global object. r=peterv sr=mrbkap

js/src/jsapi.h

@@ -595,6 +595,9 @@ JS_StringToVersion(const char *string);
                                                    that a null rval out-param
                                                    will be passed to each call
                                                    to JS_ExecuteScript. */
+#define JSOPTION_UNROOTED_GLOBAL JS_BIT(13)     /* The GC will not root the
+                                                   global objects leaving
+                                                   that up to the embedding. */
 
 extern JS_PUBLIC_API(uint32)
 JS_GetOptions(JSContext *cx);

js/src/jsgc.cpp

@@ -2950,7 +2950,7 @@ js_TraceContext(JSTracer *trc, JSContext *acx)
     }
 
     /* Mark other roots-by-definition in acx. */
-    if (acx->globalObject)
+    if (acx->globalObject && !JS_HAS_OPTION(acx, JSOPTION_UNROOTED_GLOBAL))
         JS_CALL_OBJECT_TRACER(trc, acx->globalObject, "global object");
     TraceWeakRoots(trc, &acx->weakRoots);
     if (acx->throwing) {

js/src/xpconnect/src/nsXPConnect.cpp

@@ -373,8 +373,13 @@ static JSBool
 XPCCycleCollectGCCallback(JSContext *cx, JSGCStatus status)
 {
     // Launch the cycle collector.
-    if(status == JSGC_MARK_END)
+    switch(status)
     {
+      case JSGC_BEGIN:
+        nsXPConnect::GetRuntimeInstance()->UnrootContextGlobals();
+        break;
+
+      case JSGC_MARK_END:
         // This is the hook between marking and sweeping in the JS GC. Do cycle
         // collection.
         if(!gDidCollection)
@@ -388,24 +393,22 @@ XPCCycleCollectGCCallback(JSContext *cx, JSGCStatus status)
         // Mark JS objects that are held by XPCOM objects that are in cycles
         // that will not be collected.
         nsXPConnect::GetRuntimeInstance()->
-            TraceXPConnectRoots(cx->runtime->gcMarkingTracer);
+            TraceXPConnectRoots(cx->runtime->gcMarkingTracer, JS_TRUE);
-    }
+        break;
-    else if(status == JSGC_END)
+
-    {
+      case JSGC_END:
         if(gInCollection)
         {
             gInCollection = PR_FALSE;
             gCollected = nsCycleCollector_finishCollection();
         }
-        nsXPConnect::GetRuntimeInstance()->RestoreContextGlobals();
+        break;
+
+      default:
+        break;
     }
 
-    PRBool ok = gOldJSGCCallback ? gOldJSGCCallback(cx, status) : JS_TRUE;
+    return gOldJSGCCallback ? gOldJSGCCallback(cx, status) : JS_TRUE;
-
-    if(status == JSGC_BEGIN)
-        nsXPConnect::GetRuntimeInstance()->UnsetContextGlobals();
-
-    return ok;
 }
 
 PRBool
@@ -960,13 +963,8 @@ public:
 #else
         cb.DescribeNode(RefCounted, refCount);
 #endif
-
+        cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT,
-        void* globalObject = (cx->globalObject)
+                           cx->globalObject);
-                             ? cx->globalObject
-                             : nsXPConnect::GetRuntimeInstance()->
-                                 GetUnsetContextGlobal(cx);
-
-        cb.NoteScriptChild(nsIProgrammingLanguage::JAVASCRIPT, globalObject);
 
         return NS_OK;
     }

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -346,25 +346,32 @@ struct ClearedGlobalObject : public JSDHashEntryHdr
     JSObject* mGlobalObject;
 };
 
-void XPCJSRuntime::TraceXPConnectRoots(JSTracer *trc)
+void XPCJSRuntime::TraceXPConnectRoots(JSTracer *trc, JSBool rootGlobals)
 {
-    if(mClearedGlobalObjects.ops)
+    if(mUnrootedGlobalCount != 0)
     {
         JSContext *iter = nsnull, *acx;
         while((acx = JS_ContextIterator(GetJSRuntime(), &iter)))
         {
-            JSDHashEntryHdr* entry =
+            if(JS_HAS_OPTION(acx, JSOPTION_UNROOTED_GLOBAL))
-                JS_DHashTableOperate(&mClearedGlobalObjects, acx,
-                                     JS_DHASH_LOOKUP);
-            if(JS_DHASH_ENTRY_IS_BUSY(entry))
             {
-                ClearedGlobalObject* clearedGlobal =
+                NS_ASSERTION(nsXPConnect::GetXPConnect()->GetRequestDepth(acx)
-                    reinterpret_cast<ClearedGlobalObject*>(entry);
+                             == 0, "active cx must be always rooted");
-                JS_CALL_OBJECT_TRACER(trc, clearedGlobal->mGlobalObject,
+                NS_ASSERTION(acx->globalObject, "bad state");
+                JS_CALL_OBJECT_TRACER(trc, acx->globalObject,
                                       "global object");
+                if(rootGlobals)
+                {
+                    NS_ASSERTION(mUnrootedGlobalCount != 0, "bad state");
+                    NS_ASSERTION(trc == acx->runtime->gcMarkingTracer,
+                                 "bad tracer");
+                    JS_ToggleOptions(acx, JSOPTION_UNROOTED_GLOBAL);
+                    --mUnrootedGlobalCount;
                 }
             }
         }
+        NS_ASSERTION(!rootGlobals || mUnrootedGlobalCount == 0, "bad state");
+    }
 
     XPCWrappedNativeScope::TraceJS(trc, this);
 
@@ -433,77 +440,24 @@ void XPCJSRuntime::AddXPConnectRoots(JSContext* cx,
         JS_DHashTableEnumerate(&mJSHolders, NoteJSHolder, &cb);
 }
 
-void XPCJSRuntime::UnsetContextGlobals()
+void XPCJSRuntime::UnrootContextGlobals()
 {
-    if(!mClearedGlobalObjects.ops)
+    mUnrootedGlobalCount = 0;
-        return;
-
-    RestoreContextGlobals();
-
     JSContext *iter = nsnull, *acx;
     while((acx = JS_ContextIterator(GetJSRuntime(), &iter)))
     {
+        NS_ASSERTION(!JS_HAS_OPTION(acx, JSOPTION_UNROOTED_GLOBAL),
+                     "unrooted global should be set only during CC");
         if(nsXPConnect::GetXPConnect()->GetRequestDepth(acx) == 0)
         {
             JS_ClearNewbornRoots(acx);
             if(acx->globalObject)
             {
-                JSDHashEntryHdr* entry =
+                JS_ToggleOptions(acx, JSOPTION_UNROOTED_GLOBAL);
-                    JS_DHashTableOperate(&mClearedGlobalObjects, acx,
+                ++mUnrootedGlobalCount;
-                                         JS_DHASH_ADD);
-                ClearedGlobalObject* clearedGlobal =
-                    reinterpret_cast<ClearedGlobalObject*>(entry);
-                if(clearedGlobal)
-                {
-                    clearedGlobal->mContext = acx;
-                    clearedGlobal->mGlobalObject = acx->globalObject;
-                    acx->globalObject = nsnull;
             }
         }
     }
-    }
-}
-
-JSDHashOperator
-RemoveContextGlobal(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 number,
-                    void *arg)
-{
-  return JS_DHASH_REMOVE;
-}
-
-void XPCJSRuntime::RestoreContextGlobals()
-{
-    if(!mClearedGlobalObjects.ops || mClearedGlobalObjects.entryCount == 0)
-        return;
-
-    JSContext *iter = nsnull, *acx;
-    while((acx = JS_ContextIterator(GetJSRuntime(), &iter)))
-    {
-        JSDHashEntryHdr* entry =
-            JS_DHashTableOperate(&mClearedGlobalObjects, acx, JS_DHASH_LOOKUP);
-        if(JS_DHASH_ENTRY_IS_BUSY(entry))
-        {
-            ClearedGlobalObject* clearedGlobal =
-                reinterpret_cast<ClearedGlobalObject*>(entry);
-            acx->globalObject = clearedGlobal->mGlobalObject;
-        }
-    }
-    JS_DHashTableEnumerate(&mClearedGlobalObjects, RemoveContextGlobal, nsnull);
-}
-
-JSObject* XPCJSRuntime::GetUnsetContextGlobal(JSContext* cx)
-{
-    if(!mClearedGlobalObjects.ops)
-        return nsnull;
-
-    JSDHashEntryHdr* entry =
-        JS_DHashTableOperate(&mClearedGlobalObjects, cx, JS_DHASH_LOOKUP);
-    ClearedGlobalObject* clearedGlobal =
-        reinterpret_cast<ClearedGlobalObject*>(entry);
-
-    return JS_DHASH_ENTRY_IS_BUSY(entry) ?
-           clearedGlobal->mGlobalObject :
-           nsnull;
 }
 
 // static
@@ -1005,11 +959,6 @@ XPCJSRuntime::~XPCJSRuntime()
         JS_DHashTableFinish(&mJSHolders);
         mJSHolders.ops = nsnull;
     }
-    if(mClearedGlobalObjects.ops)
-    {
-        JS_DHashTableFinish(&mClearedGlobalObjects);
-        mClearedGlobalObjects.ops = nsnull;
-    }
 
     if(mJSRuntime)
     {
@@ -1043,7 +992,8 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect)
    mDoingFinalization(JS_FALSE),
    mVariantRoots(nsnull),
    mWrappedJSRoots(nsnull),
-   mObjectHolderRoots(nsnull)
+   mObjectHolderRoots(nsnull),
+   mUnrootedGlobalCount(0)
 {
 #ifdef XPC_CHECK_WRAPPERS_AT_SHUTDOWN
     DEBUG_WrappedNativeHashtable =
@@ -1089,9 +1039,6 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect)
     if(!JS_DHashTableInit(&mJSHolders, JS_DHashGetStubOps(), nsnull,
                           sizeof(ObjectHolder), 512))
         mJSHolders.ops = nsnull;
-    if(!JS_DHashTableInit(&mClearedGlobalObjects, JS_DHashGetStubOps(), nsnull,
-                          sizeof(ClearedGlobalObject), JS_DHASH_MIN_SIZE))
-        mClearedGlobalObjects.ops = nsnull;
 
     // Install a JavaScript 'debugger' keyword handler in debug builds only
 #ifdef DEBUG

js/src/xpconnect/src/xpcprivate.h

@@ -713,7 +713,7 @@ public:
     }
 
     static void TraceJS(JSTracer* trc, void* data);
-    void TraceXPConnectRoots(JSTracer *trc);
+    void TraceXPConnectRoots(JSTracer *trc, JSBool rootGlobals = JS_FALSE);
     void AddXPConnectRoots(JSContext* cx,
                            nsCycleCollectionTraversalCallback& cb);
 
@@ -726,9 +726,7 @@ public:
     nsresult AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer);
     nsresult RemoveJSHolder(void* aHolder);
 
-    void UnsetContextGlobals();
+    void UnrootContextGlobals();
-    void RestoreContextGlobals();
-    JSObject* GetUnsetContextGlobal(JSContext* cx);
 
     void DebugDump(PRInt16 depth);
 
@@ -785,7 +783,7 @@ private:
     XPCRootSetElem *mWrappedJSRoots;
     XPCRootSetElem *mObjectHolderRoots;
     JSDHashTable mJSHolders;
-    JSDHashTable mClearedGlobalObjects;
+    uintN mUnrootedGlobalCount;
 };
 
 /***************************************************************************/