Use iterative algorithms when cloning and deleting lists. b=456196 r+sr=dbaron

layout/style/nsCSSDataBlock.cpp

@@ -470,10 +470,10 @@ nsCSSCompressedDataBlock::Clone() const
                         // fall through to keep gcc's uninitialized
                         // variable warning quiet
                     case eCSSType_ValueList:
-                        copy = new nsCSSValueList(*ValueListAtCursor(cursor));
+                        copy = ValueListAtCursor(cursor)->Clone();
                         break;
                     case eCSSType_ValuePairList:
-                        copy = new nsCSSValuePairList(*ValuePairListAtCursor(cursor));
+                        copy = ValuePairListAtCursor(cursor)->Clone();
                         break;
                 }
                 if (!copy) {

layout/style/nsCSSStruct.cpp

@@ -58,8 +58,6 @@
 #include "nsReadableUtils.h"
 #include "nsPrintfCString.h"
 
-#define CSS_IF_DELETE(ptr)  if (nsnull != ptr)  { delete ptr; ptr = nsnull; }
-
 // --- nsCSSFont -----------------
 
 nsCSSFont::nsCSSFont(void)
@@ -72,30 +70,23 @@ nsCSSFont::~nsCSSFont(void)
   MOZ_COUNT_DTOR(nsCSSFont);
 }
 
-// --- support -----------------
+// --- nsCSSValueList -----------------
 
-#define CSS_IF_COPY(val, type) \
+nsCSSValueList::~nsCSSValueList()
-  if (aCopy.val) (val) = new type(*(aCopy.val));
-
-nsCSSValueList::nsCSSValueList(void)
-  : mValue(),
-    mNext(nsnull)
-{
-  MOZ_COUNT_CTOR(nsCSSValueList);
-}
-
-nsCSSValueList::nsCSSValueList(const nsCSSValueList& aCopy)
-  : mValue(aCopy.mValue),
-    mNext(nsnull)
-{
-  MOZ_COUNT_CTOR(nsCSSValueList);
-  CSS_IF_COPY(mNext, nsCSSValueList);
-}
-
-nsCSSValueList::~nsCSSValueList(void)
 {
   MOZ_COUNT_DTOR(nsCSSValueList);
-  CSS_IF_DELETE(mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSValueList, this, mNext);
+}
+
+nsCSSValueList*
+nsCSSValueList::Clone(PRBool aDeep) const
+{
+  nsCSSValueList* result = new nsCSSValueList(*this);
+  if (NS_UNLIKELY(!result))
+    return result;
+  if (aDeep)
+    NS_CSS_CLONE_LIST_MEMBER(nsCSSValueList, this, mNext, result, (PR_FALSE));
+  return result;
 }
 
 /* static */ PRBool
@@ -135,7 +126,7 @@ nsCSSText::nsCSSText(void)
 nsCSSText::~nsCSSText(void)
 {
   MOZ_COUNT_DTOR(nsCSSText);
-  CSS_IF_DELETE(mTextShadow);
+  delete mTextShadow;
 }
 
 // --- nsCSSRect -----------------
@@ -208,7 +199,8 @@ nsCSSCornerSizes::SetAllCornersTo(const nsCSSValue& aValue)
 }
 
 void
