Call JS_ClearScope up the global object's prototype chain to fix leaks. Consolidate the cleanup of non-fastback-cached inner windows currently spread throughout SetNewDocument into a pair of calls to FreeInnerObjects, and various other cleanup. b=353090 r=jst sr=bzbarsky

content/events/src/nsEventListenerManager.cpp

@@ -425,6 +425,7 @@ GenericListenersHashEnum(nsHashKey *aKey, void *aData, void* closure)
 
 nsEventListenerManager::~nsEventListenerManager() 
 {
+  NS_ASSERTION(!mTarget, "didn't call Disconnect");
   RemoveAllListeners();
 
   --mInstanceCount;

dom/public/nsIScriptContext.h

@@ -423,12 +423,16 @@ public:
   /**
    * Clear the scope object - may be called either as we are being torn down,
    * or before we are attached to a different document.
-   * XXXmarkh - aClearPolluter is quite likely bogus - just that some places
-   * that did this clear did not call InvalidateGlobalScopePolluter.  It
-   * seems likely this param should be dropped and that fn always called.
-   * OR some extra virtual added to abstract when that Invalidate need happen.
+   *
+   * aClearFromProtoChain is probably somewhat JavaScript specific.  It
+   * indicates that the global scope polluter should be removed from the
+   * prototype chain and that the objects in the prototype chain should
+   * also have their scopes cleared.  We don't do this all the time
+   * because the prototype chain is shared between inner and outer
+   * windows, and needs to stay with inner windows that we're keeping
+   * around.
    */
-  virtual void ClearScope(void* aGlobalObj, PRBool aClearPolluter) = 0;
+  virtual void ClearScope(void* aGlobalObj, PRBool aClearFromProtoChain) = 0;
 
   /**
    * Tell the context we're about to be reinitialize it.

dom/src/base/nsGlobalWindow.cpp

@@ -590,7 +590,7 @@ nsGlobalWindow::FreeInnerObjects(nsIScriptContext *scx)
 
   if (scx)
     scx->ClearScope(mScriptGlobals[NS_STID_INDEX(scx->GetScriptTypeID())],
-                         PR_TRUE);
+                    PR_TRUE);
 }
 
 //*****************************************************************************
@@ -1103,18 +1103,6 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
 
   nsCOMPtr<nsIDocument> oldDoc(do_QueryInterface(mDocument));
 
-  // Always clear watchpoints, to deal with two cases:
-  // 1.  The first document for this window is loading, and a miscreant has
-  //     preset watchpoints on the window object in order to attack the new
-  //     document's privileged information.
-  // 2.  A document loaded and used watchpoints on its own window, leaving
-  //     them set until the next document loads. We must clean up window
-  //     watchpoints here.
-  // Watchpoints set on document and subordinate objects are all cleared
-  // when those sub-window objects are finalized, after JS_ClearScope and
-  // a GC run that finds them to be garbage.
-
-  // XXXjst: Update above comment.
   nsIScriptContext *scx = GetContextInternal();
   NS_ENSURE_TRUE(scx, NS_ERROR_NOT_INITIALIZED);
 
@@ -1214,23 +1202,6 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
 
     nsGlobalWindow *currentInner = GetCurrentInnerWindowInternal();
 
-    if (currentInner && !currentInner->IsFrozen()) {
-      if (!reUseInnerWindow) {
-        currentInner->ClearAllTimeouts();
-
-        currentInner->mChromeEventHandler = nsnull;
-      }
-
-      if (!reUseInnerWindow && currentInner->mListenerManager) {
-        currentInner->mListenerManager->Disconnect();
-        currentInner->mListenerManager = nsnull;
-      }
-
-      if (!reUseInnerWindow || aDocument != oldDoc) {
-        nsWindowSH::InvalidateGlobalScopePolluter(cx, currentInner->mJSObject);
-      }
-    }
-
     nsRefPtr<nsGlobalWindow> newInnerWindow;
 
     nsCOMPtr<nsIDOMChromeWindow> thisChrome =
@@ -1257,6 +1228,10 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
       NS_ASSERTION(!currentInner->IsFrozen(),
                    "We should never be reusing a shared inner window");
       newInnerWindow = currentInner;
+
+      if (aDocument != oldDoc) {
+        nsWindowSH::InvalidateGlobalScopePolluter(cx, currentInner->mJSObject);
+      }
     } else {
       if (aState) {
         nsCOMPtr<WindowStateHolder> wsh = do_QueryInterface(aState);
@@ -1382,17 +1357,16 @@ nsGlobalWindow::SetNewDocument(nsIDocument* aDocument,
         // Don't clear scope on our current inner window if it's going to be
         // held in the bfcache.
         if (!currentInner->IsFrozen()) {
-          // xxxmarkh - 'termfunc' still js impl specific...
-          if (!termFuncSet) {
-            scx->ClearScope(currentInner->mJSObject, PR_FALSE);
+          if (termFuncSet) {
+            // Passing null to FreeInnerObjects means it skips the
+            // ClearScope, which we need to do later.
+            currentInner->FreeInnerObjects(nsnull);
+          } else {
+            NS_STID_FOR_ID(st_id) {
+              nsIScriptContext *this_ctx = GetScriptContextInternal(st_id);
+              currentInner->FreeInnerObjects(scx);
+            }
           }
-
-          // Make the current inner window release its strong references
-          // to the document to prevent it from keeping everything
-          // around. But remember the document's principal.
-          currentInner->mDocument = nsnull;
-          currentInner->mDoc = nsnull;
-          currentInner->mDocumentPrincipal = oldPrincipal;
         }
       }
 
@@ -6222,26 +6196,14 @@ void
 nsGlobalWindow::ClearWindowScope(nsISupports *aWindow)
 {
   nsCOMPtr<nsIScriptGlobalObject> sgo(do_QueryInterface(aWindow));
-  nsIScriptContext *jsscx = sgo->GetScriptContext(
-                              nsIProgrammingLanguage::JAVASCRIPT);
-  JSContext *cx = jsscx
-                  ? NS_STATIC_CAST(JSContext *, jsscx->GetNativeContext())
-                  : nsnull;
-  if (cx) {
-    JS_BeginRequest(cx);
-  }
-  
   PRUint32 lang_id;
   NS_STID_FOR_ID(lang_id) {
     nsIScriptContext *scx = sgo->GetScriptContext(lang_id);
     if (scx) {
       void *global = sgo->GetScriptGlobal(lang_id);
-      scx->ClearScope(global, PR_FALSE);
+      scx->ClearScope(global, PR_TRUE);
     }
   }
-  if (cx) {
-    JS_EndRequest(cx);
-  }
 }
 
 //*****************************************************************************

dom/src/base/nsGlobalWindow.h

@@ -343,7 +343,8 @@ protected:
   void CleanUp();
   void ClearControllers();
 
-  void FreeInnerObjects(nsIScriptContext *cx);
+  // If |scx| is non-null, also calls ClearScope on |scx|.
+  void FreeInnerObjects(nsIScriptContext *scx);
 
   nsresult SetNewDocument(nsIDocument *aDocument,
                           nsISupports *aState,

dom/src/base/nsJSEnvironment.cpp

@@ -2919,17 +2919,36 @@ nsJSContext::InitClasses(void *aGlobalObj)
 }
 
 void
-nsJSContext::ClearScope(void *aGlobalObj, PRBool aClearPolluter)
+nsJSContext::ClearScope(void *aGlobalObj, PRBool aClearFromProtoChain)
 {
   if (aGlobalObj) {
     JSObject *obj = (JSObject *)aGlobalObj;
     JSAutoRequest ar(mContext);
     ::JS_ClearScope(mContext, obj);
+
+    // Always clear watchpoints, to deal with two cases:
+    // 1.  The first document for this window is loading, and a miscreant has
+    //     preset watchpoints on the window object in order to attack the new
+    //     document's privileged information.
+    // 2.  A document loaded and used watchpoints on its own window, leaving
+    //     them set until the next document loads. We must clean up window
+    //     watchpoints here.
+    // Watchpoints set on document and subordinate objects are all cleared
+    // when those sub-window objects are finalized, after JS_ClearScope and
+    // a GC run that finds them to be garbage.
     ::JS_ClearWatchPointsForObject(mContext, obj);
 
-    // xxxmarkh: aClearPolluter is bogus???
-    if (aClearPolluter)
-        nsWindowSH::InvalidateGlobalScopePolluter(mContext, obj);
+    // Since the prototype chain is shared between inner and outer (and
+    // stays with the inner), we don't clear things from the prototype
+    // chain when we're clearing an outer window whose current inner we
+    // still want.
+    if (aClearFromProtoChain) {
+      nsWindowSH::InvalidateGlobalScopePolluter(mContext, obj);
+
+      for (JSObject *o = ::JS_GetPrototype(mContext, obj);
+           o; o = ::JS_GetPrototype(mContext, o))
+        ::JS_ClearScope(mContext, o);
+    }
   }
   ::JS_ClearRegExpStatics(mContext);
 
@@ -3015,6 +3034,8 @@ nsresult
 nsJSContext::SetTerminationFunction(nsScriptTerminationFunc aFunc,
                                     nsISupports* aRef)
 {
+  NS_PRECONDITION(mContext->fp, "should be executing script");
+
   nsJSContext::TerminationFuncClosure* newClosure =
     new nsJSContext::TerminationFuncClosure(aFunc, aRef, mTerminations);
   if (!newClosure) {