Bug 504221 part 8. Make CreateAnonymousColFrames saner. r=bernd, sr=roc

layout/base/nsCSSFrameConstructor.cpp

@@ -676,66 +676,6 @@ CleanupFrameReferences(nsFrameManager*  aFrameManager,
 
 // -----------------------------------------------------------
 
-// Structure used when constructing formatting object trees.
-struct nsFrameItems : public nsFrameList {
-  nsIFrame* lastChild;
-  
-  nsFrameItems(nsIFrame* aFrame = nsnull);
-
-  // Appends the frame to the end of the list
-  void AddChild(nsIFrame* aChild);
-
-  void InsertFrame(nsIFrame* aParent, nsIFrame* aPrevSibling,
-                   nsIFrame* aNewFrame) {
-    nsFrameList::InsertFrame(aParent, aPrevSibling, aNewFrame);
-    if (aPrevSibling == lastChild) {
-      lastChild = aNewFrame;
-    }
-  }
-
-  void InsertFrames(nsIFrame* aParent, nsIFrame* aPrevSibling,
-                    nsFrameItems& aFrames) {
-    nsFrameList::InsertFrames(aParent, aPrevSibling, aFrames);
-    if (aPrevSibling == lastChild) {
-      lastChild = aFrames.lastChild;
-    }
-  }
-
-  void DestroyFrame(nsIFrame* aFrameToDestroy, nsIFrame* aPrevSibling) {
-    NS_PRECONDITION((!aPrevSibling && aFrameToDestroy == FirstChild()) ||
-                    aPrevSibling->GetNextSibling() == aFrameToDestroy,
-                    "Unexpected prevsibling");
-    nsFrameList::DestroyFrame(aFrameToDestroy, aPrevSibling);
-    if (aFrameToDestroy == lastChild) {
-      lastChild = aPrevSibling;
-    }
-  }
-
-  PRBool RemoveFrame(nsIFrame* aFrameToRemove, nsIFrame* aPrevSibling) {
-    NS_PRECONDITION(!aPrevSibling ||
-                    aPrevSibling->GetNextSibling() == aFrameToRemove,
-                    "Unexpected aPrevSibling");
-    if (!aPrevSibling) {
-      aPrevSibling = GetPrevSiblingFor(aFrameToRemove);
-    }
-
-    PRBool removed = nsFrameList::RemoveFrame(aFrameToRemove, aPrevSibling);
-
-    if (aFrameToRemove == lastChild) {
-      lastChild = aPrevSibling;
-    }
-
-    return removed;
-  }
-
-  void Clear() {
-    mFirstChild = lastChild = nsnull;
-  }
-
-  // For now, until we change some SetInitialChildList signatures
-  operator nsIFrame* ()  { return FirstChild(); }
-};
-
 nsFrameItems::nsFrameItems(nsIFrame* aFrame)
   : nsFrameList(aFrame), lastChild(aFrame)
 {

layout/base/nsCSSFrameConstructor.h

@@ -81,6 +81,66 @@ typedef void (nsLazyFrameConstructionCallback)
 class nsFrameConstructorState;
 class nsFrameConstructorSaveState;
   
+// Structure used when constructing formatting object trees.
+struct nsFrameItems : public nsFrameList {
+  nsIFrame* lastChild;
+  
+  nsFrameItems(nsIFrame* aFrame = nsnull);
+
+  // Appends the frame to the end of the list
+  void AddChild(nsIFrame* aChild);
+
+  void InsertFrame(nsIFrame* aParent, nsIFrame* aPrevSibling,
+                   nsIFrame* aNewFrame) {
+    nsFrameList::InsertFrame(aParent, aPrevSibling, aNewFrame);
+    if (aPrevSibling == lastChild) {
+      lastChild = aNewFrame;
+    }
+  }
+
+  void InsertFrames(nsIFrame* aParent, nsIFrame* aPrevSibling,
+                    nsFrameItems& aFrames) {
+    nsFrameList::InsertFrames(aParent, aPrevSibling, aFrames);
+    if (aPrevSibling == lastChild) {
+      lastChild = aFrames.lastChild;
+    }
+  }
+
+  void DestroyFrame(nsIFrame* aFrameToDestroy, nsIFrame* aPrevSibling) {
+    NS_PRECONDITION((!aPrevSibling && aFrameToDestroy == FirstChild()) ||
+                    aPrevSibling->GetNextSibling() == aFrameToDestroy,
+                    "Unexpected prevsibling");
+    nsFrameList::DestroyFrame(aFrameToDestroy, aPrevSibling);
+    if (aFrameToDestroy == lastChild) {
+      lastChild = aPrevSibling;
+    }
+  }
+
+  PRBool RemoveFrame(nsIFrame* aFrameToRemove, nsIFrame* aPrevSibling) {
+    NS_PRECONDITION(!aPrevSibling ||
+                    aPrevSibling->GetNextSibling() == aFrameToRemove,
+                    "Unexpected aPrevSibling");
+    if (!aPrevSibling) {
+      aPrevSibling = GetPrevSiblingFor(aFrameToRemove);
+    }
+
+    PRBool removed = nsFrameList::RemoveFrame(aFrameToRemove, aPrevSibling);
+
+    if (aFrameToRemove == lastChild) {
+      lastChild = aPrevSibling;
+    }
+
+    return removed;
+  }
+
+  void Clear() {
+    mFirstChild = lastChild = nsnull;
+  }
+
+  // For now, until we change some SetInitialChildList signatures
+  operator nsIFrame* ()  { return FirstChild(); }
+};
+
 class nsCSSFrameConstructor
 {
 public:

layout/tables/nsTableColGroupFrame.cpp

@@ -192,12 +192,8 @@ nsTableColGroupFrame::SetInitialChildList(nsIAtom*        aListName,
     return NS_ERROR_NULL_POINTER;
 
   if (!aChildList) {
-    nsIFrame* firstChild;
+    tableFrame->AppendAnonymousColFrames(this, GetSpan(), eColAnonymousColGroup, 
-    tableFrame->CreateAnonymousColFrames(this, GetSpan(), eColAnonymousColGroup, 
+                                         PR_FALSE);
-                                         PR_FALSE, nsnull, &firstChild);
-    if (firstChild) {
-      SetInitialChildList(aListName, firstChild);
-    }
     return NS_OK; 
   }
 

layout/tables/nsTableColGroupFrame.h

@@ -207,7 +207,7 @@ public:
 
   /** provide access to the mFrames list
     */
-  nsFrameList& GetChildList();
+  nsFrameList& GetWritableChildList();
 
   /** set the column index for all frames starting at aStartColFrame, it
     * will also reset the column indices in all subsequent colgroups
@@ -275,7 +275,7 @@ inline PRInt32 nsTableColGroupFrame::GetColCount() const
   return mColCount;
 }
 
-inline nsFrameList& nsTableColGroupFrame::GetChildList()
+inline nsFrameList& nsTableColGroupFrame::GetWritableChildList()
 {  
   return mFrames;
 }

layout/tables/nsTableFrame.cpp

@@ -713,23 +713,12 @@ nsTableFrame::CreateAnonymousColGroupFrame(nsTableColGroupType aColGroupType)
 void
 nsTableFrame::AppendAnonymousColFrames(PRInt32 aNumColsToAdd)
 {
-  nsTableColFrame* prevCol = nsnull;
   // get the last col group frame
-  nsTableColGroupFrame* colGroupFrame = nsnull;
+  nsTableColGroupFrame* colGroupFrame =
-  nsIFrame* childFrame = mColGroups.FirstChild();
+    static_cast<nsTableColGroupFrame*>(mColGroups.LastChild());
-  while (childFrame) {
-    if (nsGkAtoms::tableColGroupFrame == childFrame->GetType()) {
-      colGroupFrame = (nsTableColGroupFrame *)childFrame;
-    }
-    childFrame = childFrame->GetNextSibling();
-  }
 
-  if (colGroupFrame &&
+  if (!colGroupFrame ||
-      (colGroupFrame->GetColType() == eColGroupAnonymousCell)) {
+      (colGroupFrame->GetColType() != eColGroupAnonymousCell)) {
-    prevCol =
-      static_cast<nsTableColFrame*> (colGroupFrame->GetChildList().LastChild());
-  }
-  else {
     PRInt32 colIndex = (colGroupFrame) ?
                         colGroupFrame->GetStartColumnIndex() +
                         colGroupFrame->GetColCount() : 0;
@@ -741,38 +730,26 @@ nsTableFrame::AppendAnonymousColFrames(PRInt32 aNumColsToAdd)
     mColGroups.AppendFrame(this, colGroupFrame);
     colGroupFrame->SetStartColumnIndex(colIndex);
   }
-  nsIFrame* firstNewFrame;
+  AppendAnonymousColFrames(colGroupFrame, aNumColsToAdd, eColAnonymousCell,
-  CreateAnonymousColFrames(colGroupFrame, aNumColsToAdd, eColAnonymousCell,
+                           PR_TRUE);
-                           PR_TRUE, prevCol, &firstNewFrame);
 
 }
 
 // XXX this needs to be moved to nsCSSFrameConstructor
 // Right now it only creates the col frames at the end 
 void
-nsTableFrame::CreateAnonymousColFrames(nsTableColGroupFrame* aColGroupFrame,
+nsTableFrame::AppendAnonymousColFrames(nsTableColGroupFrame* aColGroupFrame,
                                        PRInt32               aNumColsToAdd,
                                        nsTableColType        aColType,
-                                       PRBool                aAddToColGroupAndTable,         
+                                       PRBool                aAddToTable)
-                                       nsIFrame*             aPrevFrameIn,
-                                       nsIFrame**            aFirstNewFrame)
 {
   NS_PRECONDITION(aColGroupFrame, "null frame");
   NS_PRECONDITION(aColType != eColAnonymousCol, "Shouldn't happen");
 
-  *aFirstNewFrame = nsnull;
+  nsIPresShell *shell = PresContext()->PresShell();
-  nsIFrame* lastColFrame = nsnull;
-  nsPresContext* presContext = PresContext();
-  nsIPresShell *shell = presContext->PresShell();
 
   // Get the last col frame
-  nsIFrame* childFrame = aColGroupFrame->GetFirstChild(nsnull);
+  nsFrameItems newColFrames;
-  while (childFrame) {
-    if (nsGkAtoms::tableColFrame == childFrame->GetType()) {
-      lastColFrame = (nsTableColGroupFrame *)childFrame;
-    }
-    childFrame = childFrame->GetNextSibling();
-  }
 
   PRInt32 startIndex = mColFrames.Length();
   PRInt32 lastIndex  = startIndex + aNumColsToAdd - 1; 
@@ -796,35 +773,26 @@ nsTableFrame::CreateAnonymousColFrames(nsTableColGroupFrame* aColGroupFrame,
     nsIFrame* colFrame = NS_NewTableColFrame(shell, styleContext);
     ((nsTableColFrame *) colFrame)->SetColType(aColType);
     colFrame->Init(iContent, aColGroupFrame, nsnull);
-    colFrame->SetInitialChildList(nsnull, nsnull);
 
-    // Add the col to the sibling chain
+    newColFrames.AddChild(colFrame);
-    if (lastColFrame) {
-      lastColFrame->SetNextSibling(colFrame);
-    }
-    lastColFrame = colFrame;
-    if (childX == startIndex) {
-      *aFirstNewFrame = colFrame;
-    }
   }
-  if (aAddToColGroupAndTable) {
+  nsFrameList& cols = aColGroupFrame->GetWritableChildList();
-    nsFrameList& cols = aColGroupFrame->GetChildList();
+  nsIFrame* oldLastCol = cols.LastChild();
-    // the chain already exists, now add it to the col group child list
+  nsIFrame* firstNewCol = newColFrames.FirstChild();
-    if (!aPrevFrameIn) {
+  nsIFrame* lastNewCol = newColFrames.lastChild;
-      cols.AppendFrames(aColGroupFrame, *aFirstNewFrame);
+  cols.InsertFrames(nsnull, oldLastCol, newColFrames);
-    }
+  if (aAddToTable) {
     // get the starting col index in the cache
-    PRInt32 startColIndex = aColGroupFrame->GetStartColumnIndex();
+    PRInt32 startColIndex;
-    if (aPrevFrameIn) {
+    if (oldLastCol) {
-      nsTableColFrame* colFrame = 
+      startColIndex =
-        (nsTableColFrame*)nsTableFrame::GetFrameAtOrBefore((nsIFrame*) aColGroupFrame, aPrevFrameIn, 
+        static_cast<nsTableColFrame*>(oldLastCol)->GetColIndex() + 1;
-                                                           nsGkAtoms::tableColFrame);
+    } else {
-      if (colFrame) {
+      startColIndex = aColGroupFrame->GetStartColumnIndex();
-        startColIndex = colFrame->GetColIndex() + 1;
-      }
     }
+
     aColGroupFrame->AddColsToTable(startColIndex, PR_TRUE, 
-                                  *aFirstNewFrame, lastColFrame);
+                                   firstNewCol, lastNewCol);
   }
 }
 

layout/tables/nsTableFrame.h

@@ -451,14 +451,18 @@ public:
 
   PRInt32 DestroyAnonymousColFrames(PRInt32 aNumFrames);
 
+  // Append aNumColsToAdd anonymous col frames of type eColAnonymousCell to our
+  // last eColGroupAnonymousCell colgroup.  If we have no such colgroup, then
+  // create one.
   void AppendAnonymousColFrames(PRInt32 aNumColsToAdd);
 
-  void CreateAnonymousColFrames(nsTableColGroupFrame* aColGroupFrame,
+  // Append aNumColsToAdd anonymous col frames of type aColType to
+  // aColGroupFrame.  If aAddToTable is true, also call AddColsToTable on the
+  // new cols.
+  void AppendAnonymousColFrames(nsTableColGroupFrame* aColGroupFrame,
                                 PRInt32               aNumColsToAdd,
                                 nsTableColType        aColType,
-                                PRBool                aAddToColGroupAndTable,
+                                PRBool                aAddToTable);
-                                nsIFrame*             aPrevCol,
-                                nsIFrame**            aFirstNewFrame);
 
   void MatchCellMapToColCache(nsTableCellMap* aCellMap);
   /** empty the column frame cache */