-nsCSSCornerSizes::Reset() {
+nsCSSCornerSizes::Reset()
+{
   NS_FOR_CSS_FULL_CORNERS(corner) {
     this->GetFullCorner(corner).Reset();
   }
@@ -284,7 +276,7 @@ nsCSSMargin::nsCSSMargin(void)
 nsCSSMargin::~nsCSSMargin(void)
 {
   MOZ_COUNT_DTOR(nsCSSMargin);
-  CSS_IF_DELETE(mBoxShadow);
+  delete mBoxShadow;
 }
 
 // --- nsCSSPosition -----------------
@@ -349,25 +341,22 @@ nsCSSPage::~nsCSSPage(void)
 
 // --- nsCSSContent support -----------------
 
-nsCSSValuePairList::nsCSSValuePairList()
-  : mNext(nsnull)
-{
-  MOZ_COUNT_CTOR(nsCSSValuePairList);
-}
-
-nsCSSValuePairList::nsCSSValuePairList(const nsCSSValuePairList& aCopy)
-  : mXValue(aCopy.mXValue),
-    mYValue(aCopy.mYValue),
-    mNext(nsnull)
-{
-  MOZ_COUNT_CTOR(nsCSSValuePairList);
-  CSS_IF_COPY(mNext, nsCSSValuePairList);
-}
-
 nsCSSValuePairList::~nsCSSValuePairList()
 {
   MOZ_COUNT_DTOR(nsCSSValuePairList);
-  CSS_IF_DELETE(mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSValuePairList, this, mNext);
+}
+
+nsCSSValuePairList*
+nsCSSValuePairList::Clone(PRBool aDeep) const
+{
+  nsCSSValuePairList* result = new nsCSSValuePairList(*this);
+  if (NS_UNLIKELY(!result))
+    return result;
+  if (aDeep)
+    NS_CSS_CLONE_LIST_MEMBER(nsCSSValuePairList, this, mNext, result,
+                             (PR_FALSE));
+  return result;
 }
 
 /* static */ PRBool
@@ -400,10 +389,10 @@ nsCSSContent::nsCSSContent(void)
 nsCSSContent::~nsCSSContent(void)
 {
   MOZ_COUNT_DTOR(nsCSSContent);
-  CSS_IF_DELETE(mContent);
+  delete mContent;
-  CSS_IF_DELETE(mCounterIncrement);
+  delete mCounterIncrement;
-  CSS_IF_DELETE(mCounterReset);
+  delete mCounterReset;
-  CSS_IF_DELETE(mQuotes);
+  delete mQuotes;
 }
 
 // --- nsCSSUserInterface -----------------
@@ -417,7 +406,7 @@ nsCSSUserInterface::nsCSSUserInterface(void)
 nsCSSUserInterface::~nsCSSUserInterface(void)
 {
   MOZ_COUNT_DTOR(nsCSSUserInterface);
-  CSS_IF_DELETE(mCursor);
+  delete mCursor;
 }
 
 // --- nsCSSAural -----------------
@@ -467,7 +456,7 @@ nsCSSSVG::nsCSSSVG(void) : mStrokeDasharray(nsnull)
 nsCSSSVG::~nsCSSSVG(void)
 {
   MOZ_COUNT_DTOR(nsCSSSVG);
-  CSS_IF_DELETE(mStrokeDasharray);
+  delete mStrokeDasharray;
 }
 
 #endif // MOZ_SVG

layout/style/nsCSSStruct.h

@@ -47,18 +47,26 @@
 
 #include "nsCSSValue.h"
 #include "nsStyleConsts.h"
-#include <stdio.h>
 
 // Prefer nsCSSValue::Array for lists of fixed size.
 struct nsCSSValueList {
-  nsCSSValueList(void);
+  nsCSSValueList() : mNext(nsnull) { MOZ_COUNT_CTOR(nsCSSValueList); }
-  nsCSSValueList(const nsCSSValueList& aCopy);
+  ~nsCSSValueList();
-  ~nsCSSValueList(void);
+
+  nsCSSValueList* Clone() const { return Clone(PR_TRUE); }
 
   static PRBool Equal(nsCSSValueList* aList1, nsCSSValueList* aList2);
 
   nsCSSValue      mValue;
   nsCSSValueList* mNext;
+
+private:
+  nsCSSValueList(const nsCSSValueList& aCopy) // makes a shallow copy
+    : mValue(aCopy.mValue), mNext(nsnull)
+  {
+    MOZ_COUNT_CTOR(nsCSSValueList);
+  }
+  nsCSSValueList* Clone(PRBool aDeep) const;
 };
 
 struct nsCSSRect {
@@ -227,15 +235,24 @@ struct nsCSSValueListRect {
 
 // Maybe should be replaced with nsCSSValueList and nsCSSValue::Array?
 struct nsCSSValuePairList {
-  nsCSSValuePairList(void);
+  nsCSSValuePairList() : mNext(nsnull) { MOZ_COUNT_CTOR(nsCSSValuePairList); }
-  nsCSSValuePairList(const nsCSSValuePairList& aCopy);
+  ~nsCSSValuePairList();
-  ~nsCSSValuePairList(void);
+
+  nsCSSValuePairList* Clone() const { return Clone(PR_TRUE); }
 
   static PRBool Equal(nsCSSValuePairList* aList1, nsCSSValuePairList* aList2);
 
   nsCSSValue          mXValue;
   nsCSSValue          mYValue;
   nsCSSValuePairList* mNext;
+
+private:
+  nsCSSValuePairList(const nsCSSValuePairList& aCopy) // makes a shallow copy
+    : mXValue(aCopy.mXValue), mYValue(aCopy.mYValue), mNext(nsnull)
+  {
+    MOZ_COUNT_CTOR(nsCSSValuePairList);
+  }
+  nsCSSValuePairList* Clone(PRBool aDeep) const;
 };
 
 /****************************************************************************/

layout/style/nsCSSStyleRule.cpp

@@ -91,38 +91,10 @@
     }                                                                         \
   PR_END_MACRO
 
-#define NS_IF_DEEP_CLONE(type_, member_, args_)                               \
-  PR_BEGIN_MACRO                                                              \
-    type_ *dest = result;                                                     \
-    for (type_ *src = member_; src; src = src->member_) {                     \
-      type_ *clone = src->Clone args_;                                        \
-      if (!clone) {                                                           \
-        delete result;                                                        \
-        return nsnull;                                                        \
-      }                                                                       \
-      dest->member_ = clone;                                                  \
-      dest = clone;                                                           \
-    }                                                                         \
-  PR_END_MACRO
-
 #define NS_IF_DELETE(ptr)                                                     \
   PR_BEGIN_MACRO                                                              \
-    if (ptr) {                                                                \
     delete ptr;                                                               \
     ptr = nsnull;                                                             \
-    }                                                                         \
-  PR_END_MACRO
-
-#define NS_IF_DEEP_DELETE(type_, member_)                                     \
-  PR_BEGIN_MACRO                                                              \
-    type_ *cur = member_;                                                     \
-    member_ = nsnull;                                                         \
-    while (cur) {                                                             \
-      type_ *next = cur->member_;                                             \
-      cur->member_ = nsnull;                                                  \
-      delete cur;                                                             \
-      cur = next;                                                             \
-    }                                                                         \
   PR_END_MACRO
 
 /* ************************************************************************** */
@@ -150,14 +122,14 @@ nsAtomList::Clone(PRBool aDeep) const
     return nsnull;
 
   if (aDeep)
-    NS_IF_DEEP_CLONE(nsAtomList, mNext, (PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsAtomList, this, mNext, result, (PR_FALSE));
   return result;
 }
 
 nsAtomList::~nsAtomList(void)
 {
   MOZ_COUNT_DTOR(nsAtomList);
-  NS_IF_DEEP_DELETE(nsAtomList, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsAtomList, this, mNext);
 }
 
 nsPseudoClassList::nsPseudoClassList(nsIAtom* aAtom)
@@ -209,7 +181,8 @@ nsPseudoClassList::Clone(PRBool aDeep) const
   }
 
   if (aDeep)
-    NS_IF_DEEP_CLONE(nsPseudoClassList, mNext, (PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsPseudoClassList, this, mNext, result,
+                             (PR_FALSE));
 
   return result;
 }
@@ -219,7 +192,7 @@ nsPseudoClassList::~nsPseudoClassList(void)
   MOZ_COUNT_DTOR(nsPseudoClassList);
   if (u.mMemory)
     NS_Free(u.mMemory);
-  NS_IF_DEEP_DELETE(nsPseudoClassList, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsPseudoClassList, this, mNext);
 }
 
 nsAttrSelector::nsAttrSelector(PRInt32 aNameSpace, const nsString& aAttr)
@@ -269,7 +242,7 @@ nsAttrSelector::Clone(PRBool aDeep) const
     new nsAttrSelector(mNameSpace, mAttr, mFunction, mValue, mCaseSensitive);
 
   if (aDeep)
-    NS_IF_DEEP_CLONE(nsAttrSelector, mNext, (PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsAttrSelector, this, mNext, result, (PR_FALSE));
 
   return result;
 }
@@ -278,7 +251,7 @@ nsAttrSelector::~nsAttrSelector(void)
 {
   MOZ_COUNT_DTOR(nsAttrSelector);
 
-  NS_IF_DEEP_DELETE(nsAttrSelector, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsAttrSelector, this, mNext);
 }
 
 // -- nsCSSSelector -------------------------------
@@ -313,12 +286,16 @@ nsCSSSelector::Clone(PRBool aDeepNext, PRBool aDeepNegations) const
 
   // No need to worry about multiple levels of recursion since an
   // mNegations can't have an mNext.
+  NS_ASSERTION(!mNegations || !mNegations->mNext,
+               "mNegations can't have non-null mNext");
   if (aDeepNegations) {
-    NS_IF_DEEP_CLONE(nsCSSSelector, mNegations, (PR_TRUE, PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsCSSSelector, this, mNegations, result,
+                             (PR_TRUE, PR_FALSE));
   }
 
   if (aDeepNext) {
-    NS_IF_DEEP_CLONE(nsCSSSelector, mNext, (PR_FALSE, PR_TRUE));
+    NS_CSS_CLONE_LIST_MEMBER(nsCSSSelector, this, mNext, result,
+                             (PR_FALSE, PR_TRUE));
   }
 
   return result;
@@ -330,7 +307,7 @@ nsCSSSelector::~nsCSSSelector(void)
   Reset();
   // No need to worry about multiple levels of recursion since an
   // mNegations can't have an mNext.
-  NS_IF_DEEP_DELETE(nsCSSSelector, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSSelector, this, mNext);
 }
 
 void nsCSSSelector::Reset(void)
@@ -343,7 +320,9 @@ void nsCSSSelector::Reset(void)
   NS_IF_DELETE(mAttrList);
   // No need to worry about multiple levels of recursion since an
   // mNegations can't have an mNext.
-  NS_IF_DEEP_DELETE(nsCSSSelector, mNegations);
+  NS_ASSERTION(!mNegations || !mNegations->mNext,
+               "mNegations can't have non-null mNext");
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSSelector, this, mNegations);
   mOperator = PRUnichar(0);
 }
 
