Process XBL constructors right after the frame construction inInitialReflow(). Bug 377119, r+sr=sicking

content/base/src/nsContentSink.cpp

@@ -867,6 +867,7 @@ nsresult
 nsContentSink::RefreshIfEnabled(nsIViewManager* vm)
 {
   if (!vm) {
+    // vm might be null if the shell got Destroy() called already
     return NS_OK;
   }
 
@@ -913,8 +914,11 @@ nsContentSink::StartLayout(PRBool aIgnorePendingSheets)
   mLayoutStarted = PR_TRUE;
   mLastNotificationTime = PR_Now();
   
-  PRUint32 i, ns = mDocument->GetNumberOfShells();
-  for (i = 0; i < ns; i++) {
+  PRUint32 i;
+
+  // XXXbz Shells can get removed (or added!) as we iterate through this loop.
+  // We should try to use an nsTObserverArray for this.
+  for (i = 0; i < mDocument->GetNumberOfShells(); i++) {
     nsIPresShell *shell = mDocument->GetShellAt(i);
 
     if (shell) {
@@ -940,6 +944,7 @@ nsContentSink::StartLayout(PRBool aIgnorePendingSheets)
 
       // Resize-reflow this time
       nsRect r = shell->GetPresContext()->GetVisibleArea();
+      nsCOMPtr<nsIPresShell> shellGrip = shell;
       nsresult rv = shell->InitialReflow(r.width, r.height);
       if (NS_FAILED(rv)) {
         return;

content/html/document/src/nsMediaDocument.cpp

@@ -267,6 +267,8 @@ nsresult
 nsMediaDocument::StartLayout()
 {
   PRUint32 numberOfShells = GetNumberOfShells();
+  // XXXbz Shells can get removed (or added!) as we iterate through this loop.
+  // We should try to use an nsTObserverArray for this.
   for (PRUint32 i = 0; i < numberOfShells; i++) {
     nsIPresShell *shell = GetShellAt(i);
 
@@ -275,10 +277,12 @@ nsMediaDocument::StartLayout()
 
     // Initial-reflow this time.
     nsRect visibleArea = shell->GetPresContext()->GetVisibleArea();
+    nsCOMPtr<nsIPresShell> shellGrip = shell;
     nsresult rv = shell->InitialReflow(visibleArea.width, visibleArea.height);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    // Now trigger a refresh.
+    // Now trigger a refresh.  vm might be null if the presshell got
+    // Destroy() called already.
     nsIViewManager* vm = shell->GetViewManager();
     if (vm) {
       vm->EnableRefresh(NS_VMREFRESH_IMMEDIATE);

content/xul/document/src/nsXULDocument.cpp

@@ -1973,8 +1973,9 @@ nsXULDocument::StartLayout(void)
         return NS_OK;
     }
 
-    PRUint32 count = GetNumberOfShells();
-    for (PRUint32 i = 0; i < count; ++i) {
+    // XXXbz Shells can get removed (or added!) as we iterate through this
+    // loop.  We should try to use an nsTObserverArray for this.
+    for (PRUint32 i = 0; i < GetNumberOfShells(); ++i) {
         nsIPresShell *shell = GetShellAt(i);
 
         // Resize-reflow this time
@@ -2000,6 +2001,7 @@ nsXULDocument::StartLayout(void)
         // dropping dirty rects if refresh is disabled rather than
         // accumulating them until refresh is enabled and then
         // triggering a repaint...
+        // XXXbz Is that still the case?
         nsresult rv = NS_OK;
         nsIViewManager* vm = shell->GetViewManager();
         if (vm) {
@@ -2014,12 +2016,18 @@ nsXULDocument::StartLayout(void)
             }
         }
 
+        // Make sure we're holding a strong ref to |shell| before we call
+        // InitialReflow()
+        nsCOMPtr<nsIPresShell> shellGrip = shell;
         rv = shell->InitialReflow(r.width, r.height);
         NS_ENSURE_SUCCESS(rv, rv);
 
         // Start observing the document _after_ we do the initial
         // reflow. Otherwise, we'll get into an trouble trying to
         // create kids before the root frame is established.
+        // XXXbz why is that an issue here and not in nsContentSink or
+        // nsDocumentViewer?  Perhaps we should just flush the way
+        // nsDocumentViewer does?
         shell->BeginObservingDocument();
     }
 

layout/base/nsDocumentViewer.cpp

@@ -701,11 +701,12 @@ DocumentViewerImpl::InitPresentationStuff(PRBool aDoInitialReflow)
       htmlDoc->SetIsFrameset(frameset != nsnull);
     }
 
+    nsCOMPtr<nsIPresShell> shellGrip = mPresShell;
     // Initial reflow
     mPresShell->InitialReflow(width, height);
 
     // Now trigger a refresh
-    if (mEnableRendering) {
+    if (mEnableRendering && mViewManager) {
       mViewManager->EnableRefresh(NS_VMREFRESH_IMMEDIATE);
     }
   } else {

layout/base/nsIPresShell.h

@@ -253,7 +253,8 @@ public:
    */
   nsFrameSelection* FrameSelection() { return mSelection; }
 
-  // Make shell be a document observer
+  // Make shell be a document observer.  If called after Destroy() has
+  // been called on the shell, this will be ignored.
   NS_IMETHOD BeginObservingDocument() = 0;
 
   // Make shell stop being a document observer
@@ -271,7 +272,11 @@ public:
    * object and then reflows the frame model into the specified width and
    * height.
    *
-   * The coordinates for aWidth and aHeight must be in standard nscoord's.
+   * The coordinates for aWidth and aHeight must be in standard nscoords.
+   *
+   * Callers of this method must hold a reference to this shell that
+   * is guaranteed to survive through arbitrary script execution.
+   * Calling InitialReflow can execute arbitrary script.
    */
   NS_IMETHOD InitialReflow(nscoord aWidth, nscoord aHeight) = 0;
 

layout/base/nsPresShell.cpp

@@ -2286,7 +2286,7 @@ PresShell::RepaintSelection(SelectionType aType)
 NS_IMETHODIMP
 PresShell::BeginObservingDocument()
 {
-  if (mDocument) {
+  if (mDocument && !mIsDestroying) {
     mDocument->AddObserver(this);
     if (mIsDocumentGone) {
       NS_WARNING("Adding a presshell that was disconnected from the document "
@@ -2460,6 +2460,24 @@ PresShell::InitialReflow(nscoord aWidth, nscoord aHeight)
     // Something in mFrameConstructor->ContentInserted may have caused
     // Destroy() to get called, bug 337586.
     NS_ENSURE_STATE(!mHaveShutDown);
+
+    // Run the XBL binding constructors for any new frames we've constructed
+    mDocument->BindingManager()->ProcessAttachedQueue();
+
+    // Constructors may have killed us too
+    NS_ENSURE_STATE(!mHaveShutDown);
+
+    // Now flush out pending restyles before we actually reflow, in
+    // case XBL constructors changed styles somewhere.
+    mFrameConstructor->ProcessPendingRestyles();
+
+    // And that might have run _more_ XBL constructors
+    NS_ENSURE_STATE(!mHaveShutDown);
+
+    // Now reget the root frame, since all that script might have affected it
+    // somehow.  Currently that can't happen, as long as mHaveShutDown is
+    // false, but let's not rely on that.
+    rootFrame = FrameManager()->GetRootFrame();
   }
 
   if (rootFrame) {
@@ -2509,9 +2527,6 @@ PresShell::InitialReflow(nscoord aWidth, nscoord aHeight)
     }
   }
 
-  // Run the XBL binding constructors for any new frames we've constructed
-  mDocument->BindingManager()->ProcessAttachedQueue();
-
   return NS_OK; //XXX this needs to be real. MMP
 }
 

layout/printing/nsPrintEngine.cpp

@@ -1917,6 +1917,7 @@ nsPrintEngine::ReflowPrintObject(nsPrintObject * aPO)
   rv = aPO->mPresShell->InitialReflow(adjSize.width, adjSize.height);
 
   NS_ENSURE_SUCCESS(rv, rv);
+  NS_ASSERTION(aPO->mPresShell, "Presshell should still be here");
 
   // Process the reflow event InitialReflow posted
   aPO->mPresShell->FlushPendingNotifications(Flush_OnlyReflow);