Bug 490718 - 'XPCWrappedNativeScope creates a needless cycle with its principal provider.' r=peterv, sr=jst.
authorBen Turner <bent.mozilla@gmail.com>
Tue, 19 May 2009 10:56:01 -0700
changeset 28592 6dd95b1cba8eae736b5eb9368090c36e5d1b29bf
parent 28591 998d6f6e95b7a463cf16c43059f286125ee17fbb
child 28593 9e97a6eb4eadb430a6da9b6500d680070f36ae0e
push id7131
push userbturner@mozilla.com
push dateTue, 19 May 2009 17:56:30 +0000
treeherdermozilla-central@6dd95b1cba8e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, jst
bugs490718
milestone1.9.2a1pre
Bug 490718 - 'XPCWrappedNativeScope creates a needless cycle with its principal provider.' r=peterv, sr=jst.
js/src/xpconnect/src/nsXPConnect.cpp
js/src/xpconnect/src/xpcprivate.h
js/src/xpconnect/src/xpcwrappednativescope.cpp
--- a/js/src/xpconnect/src/nsXPConnect.cpp
+++ b/js/src/xpconnect/src/nsXPConnect.cpp
@@ -566,53 +566,32 @@ nsXPConnect::BeginCycleCollection(nsCycl
 #else
     NS_ASSERTION(mCycleCollectionContext,
                  "Didn't call nsXPConnect::Collect()?");
 #endif
 
     GetRuntime()->AddXPConnectRoots(mCycleCollectionContext->GetJSContext(),
                                     cb);
 
-#ifndef XPCONNECT_STANDALONE
-    if(!mScopes.IsInitialized())
-    {
-        mScopes.Init();
-    }
-    NS_ASSERTION(mScopes.Count() == 0, "Didn't clear mScopes?");
-    XPCWrappedNativeScope::TraverseScopes(*mCycleCollectionContext);
-#endif
-
     return NS_OK;
 }
 
-#ifndef XPCONNECT_STANDALONE
-void
-nsXPConnect::RecordTraversal(void *p, nsISupports *s)
-{
-    mScopes.Put(p, s);
-}
-#endif
-
 nsresult 
 nsXPConnect::FinishCycleCollection()
 {
 #ifdef DEBUG_CC
     if(mExplainCycleCollectionContext)
     {
         mCycleCollectionContext = nsnull;
         mExplainCycleCollectionContext = nsnull;
 
         GetRuntime()->RootContextGlobals();
     }
 #endif
 
-#ifndef XPCONNECT_STANDALONE
-    mScopes.Clear();
-#endif
-
 #ifdef DEBUG_CC
     if(mJSRoots.ops)
     {
         PL_DHashTableFinish(&mJSRoots);
         mJSRoots.ops = nsnull;
     }
 #endif
 
@@ -942,26 +921,16 @@ nsXPConnect::Traverse(void *p, nsCycleCo
     else if(clazz->flags & JSCLASS_HAS_PRIVATE &&
             clazz->flags & JSCLASS_PRIVATE_IS_NSISUPPORTS &&
             !XPCNativeWrapper::IsNativeWrapperClass(clazz))
     {
         NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "xpc_GetJSPrivate(obj)");
         cb.NoteXPCOMChild(static_cast<nsISupports*>(xpc_GetJSPrivate(obj)));
     }
 
-#ifndef XPCONNECT_STANDALONE
-    if(clazz->flags & JSCLASS_IS_GLOBAL)
-    {
-        nsISupports *principal = nsnull;
-        mScopes.Get(obj, &principal);
-        NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "scope principal");
-        cb.NoteXPCOMChild(principal);
-    }
-#endif
-
     return NS_OK;
 }
 
 PRInt32
 nsXPConnect::GetRequestDepth(JSContext* cx)
 {
     PRInt32 requestDepth = cx->outstandingRequests;
     XPCCallContext* context = GetCycleCollectionContext();
--- a/js/src/xpconnect/src/xpcprivate.h
+++ b/js/src/xpconnect/src/xpcprivate.h
@@ -1320,23 +1320,16 @@ public:
     void SetComponents(nsXPCComponents* aComponents);
     void SetGlobal(XPCCallContext& ccx, JSObject* aGlobal);
 
     static void InitStatics() { gScopes = nsnull; gDyingScopes = nsnull; }
 
     XPCContext *GetContext() { return mContext; }
     void SetContext(XPCContext *xpcc) { mContext = nsnull; }
 
-#ifndef XPCONNECT_STANDALONE
-    /**
-     * Fills the hash mapping global object to principal.
-     */
-    static void TraverseScopes(XPCCallContext& ccx);
-#endif
-
 protected:
     XPCWrappedNativeScope(XPCCallContext& ccx, JSObject* aGlobal);
     virtual ~XPCWrappedNativeScope();
 
     static void KillDyingScopes();
 
     XPCWrappedNativeScope(); // not implemented
 
@@ -1366,17 +1359,17 @@ private:
     XPCContext*                      mContext;
 
 #ifndef XPCONNECT_STANDALONE
     // The script object principal instance corresponding to our current global
     // JS object.
     // XXXbz what happens if someone calls JS_SetPrivate on mGlobalJSObject.
     // How do we deal?  Do we need to?  I suspect this isn't worth worrying
     // about, since all of our scope objects are verified as not doing that.
-    nsCOMPtr<nsIScriptObjectPrincipal> mScriptObjectPrincipal;
+    nsIScriptObjectPrincipal* mScriptObjectPrincipal;
 #endif
 };
 
 JSObject* xpc_CloneJSFunction(XPCCallContext &ccx, JSObject *funobj,
                               JSObject *parent);
 
 /***************************************************************************/
 // XPCNativeMember represents a single idl declared method, attribute or
