Don't fire onload on a parent frame while we're in the middle of executing a

child's onload.  Bug 330089, r=biesi, sr=darin

uriloader/base/nsDocLoader.cpp

@@ -310,6 +310,12 @@ nsDocLoader::Stop(void)
   if (mLoadGroup)
     rv = mLoadGroup->Cancel(NS_BINDING_ABORTED);
 
+  // Clear out mChildrenInOnload.  We want to make sure to fire our
+  // onload at this point, and there's no issue with mChildrenInOnload
+  // after this, since mDocumentRequest will be null after the
+  // DocLoaderIsEmpty() call.
+  mChildrenInOnload.Clear();
+
   // Make sure to call DocLoaderIsEmpty now so that we reset mDocumentRequest,
   // etc, as needed.  We could be getting into here from a subframe onload, in
   // which case the call to DocLoaderIsEmpty() is coming but hasn't quite
@@ -319,6 +325,8 @@ nsDocLoader::Stop(void)
 
   // XXXbz If the child frame loadgroups were requests in mLoadgroup, I suspect
   // we wouldn't need the call here....
+
+  NS_ASSERTION(!IsBusy(), "Shouldn't be busy here");
   DocLoaderIsEmpty();
   
   return rv;
@@ -333,10 +341,16 @@ nsDocLoader::IsBusy()
   //
   // A document loader is busy if either:
   //
-  //   1. It is currently loading a document (ie. one or more URIs)
-  //   2. One of it's child document loaders is busy...
+  //   1. One of its children is in the middle of an onload handler.  Note that
+  //      the handler may have already removed this child from mChildList!
+  //   2. It is currently loading a document (ie. one or more URIs)
+  //   3. One of it's child document loaders is busy...
   //
 
+  if (mChildrenInOnload.Count()) {
+    return PR_TRUE;
+  }
+
   /* Is this document loader busy? */
   if (mIsLoadingDocument) {
     PRBool busy;
@@ -733,16 +747,20 @@ void nsDocLoader::DocLoaderIsEmpty()
       // Take a ref to our parent now so that we can call DocLoaderIsEmpty() on
       // it even if our onload handler removes us from the docloader tree.
       nsRefPtr<nsDocLoader> parent = mParent;
-      
-      //
-      // Do nothing after firing the OnEndDocumentLoad(...). The document
-      // loader may be loading a *new* document - if LoadDocument()
-      // was called from a handler!
-      //
-      doStopDocumentLoad(docRequest, loadGroupStatus);
 
-      if (parent) {
-        parent->DocLoaderIsEmpty();
+      // Note that if calling ChildEnteringOnload() on the parent returns false
+      // then calling our onload handler is not safe.  That can only happen on
+      // OOM, so that's ok.
+      if (!parent || parent->ChildEnteringOnload(this)) {
+        // Do nothing with our state after firing the
+        // OnEndDocumentLoad(...). The document loader may be loading a *new*
+        // document - if LoadDocument() was called from a handler!
+        //
+        doStopDocumentLoad(docRequest, loadGroupStatus);
+
+        if (parent) {
+          parent->ChildDoneWithOnload(this);
+        }
       }
     }
   }

uriloader/base/nsDocLoader.h

@@ -133,11 +133,6 @@ protected:
 
     virtual nsresult SetDocLoaderParent(nsDocLoader * aLoader);
 
-    // DocLoaderIsEmpty should be called whenever the docloader may be empty.
-    // This method is idempotent and does nothing if the docloader is not in
-    // fact empty.
-    void DocLoaderIsEmpty();
-
     PRBool IsBusy();
 
     void Destroy();
@@ -228,6 +223,33 @@ protected:
     PRBool mIsRestoringDocument;
 
 private:
+    // A list of kids that are in the middle of their onload calls and will let
+    // us know once they're done.  We don't want to fire onload for "normal"
+    // DocLoaderIsEmpty calls (those coming from requests finishing in our
+    // loadgroup) unless this is empty.
+    nsCOMArray<nsIDocumentLoader> mChildrenInOnload;
+    
+    // DocLoaderIsEmpty should be called whenever the docloader may be empty.
+    // This method is idempotent and does nothing if the docloader is not in
+    // fact empty.
+    void DocLoaderIsEmpty();
+
+    // Inform a parent docloader that aChild is about to call its onload
+    // handler.
+    PRBool ChildEnteringOnload(nsIDocumentLoader* aChild) {
+        // It's ok if we're already in the list -- we'll just be in there twice
+        // and then the RemoveObject calls from ChildDoneWithOnload will remove
+        // us.
+        return mChildrenInOnload.AppendObject(aChild);
+    }
+
+    // Inform a parent docloader that aChild is done calling its onload
+    // handler.
+    void ChildDoneWithOnload(nsIDocumentLoader* aChild) {
+        mChildrenInOnload.RemoveObject(aChild);
+        DocLoaderIsEmpty();
+    }        
+
     nsListenerInfo *GetListenerInfo(nsIWebProgressListener* aListener);
 
     PRInt64 GetMaxTotalProgress();