Bug 504221 part 8. Make CreateAnonymousColFrames saner. r=bernd, sr=roc
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 28 Jul 2009 08:53:18 -0400
changeset 30786 42a2d6b00a95619a8325a12841eb6d1b23e4db43
parent 30785 48c58133db953ca5051270cdbe74aa0ca7c81b8f
child 30787 25ee2119ace10ccd32aacc79f70618e4f74a4b4e
push idunknown
push userunknown
push dateunknown
reviewersbernd, roc
bugs504221
milestone1.9.2a1pre
Bug 504221 part 8. Make CreateAnonymousColFrames saner. r=bernd, sr=roc
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/tables/nsTableColGroupFrame.cpp
layout/tables/nsTableColGroupFrame.h
layout/tables/nsTableFrame.cpp
layout/tables/nsTableFrame.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -671,76 +671,16 @@ CleanupFrameReferences(nsFrameManager*  
 {
   for (nsFrameList::Enumerator e(aFrameList); !e.AtEnd(); e.Next()) {
     DoCleanupFrameReferences(aFrameManager, e.get());
   }
 }
 
 // -----------------------------------------------------------
 
-// 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)
 {
 }
 
 void 
 nsFrameItems::AddChild(nsIFrame* aChild)
 {
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -76,16 +76,76 @@ struct nsFindFrameHint
 };
 
 typedef void (nsLazyFrameConstructionCallback)
              (nsIContent* aContent, nsIFrame* aFrame, void* aArg);
 
 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:
   nsCSSFrameConstructor(nsIDocument *aDocument, nsIPresShell* aPresShell);
   ~nsCSSFrameConstructor(void) {
     NS_ASSERTION(mUpdateCount == 0, "Dying in the middle of our own update?");
   }
 
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -187,22 +187,18 @@ nsTableColGroupFrame::SetInitialChildLis
     NS_NOTREACHED("unknown frame list");
     return NS_ERROR_INVALID_ARG;
   } 
   nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
   if (!tableFrame)
     return NS_ERROR_NULL_POINTER;
 
   if (!aChildList) {
-    nsIFrame* firstChild;
-    tableFrame->CreateAnonymousColFrames(this, GetSpan(), eColAnonymousColGroup, 
-                                         PR_FALSE, nsnull, &firstChild);
-    if (firstChild) {
-      SetInitialChildList(aListName, firstChild);
-    }
+    tableFrame->AppendAnonymousColFrames(this, GetSpan(), eColAnonymousColGroup, 
+                                         PR_FALSE);
     return NS_OK; 
   }
 
   mFrames.AppendFrames(this, aChildList);
   return NS_OK;
 }
 
 /* virtual */ void