--- a/js/src/xpconnect/src/xpcwrappednativescope.cpp
+++ b/js/src/xpconnect/src/xpcwrappednativescope.cpp
@@ -140,16 +140,19 @@ XPCWrappedNativeScope::XPCWrappedNativeS
         mWrappedNativeProtoMap(ClassInfo2WrappedNativeProtoMap::newMap(XPC_NATIVE_PROTO_MAP_SIZE)),
         mWrapperMap(WrappedNative2WrapperMap::newMap(XPC_WRAPPER_MAP_SIZE)),
         mComponents(nsnull),
         mNext(nsnull),
         mGlobalJSObject(nsnull),
         mPrototypeJSObject(nsnull),
         mPrototypeJSFunction(nsnull),
         mPrototypeNoHelper(nsnull)
+#ifndef XPCONNECT_STANDALONE
+        , mScriptObjectPrincipal(nsnull)
+#endif
 {
     // add ourselves to the scopes list
     {   // scoped lock
         XPCAutoLock lock(mRuntime->GetMapLock());
 
 #ifdef DEBUG
         for(XPCWrappedNativeScope* cur = gScopes; cur; cur = cur->mNext)
             NS_ASSERTION(aGlobal != cur->GetGlobalJSObject(), "dup object");
@@ -240,24 +243,26 @@ XPCWrappedNativeScope::SetGlobal(XPCCall
     if(!(~jsClass->flags & (JSCLASS_HAS_PRIVATE |
                             JSCLASS_PRIVATE_IS_NSISUPPORTS)))
     {
         // Our global has an nsISupports native pointer.  Let's
         // see whether it's what we want.
         nsISupports* priv = (nsISupports*)xpc_GetJSPrivate(aGlobal);
         nsCOMPtr<nsIXPConnectWrappedNative> native =
             do_QueryInterface(priv);
+        nsCOMPtr<nsIScriptObjectPrincipal> sop;
         if(native)
         {
-            mScriptObjectPrincipal = do_QueryWrappedNative(native);
+            sop = do_QueryWrappedNative(native);
         }
-        if(!mScriptObjectPrincipal)
+        if(!sop)
         {
-            mScriptObjectPrincipal = do_QueryInterface(priv);
+            sop = do_QueryInterface(priv);
         }
+        mScriptObjectPrincipal = sop;
     }
 #endif
 
     // Lookup 'globalObject.Object.prototype' for our wrapper's proto
     {
         AutoJSErrorAndExceptionEater eater(ccx); // scoped error eater
 
         jsval val;
@@ -447,17 +452,19 @@ XPCWrappedNativeScope::FinishedMarkPhase
 
     while(cur)
     {
         XPCWrappedNativeScope* next = cur->mNext;
         if(cur->mGlobalJSObject &&
            JS_IsAboutToBeFinalized(cx, cur->mGlobalJSObject))
         {
             cur->mGlobalJSObject = nsnull;
-
+#ifndef XPCONNECT_STANDALONE
+            cur->mScriptObjectPrincipal = nsnull;
+#endif
             // Move this scope from the live list to the dying list.
             if(prev)
                 prev->mNext = next;
             else
                 gScopes = next;
             cur->mNext = gDyingScopes;
             gDyingScopes = cur;
             cur = nsnull;
@@ -967,25 +974,8 @@ XPCWrappedNativeScope::DebugDump(PRInt16
         {
             XPC_LOG_INDENT();
             mWrappedNativeProtoMap->Enumerate(WrappedNativeProtoMapDumpEnumerator, &depth);
             XPC_LOG_OUTDENT();
         }
     XPC_LOG_OUTDENT();
 #endif
 }
-
-#ifndef XPCONNECT_STANDALONE
-// static
-void
-XPCWrappedNativeScope::TraverseScopes(XPCCallContext& ccx)
-{
-    // Hold the lock throughout.
-    XPCAutoLock lock(ccx.GetRuntime()->GetMapLock());
-
-    for(XPCWrappedNativeScope* cur = gScopes; cur; cur = cur->mNext)
-        if(cur->mGlobalJSObject && cur->mScriptObjectPrincipal)
-        {
-            ccx.GetXPConnect()->RecordTraversal(cur->mGlobalJSObject,
-                                                cur->mScriptObjectPrincipal);
-        }
-}
-#endif