@@ -738,8 +717,8 @@ nsCSSSelectorList::nsCSSSelectorList(void)
 nsCSSSelectorList::~nsCSSSelectorList()
 {
   MOZ_COUNT_DTOR(nsCSSSelectorList);
-  NS_IF_DELETE(mSelectors);
+  delete mSelectors;
-  NS_IF_DEEP_DELETE(nsCSSSelectorList, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSSelectorList, this, mNext);
 }
 
 void nsCSSSelectorList::AddSelector(nsAutoPtr<nsCSSSelector>& aSelector)
@@ -773,7 +752,8 @@ nsCSSSelectorList::Clone(PRBool aDeep) const
   NS_IF_CLONE(mSelectors);
 
   if (aDeep) {
-    NS_IF_DEEP_CLONE(nsCSSSelectorList, mNext, (PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsCSSSelectorList, this, mNext, result,
+                             (PR_FALSE));
   }
   return result;
 }

layout/style/nsCSSValue.h

@@ -54,6 +54,37 @@ class imgIRequest;
 class nsIDocument;
 class nsIPrincipal;
 
+// Deletes a linked list iteratively to avoid blowing up the stack (bug 456196).
+#define NS_CSS_DELETE_LIST_MEMBER(type_, ptr_, member_)                        \
+  {                                                                            \
+    type_ *cur = (ptr_)->member_;                                              \
+    (ptr_)->member_ = nsnull;                                                  \
+    while (cur) {                                                              \
+      type_ *next = cur->member_;                                              \
+      cur->member_ = nsnull;                                                   \
+      delete cur;                                                              \
+      cur = next;                                                              \
+    }                                                                          \
+  }
+
+// Clones a linked list iteratively to avoid blowing up the stack.
+// If it fails to clone the entire list then 'to_' is deleted and
+// we return null.
+#define NS_CSS_CLONE_LIST_MEMBER(type_, from_, member_, to_, args_)            \
+  {                                                                            \
+    type_ *dest = (to_);                                                       \
+    (to_)->member_ = nsnull;                                                   \
+    for (const type_ *src = (from_)->member_; src; src = src->member_) {       \
+      type_ *clone = src->Clone args_;                                         \
+      if (!clone) {                                                            \
+        delete (to_);                                                          \
+        return nsnull;                                                         \
+      }                                                                        \
+      dest->member_ = clone;                                                   \
+      dest = clone;                                                            \
+    }                                                                          \
+  }
+
 enum nsCSSUnit {
   eCSSUnit_Null         = 0,      // (n/a) null unit, value is not specified
   eCSSUnit_Auto         = 1,      // (n/a) value is algorithmic

layout/style/nsStyleStruct.cpp

@@ -377,12 +377,29 @@ nsStyleBorder::nsStyleBorder(nsPresContext* aPresContext)
   mTwipsPerPixel = aPresContext->DevPixelsToAppUnits(1);
 }
 
