Bug 580128 - Fix a leak caused by holding a non-cycle collectable object instead of a cycle-collectable one. r=peterv

dom/base/nsDOMClassInfo.cpp

@@ -1581,7 +1581,6 @@ jsid nsDOMClassInfo::sNamedItem_id       = JSID_VOID;
 jsid nsDOMClassInfo::sEnumerate_id       = JSID_VOID;
 jsid nsDOMClassInfo::sNavigator_id       = JSID_VOID;
 jsid nsDOMClassInfo::sDocument_id        = JSID_VOID;
-jsid nsDOMClassInfo::sWindow_id          = JSID_VOID;
 jsid nsDOMClassInfo::sFrames_id          = JSID_VOID;
 jsid nsDOMClassInfo::sSelf_id            = JSID_VOID;
 jsid nsDOMClassInfo::sOpener_id          = JSID_VOID;
@@ -1806,7 +1805,6 @@ nsDOMClassInfo::DefineStaticJSVals(JSContext *cx)
   SET_JSID_TO_STRING(sEnumerate_id,       cx, "enumerateProperties");
   SET_JSID_TO_STRING(sNavigator_id,       cx, "navigator");
   SET_JSID_TO_STRING(sDocument_id,        cx, "document");
-  SET_JSID_TO_STRING(sWindow_id,          cx, "window");
   SET_JSID_TO_STRING(sFrames_id,          cx, "frames");
   SET_JSID_TO_STRING(sSelf_id,            cx, "self");
   SET_JSID_TO_STRING(sOpener_id,          cx, "opener");
@@ -4843,7 +4841,6 @@ nsDOMClassInfo::ShutDown()
   sEnumerate_id       = JSID_VOID;
   sNavigator_id       = JSID_VOID;
   sDocument_id        = JSID_VOID;
-  sWindow_id          = JSID_VOID;
   sFrames_id          = JSID_VOID;
   sSelf_id            = JSID_VOID;
   sOpener_id          = JSID_VOID;
@@ -6799,29 +6796,6 @@ nsWindowSH::NewResolve(nsIXPConnectWrappedNative *wrapper, JSContext *cx,
       return NS_OK;
     }
 
-    if (id == sWindow_id) {
-      // window should *always* be the outer window object.
-      win = win->GetOuterWindowInternal();
-      NS_ENSURE_TRUE(win, NS_ERROR_NOT_AVAILABLE);
-
-      jsval winVal;
-      nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-      rv = WrapNative(cx, obj, nsGlobalWindow::ToSupports(win), PR_TRUE,
-                      &winVal, getter_AddRefs(holder));
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      PRBool ok =
-        ::JS_DefinePropertyById(cx, obj, id, winVal, JS_PropertyStub, JS_PropertyStub,
-                                JSPROP_READONLY | JSPROP_ENUMERATE);
-
-      if (!ok) {
-        return NS_ERROR_FAILURE;
-      }
-      *objp = obj;
-
-      return NS_OK;
-    }
-
     if (id == sJava_id || id == sPackages_id) {
       static PRBool isResolvingJavaProperties;
 

dom/base/nsDOMClassInfo.h

@@ -345,7 +345,6 @@ protected:
   static jsid sEnumerate_id;
   static jsid sNavigator_id;
   static jsid sDocument_id;
-  static jsid sWindow_id;
   static jsid sFrames_id;
   static jsid sSelf_id;
   static jsid sOpener_id;

dom/base/nsGlobalWindow.cpp

@@ -1956,6 +1956,14 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
         NS_ERROR("can't set prototype");
         return NS_ERROR_FAILURE;
       }
+    } else {
+      if (!JS_DefineProperty(cx, newInnerWindow->mJSObject, "window",
+                             OBJECT_TO_JSVAL(mJSObject),
+                             JS_PropertyStub, JS_PropertyStub,
+                             JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT)) {
+        NS_ERROR("can't create the 'window' property");
+        return NS_ERROR_FAILURE;
+      }
     }
   }
 

dom/base/nsJSEnvironment.cpp

@@ -1314,7 +1314,7 @@ nsJSContext::~nsJSContext()
 #endif
   NS_PRECONDITION(!mTerminations, "Shouldn't have termination funcs by now");
 
-  mGlobalWrapperRef = nsnull;
+  mGlobalObjectRef = nsnull;
 
   DestroyJSContext();
 
@@ -1372,11 +1372,11 @@ NS_IMPL_CYCLE_COLLECTION_ROOT_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(nsJSContext)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(nsJSContext)
-  NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mGlobalWrapperRef)
+  NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mGlobalObjectRef)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INTERNAL(nsJSContext)
   NS_IMPL_CYCLE_COLLECTION_DESCRIBE(nsJSContext, tmp->GetCCRefcnt())
-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mGlobalWrapperRef)
+  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mGlobalObjectRef)
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mContext");
   nsContentUtils::XPConnect()->NoteJSContext(tmp->mContext, cb);
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
@@ -2553,6 +2553,8 @@ nsresult
 nsJSContext::CreateOuterObject(nsIScriptGlobalObject *aGlobalObject,
                                nsIScriptGlobalObject *aCurrentInner)
 {
+  mGlobalObjectRef = aGlobalObject;
+
   nsCOMPtr<nsIDOMChromeWindow> chromeWindow(do_QueryInterface(aGlobalObject));
   PRUint32 flags = 0;
 
@@ -2583,19 +2585,8 @@ nsJSContext::SetOuterObject(void *aOuterObject)
 {
   JSObject *outer = static_cast<JSObject *>(aOuterObject);
 
-  nsIXPConnect *xpc = nsContentUtils::XPConnect();
-  nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
-
-  nsresult rv = xpc->HoldObject(mContext, outer, getter_AddRefs(holder));
-  NS_ENSURE_SUCCESS(rv, rv);
-
   // Force our context's global object to be the outer.
   JS_SetGlobalObject(mContext, outer);
-
-  // Hold a strong reference to the wrapper for the global to avoid
-  // rooting and unrooting the global object every time its AddRef()
-  // or Release() methods are called
-  mGlobalWrapperRef = holder;
   return NS_OK;
 }
 

dom/base/nsJSEnvironment.h

@@ -312,12 +312,9 @@ private:
   PRTime mModalStateTime;
   PRUint32 mModalStateDepth;
 
-  // mGlobalWrapperRef is used only to hold a strong reference to the
-  // global object wrapper while the nsJSContext is alive. This cuts
-  // down on the number of rooting and unrooting calls XPConnect has
-  // to make when the global object is touched in JS.
-
-  nsCOMPtr<nsISupports> mGlobalWrapperRef;
+  // mGlobalObjectRef ensures that the outer window stays alive as long as the
+  // context does. It is eventually collected by the cycle collector.
+  nsCOMPtr<nsISupports> mGlobalObjectRef;
 
   static int JSOptionChangedCallback(const char *pref, void *data);