--- a/layout/tables/nsTableColGroupFrame.h
+++ b/layout/tables/nsTableColGroupFrame.h
@@ -202,17 +202,17 @@ public:
     */
   void SetStartColumnIndex(PRInt32 aIndex);
 
   /** helper method to get the span attribute for this colgroup */
   PRInt32 GetSpan();
 
   /** 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
     * @param aFirstColGroup - start the reset operation inside this colgroup
     * @param aFirstColIndex - first column that is reset should get this index
     * @param aStartColFrame - if specified the reset starts with this column
     *                         inside the colgroup; if not specified, the reset
     *                         starts with the first column
@@ -270,15 +270,15 @@ inline void nsTableColGroupFrame::SetSta
   mStartColIndex = aIndex;
 }
 
 inline PRInt32 nsTableColGroupFrame::GetColCount() const
 {  
   return mColCount;
 }
 
-inline nsFrameList& nsTableColGroupFrame::GetChildList()
+inline nsFrameList& nsTableColGroupFrame::GetWritableChildList()
 {  
   return mFrames;
 }
 
 #endif
 
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -708,76 +708,53 @@ nsTableFrame::CreateAnonymousColGroupFra
     newFrame->Init(colGroupContent, this, nsnull);
   }
   return (nsTableColGroupFrame *)newFrame;
 }
 
 void
 nsTableFrame::AppendAnonymousColFrames(PRInt32 aNumColsToAdd)
 {
-  nsTableColFrame* prevCol = nsnull;
   // get the last col group frame
-  nsTableColGroupFrame* colGroupFrame = nsnull;
-  nsIFrame* childFrame = mColGroups.FirstChild();
-  while (childFrame) {
-    if (nsGkAtoms::tableColGroupFrame == childFrame->GetType()) {
-      colGroupFrame = (nsTableColGroupFrame *)childFrame;
-    }
-    childFrame = childFrame->GetNextSibling();
-  }
-
-  if (colGroupFrame &&
-      (colGroupFrame->GetColType() == eColGroupAnonymousCell)) {
-    prevCol =
-      static_cast<nsTableColFrame*> (colGroupFrame->GetChildList().LastChild());
-  }
-  else {
+  nsTableColGroupFrame* colGroupFrame =
+    static_cast<nsTableColGroupFrame*>(mColGroups.LastChild());
+
+  if (!colGroupFrame ||
+      (colGroupFrame->GetColType() != eColGroupAnonymousCell)) {
     PRInt32 colIndex = (colGroupFrame) ?
                         colGroupFrame->GetStartColumnIndex() +
                         colGroupFrame->GetColCount() : 0;
     colGroupFrame = CreateAnonymousColGroupFrame(eColGroupAnonymousCell);
     if (!colGroupFrame) {
       return;
     }
     // add the new frame to the child list
     mColGroups.AppendFrame(this, colGroupFrame);
     colGroupFrame->SetStartColumnIndex(colIndex);
   }
-  nsIFrame* firstNewFrame;
-  CreateAnonymousColFrames(colGroupFrame, aNumColsToAdd, eColAnonymousCell,
-                           PR_TRUE, prevCol, &firstNewFrame);
+  AppendAnonymousColFrames(colGroupFrame, aNumColsToAdd, eColAnonymousCell,
+                           PR_TRUE);
 
 }
 
 // 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,         
-                                       nsIFrame*             aPrevFrameIn,
-                                       nsIFrame**            aFirstNewFrame)
+                                       PRBool                aAddToTable)
 {
   NS_PRECONDITION(aColGroupFrame, "null frame");
   NS_PRECONDITION(aColType != eColAnonymousCol, "Shouldn't happen");
 
-  *aFirstNewFrame = nsnull;
-  nsIFrame* lastColFrame = nsnull;
-  nsPresContext* presContext = PresContext();
-  nsIPresShell *shell = presContext->PresShell();
+  nsIPresShell *shell = PresContext()->PresShell();
 
   // Get the last col frame
-  nsIFrame* childFrame = aColGroupFrame->GetFirstChild(nsnull);
-  while (childFrame) {
-    if (nsGkAtoms::tableColFrame == childFrame->GetType()) {
-      lastColFrame = (nsTableColGroupFrame *)childFrame;
-    }
-    childFrame = childFrame->GetNextSibling();
-  }
+  nsFrameItems newColFrames;
 
   PRInt32 startIndex = mColFrames.Length();
   PRInt32 lastIndex  = startIndex + aNumColsToAdd - 1; 
 
   for (PRInt32 childX = startIndex; childX <= lastIndex; childX++) {
     nsIContent* iContent;
     nsRefPtr<nsStyleContext> styleContext;
     nsStyleContext* parentStyleContext;
@@ -791,45 +768,36 @@ nsTableFrame::CreateAnonymousColFrames(n
                                                             parentStyleContext);
     // ASSERTION to check for bug 54454 sneaking back in...
     NS_ASSERTION(iContent, "null content in CreateAnonymousColFrames");
 
     // create the new col frame
     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
-    if (lastColFrame) {
-      lastColFrame->SetNextSibling(colFrame);
-    }
-    lastColFrame = colFrame;
-    if (childX == startIndex) {
-      *aFirstNewFrame = colFrame;
-    }
-  }
-  if (aAddToColGroupAndTable) {
-    nsFrameList& cols = aColGroupFrame->GetChildList();
-    // the chain already exists, now add it to the col group child list
-    if (!aPrevFrameIn) {
-      cols.AppendFrames(aColGroupFrame, *aFirstNewFrame);
-    }
+
+    newColFrames.AddChild(colFrame);
+  }
+  nsFrameList& cols = aColGroupFrame->GetWritableChildList();
+  nsIFrame* oldLastCol = cols.LastChild();
+  nsIFrame* firstNewCol = newColFrames.FirstChild();
+  nsIFrame* lastNewCol = newColFrames.lastChild;
+  cols.InsertFrames(nsnull, oldLastCol, newColFrames);
+  if (aAddToTable) {
     // get the starting col index in the cache
-    PRInt32 startColIndex = aColGroupFrame->GetStartColumnIndex();
-    if (aPrevFrameIn) {
-      nsTableColFrame* colFrame = 
-        (nsTableColFrame*)nsTableFrame::GetFrameAtOrBefore((nsIFrame*) aColGroupFrame, aPrevFrameIn, 
-                                                           nsGkAtoms::tableColFrame);
-      if (colFrame) {
-        startColIndex = colFrame->GetColIndex() + 1;
-      }
-    }
+    PRInt32 startColIndex;
+    if (oldLastCol) {
+      startColIndex =
+        static_cast<nsTableColFrame*>(oldLastCol)->GetColIndex() + 1;
+    } else {
+      startColIndex = aColGroupFrame->GetStartColumnIndex();
+    }
+
     aColGroupFrame->AddColsToTable(startColIndex, PR_TRUE, 
-                                  *aFirstNewFrame, lastColFrame);
+                                   firstNewCol, lastNewCol);
   }
 }
 
 void
 nsTableFrame::MatchCellMapToColCache(nsTableCellMap* aCellMap)
 {
   PRInt32 numColsInMap   = GetColCount();
   PRInt32 numColsInCache = mColFrames.Length();
--- a/layout/tables/nsTableFrame.h
+++ b/layout/tables/nsTableFrame.h
@@ -446,24 +446,28 @@ public:
     */
   void InsertCol(nsTableColFrame& aColFrame,
                  PRInt32          aColIndex);
 
   nsTableColGroupFrame* CreateAnonymousColGroupFrame(nsTableColGroupType aType);
 
   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,
-                                nsIFrame*             aPrevCol,
-                                nsIFrame**            aFirstNewFrame);
+                                PRBool                aAddToTable);
 
   void MatchCellMapToColCache(nsTableCellMap* aCellMap);
   /** empty the column frame cache */
   void ClearColCache();
 
   void DidResizeColumns();
 
   virtual void AppendCell(nsTableCellFrame& aCellFrame,