+nsBorderColors::~nsBorderColors()
+{
+  NS_CSS_DELETE_LIST_MEMBER(nsBorderColors, this, mNext);
+}
+
+nsBorderColors*
+nsBorderColors::Clone(PRBool aDeep) const
+{
+  nsBorderColors* result = new nsBorderColors(mColor);
+  if (NS_UNLIKELY(!result))
+    return result;
+  if (aDeep)
+    NS_CSS_CLONE_LIST_MEMBER(nsBorderColors, this, mNext, result, (PR_FALSE));
+  return result;
+}
+
 nsStyleBorder::nsStyleBorder(const nsStyleBorder& aSrc)
   : mBorderRadius(aSrc.mBorderRadius),
     mBorderImageSplit(aSrc.mBorderImageSplit),
     mFloatEdge(aSrc.mFloatEdge),
     mBorderImageHFill(aSrc.mBorderImageHFill),
     mBorderImageVFill(aSrc.mBorderImageVFill),
+    mBorderColors(nsnull),
     mBoxShadow(aSrc.mBoxShadow),
     mHaveBorderImageWidth(aSrc.mHaveBorderImageWidth),
     mBorderImageWidth(aSrc.mBorderImageWidth),
@@ -391,12 +408,11 @@ nsStyleBorder::nsStyleBorder(const nsStyleBorder& aSrc)
     mBorderImage(aSrc.mBorderImage),
     mTwipsPerPixel(aSrc.mTwipsPerPixel)
 {
-  mBorderColors = nsnull;
   if (aSrc.mBorderColors) {
     EnsureBorderColors();
     for (PRInt32 i = 0; i < 4; i++)
       if (aSrc.mBorderColors[i])
-        mBorderColors[i] = aSrc.mBorderColors[i]->CopyColors();
+        mBorderColors[i] = aSrc.mBorderColors[i]->Clone();
       else
         mBorderColors[i] = nsnull;
   }

layout/style/nsStyleStruct.h

@@ -275,23 +275,11 @@ struct nsBorderColors {
   nsBorderColors* mNext;
   nscolor mColor;
 
-  nsBorderColors* CopyColors() {
+  nsBorderColors() : mNext(nsnull), mColor(NS_RGB(0,0,0)) {}
-    nsBorderColors* next = nsnull;
+  nsBorderColors(const nscolor& aColor) : mNext(nsnull), mColor(aColor) {}
-    if (mNext)
+  ~nsBorderColors();
-      next = mNext->CopyColors();
-    return new nsBorderColors(mColor, next);
-  }
 
-  nsBorderColors() :mNext(nsnull) { mColor = NS_RGB(0,0,0); }
+  nsBorderColors* Clone() const { return Clone(PR_TRUE); }
-
-  nsBorderColors(const nscolor& aColor, nsBorderColors* aNext=nsnull) {
-    mColor = aColor;
-    mNext = aNext;
-  }
-
-  ~nsBorderColors() {
-    delete mNext;
-  }
 
   static PRBool Equal(const nsBorderColors* c1,
                       const nsBorderColors* c2) {
@@ -307,6 +295,9 @@ struct nsBorderColors {
     // has more colors than another
     return !c1 && !c2;
   }
+
+private:
+  nsBorderColors* Clone(PRBool aDeep) const;
 };
 
 struct nsCSSShadowItem {