Use iterative algorithms when cloning and deleting lists. b=456196 r+sr=dbaron
authorMats Palmgren <mats.palmgren@bredband.net>
Fri, 17 Oct 2008 10:13:16 +0200
changeset 20563 44687f34236ccd4523b3fa00930feb31d52a48aa
parent 20562 58933c1b499492af702aa9714d53d2431869a89f
child 20564 8d0e5b53857fc284f74815b9a2829e7e9535e65a
push id2968
push usermpalmgren@mozilla.com
push dateFri, 17 Oct 2008 08:14:18 +0000
treeherdermozilla-central@8d0e5b53857f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs456196
milestone1.9.1b2pre
Use iterative algorithms when cloning and deleting lists. b=456196 r+sr=dbaron
layout/style/nsCSSDataBlock.cpp
layout/style/nsCSSStruct.cpp
layout/style/nsCSSStruct.h
layout/style/nsCSSStyleRule.cpp
layout/style/nsCSSValue.h
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -465,20 +465,20 @@ nsCSSCompressedDataBlock::Clone() const
                 void *copy;
                 NS_ASSERTION(PointerAtCursor(cursor), "oops");
                 switch (nsCSSProps::kTypeTable[iProp]) {
                     default:
                         NS_NOTREACHED("unreachable");
                         // 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) {
                     result->mBlockEnd = result_cursor;
                     result->Destroy();
                     return nsnull;
                 }
                 PointerAtCursor(result_cursor) = copy;
--- a/layout/style/nsCSSStruct.cpp
+++ b/layout/style/nsCSSStruct.cpp
@@ -53,54 +53,45 @@
 #include "nsFont.h"
 
 #include "nsStyleConsts.h"
 
 #include "nsCOMPtr.h"
 #include "nsReadableUtils.h"
 #include "nsPrintfCString.h"
 
-#define CSS_IF_DELETE(ptr)  if (nsnull != ptr)  { delete ptr; ptr = nsnull; }
-
 // --- nsCSSFont -----------------
 
 nsCSSFont::nsCSSFont(void)
 {
   MOZ_COUNT_CTOR(nsCSSFont);
 }
 
 nsCSSFont::~nsCSSFont(void)
 {
   MOZ_COUNT_DTOR(nsCSSFont);
 }
 
-// --- support -----------------
-
-#define CSS_IF_COPY(val, type) \
-  if (aCopy.val) (val) = new type(*(aCopy.val));
+// --- nsCSSValueList -----------------
 
-nsCSSValueList::nsCSSValueList(void)
-  : mValue(),
-    mNext(nsnull)
+nsCSSValueList::~nsCSSValueList()
 {
-  MOZ_COUNT_CTOR(nsCSSValueList);
+  MOZ_COUNT_DTOR(nsCSSValueList);
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSValueList, this, mNext);
 }
 
-nsCSSValueList::nsCSSValueList(const nsCSSValueList& aCopy)
-  : mValue(aCopy.mValue),
-    mNext(nsnull)
+nsCSSValueList*
+nsCSSValueList::Clone(PRBool aDeep) const
 {
-  MOZ_COUNT_CTOR(nsCSSValueList);
-  CSS_IF_COPY(mNext, nsCSSValueList);
-}
-
-nsCSSValueList::~nsCSSValueList(void)
-{
-  MOZ_COUNT_DTOR(nsCSSValueList);
-  CSS_IF_DELETE(mNext);
+  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
 nsCSSValueList::Equal(nsCSSValueList* aList1, nsCSSValueList* aList2)
 {
   if (aList1 == aList2)
     return PR_TRUE;
     
@@ -130,17 +121,17 @@ nsCSSText::nsCSSText(void)
   : mTextShadow(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSText);
 }
 
 nsCSSText::~nsCSSText(void)
 {
   MOZ_COUNT_DTOR(nsCSSText);
-  CSS_IF_DELETE(mTextShadow);
+  delete mTextShadow;
 }
 
 // --- nsCSSRect -----------------
 
 nsCSSRect::nsCSSRect(void)
 {
   MOZ_COUNT_CTOR(nsCSSRect);
 }
@@ -203,17 +194,18 @@ void
 nsCSSCornerSizes::SetAllCornersTo(const nsCSSValue& aValue)
 {
   NS_FOR_CSS_FULL_CORNERS(corner) {
     this->GetFullCorner(corner).SetBothValuesTo(aValue);
   }
 }
 
 void
-nsCSSCornerSizes::Reset() {
+nsCSSCornerSizes::Reset()
+{
   NS_FOR_CSS_FULL_CORNERS(corner) {
     this->GetFullCorner(corner).Reset();
   }
 }
 
 #if NS_CORNER_TOP_LEFT != 0 || NS_CORNER_TOP_RIGHT != 1 || \
     NS_CORNER_BOTTOM_RIGHT != 2 || NS_CORNER_BOTTOM_LEFT != 3
 #error "Somebody changed the corner constants."
@@ -279,17 +271,17 @@ nsCSSMargin::nsCSSMargin(void)
   : mBoxShadow(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSMargin);
 }
 
 nsCSSMargin::~nsCSSMargin(void)
 {
   MOZ_COUNT_DTOR(nsCSSMargin);
-  CSS_IF_DELETE(mBoxShadow);
+  delete mBoxShadow;
 }
 
 // --- nsCSSPosition -----------------
 
 nsCSSPosition::nsCSSPosition(void)
 {
   MOZ_COUNT_CTOR(nsCSSPosition);
 }
@@ -344,35 +336,32 @@ nsCSSPage::nsCSSPage(void)
 
 nsCSSPage::~nsCSSPage(void)
 {
   MOZ_COUNT_DTOR(nsCSSPage);
 }
 
 // --- 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
 nsCSSValuePairList::Equal(nsCSSValuePairList* aList1,
                           nsCSSValuePairList* aList2)
 {
   if (aList1 == aList2)
     return PR_TRUE;
@@ -395,34 +384,34 @@ nsCSSContent::nsCSSContent(void)
     mQuotes(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSContent);
 }
 
 nsCSSContent::~nsCSSContent(void)
 {
   MOZ_COUNT_DTOR(nsCSSContent);
-  CSS_IF_DELETE(mContent);
-  CSS_IF_DELETE(mCounterIncrement);
-  CSS_IF_DELETE(mCounterReset);
-  CSS_IF_DELETE(mQuotes);
+  delete mContent;
+  delete mCounterIncrement;
+  delete mCounterReset;
+  delete mQuotes;
 }
 
 // --- nsCSSUserInterface -----------------
 
 nsCSSUserInterface::nsCSSUserInterface(void)
   : mCursor(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSUserInterface);
 }
 
 nsCSSUserInterface::~nsCSSUserInterface(void)
 {
   MOZ_COUNT_DTOR(nsCSSUserInterface);
-  CSS_IF_DELETE(mCursor);
+  delete mCursor;
 }
 
 // --- nsCSSAural -----------------
 
 nsCSSAural::nsCSSAural(void)
 {
   MOZ_COUNT_CTOR(nsCSSAural);
 }
@@ -462,12 +451,12 @@ nsCSSColumn::~nsCSSColumn(void)
 nsCSSSVG::nsCSSSVG(void) : mStrokeDasharray(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSSVG);
 }
 
 nsCSSSVG::~nsCSSSVG(void)
 {
   MOZ_COUNT_DTOR(nsCSSSVG);
-  CSS_IF_DELETE(mStrokeDasharray);
+  delete mStrokeDasharray;
 }
 
 #endif // MOZ_SVG
--- a/layout/style/nsCSSStruct.h
+++ b/layout/style/nsCSSStruct.h
@@ -42,28 +42,36 @@
  * representation of complex values for CSS properties
  */
 
 #ifndef nsCSSStruct_h___
 #define nsCSSStruct_h___
 
 #include "nsCSSValue.h"
 #include "nsStyleConsts.h"
-#include <stdio.h>
 
 // Prefer nsCSSValue::Array for lists of fixed size.
 struct nsCSSValueList {
-  nsCSSValueList(void);
-  nsCSSValueList(const nsCSSValueList& aCopy);
-  ~nsCSSValueList(void);
+  nsCSSValueList() : mNext(nsnull) { MOZ_COUNT_CTOR(nsCSSValueList); }
+  ~nsCSSValueList();
+
+  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 {
   nsCSSRect(void);
   nsCSSRect(const nsCSSRect& aCopy);
   ~nsCSSRect();
 
   PRBool operator==(const nsCSSRect& aOther) const {
@@ -222,25 +230,34 @@ struct nsCSSValueListRect {
   nsCSSValueList* mLeft;
 
   typedef nsCSSValueList* nsCSSValueListRect::*side_type;
   static const side_type sides[4];
 };
 
 // Maybe should be replaced with nsCSSValueList and nsCSSValue::Array?
 struct nsCSSValuePairList {
-  nsCSSValuePairList(void);
-  nsCSSValuePairList(const nsCSSValuePairList& aCopy);
-  ~nsCSSValuePairList(void);
+  nsCSSValuePairList() : mNext(nsnull) { MOZ_COUNT_CTOR(nsCSSValuePairList); }
+  ~nsCSSValuePairList();
+
+  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;
 };
 
 /****************************************************************************/
 
 struct nsCSSStruct {
   // EMPTY on purpose.  ABSTRACT with no virtuals (typedef void nsCSSStruct?)
 };
 
--- a/layout/style/nsCSSStyleRule.cpp
+++ b/layout/style/nsCSSStyleRule.cpp
@@ -86,48 +86,20 @@
       result->member_ = member_->Clone();                                     \
       if (!result->member_) {                                                 \
         delete result;                                                        \
         return nsnull;                                                        \
       }                                                                       \
     }                                                                         \
   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;                                                             \
-    }                                                                         \
+    delete ptr;                                                               \
+    ptr = nsnull;                                                             \
   PR_END_MACRO
 
 /* ************************************************************************** */
 
 nsAtomList::nsAtomList(nsIAtom* aAtom)
   : mAtom(aAtom),
     mNext(nsnull)
 {
@@ -145,24 +117,24 @@ nsAtomList::nsAtomList(const nsString& a
 nsAtomList*
 nsAtomList::Clone(PRBool aDeep) const
 {
   nsAtomList *result = new nsAtomList(mAtom);
   if (!result)
     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)
   : mAtom(aAtom),
     mNext(nsnull)
 {
   NS_ASSERTION(!nsCSSPseudoClasses::HasStringArg(aAtom) &&
                !nsCSSPseudoClasses::HasNthPairArg(aAtom),
@@ -204,27 +176,28 @@ nsPseudoClassList::Clone(PRBool aDeep) c
     result = new nsPseudoClassList(mAtom, u.mString);
   } else {
     NS_ASSERTION(nsCSSPseudoClasses::HasNthPairArg(mAtom),
                  "unexpected pseudo-class");
     result = new nsPseudoClassList(mAtom, u.mNumbers);
   }
 
   if (aDeep)
-    NS_IF_DEEP_CLONE(nsPseudoClassList, mNext, (PR_FALSE));
+    NS_CSS_CLONE_LIST_MEMBER(nsPseudoClassList, this, mNext, result,
+                             (PR_FALSE));
 
   return result;
 }
 
 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)
   : mNameSpace(aNameSpace),
     mAttr(nsnull),
     mFunction(NS_ATTR_FUNC_SET),
     mCaseSensitive(1),
     mValue(),
@@ -264,26 +237,26 @@ nsAttrSelector::nsAttrSelector(PRInt32 a
 
 nsAttrSelector*
 nsAttrSelector::Clone(PRBool aDeep) const
 {
   nsAttrSelector *result =
     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;
 }
 
 nsAttrSelector::~nsAttrSelector(void)
 {
   MOZ_COUNT_DTOR(nsAttrSelector);
 
-  NS_IF_DEEP_DELETE(nsAttrSelector, mNext);
+  NS_CSS_DELETE_LIST_MEMBER(nsAttrSelector, this, mNext);
 }
 
 // -- nsCSSSelector -------------------------------
 
 nsCSSSelector::nsCSSSelector(void)
   : mNameSpace(kNameSpaceID_Unknown), mTag(nsnull), 
     mIDList(nsnull), 
     mClassList(nsnull), 
@@ -308,47 +281,53 @@ nsCSSSelector::Clone(PRBool aDeepNext, P
   
   NS_IF_CLONE(mIDList);
   NS_IF_CLONE(mClassList);
   NS_IF_CLONE(mPseudoClassList);
   NS_IF_CLONE(mAttrList);
 
   // 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;
 }
 
 nsCSSSelector::~nsCSSSelector(void)  
 {
   MOZ_COUNT_DTOR(nsCSSSelector);
   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)
 {
   mNameSpace = kNameSpaceID_Unknown;
   mTag = nsnull;
   NS_IF_DELETE(mIDList);
   NS_IF_DELETE(mClassList);
   NS_IF_DELETE(mPseudoClassList);
   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);
 }
 
 void nsCSSSelector::SetNameSpace(PRInt32 aNameSpace)
 {
   mNameSpace = aNameSpace;
 }
 
@@ -733,18 +712,18 @@ nsCSSSelectorList::nsCSSSelectorList(voi
     mNext(nsnull)
 {
   MOZ_COUNT_CTOR(nsCSSSelectorList);
 }
 
 nsCSSSelectorList::~nsCSSSelectorList()
 {
   MOZ_COUNT_DTOR(nsCSSSelectorList);
-  NS_IF_DELETE(mSelectors);
-  NS_IF_DEEP_DELETE(nsCSSSelectorList, mNext);
+  delete mSelectors;
+  NS_CSS_DELETE_LIST_MEMBER(nsCSSSelectorList, this, mNext);
 }
 
 void nsCSSSelectorList::AddSelector(nsAutoPtr<nsCSSSelector>& aSelector)
 { // prepend to list
   nsCSSSelector* newSel = aSelector.forget();
   if (newSel) {
     newSel->mNext = mSelectors;
     mSelectors = newSel;
@@ -768,17 +747,18 @@ nsCSSSelectorList::ToString(nsAString& a
 nsCSSSelectorList*
 nsCSSSelectorList::Clone(PRBool aDeep) const
 {
   nsCSSSelectorList *result = new nsCSSSelectorList();
   result->mWeight = mWeight;
   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;
 }
 
 // -- CSSImportantRule -------------------------------
 
 class CSSStyleRuleImpl;
 
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -49,16 +49,47 @@
 #include "nsAutoPtr.h"
 #include "nsCRTGlue.h"
 #include "nsStringBuffer.h"
 
 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
   eCSSUnit_Inherit      = 2,      // (n/a) value is inherited
   eCSSUnit_Initial      = 3,      // (n/a) value is default UA value
   eCSSUnit_None         = 4,      // (n/a) value is none
   eCSSUnit_Normal       = 5,      // (n/a) value is normal (algorithmic, different than auto)
   eCSSUnit_System_Font  = 6,      // (n/a) value is -moz-use-system-font
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -372,36 +372,52 @@ nsStyleBorder::nsStyleBorder(nsPresConte
   mBorderColors = nsnull;
   mBoxShadow = nsnull;
 
   mFloatEdge = NS_STYLE_FLOAT_EDGE_CONTENT;
 
   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),
     mComputedBorder(aSrc.mComputedBorder),
     mBorder(aSrc.mBorder),
     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;
   }
 
   NS_FOR_CSS_SIDES(side) {
     mBorderStyle[side] = aSrc.mBorderStyle[side];
     mBorderColor[side] = aSrc.mBorderColor[side];
   }
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -270,48 +270,39 @@ protected:
   PRPackedBool  mHasCachedPadding;
   nsMargin      mCachedPadding;
 };
 
 struct nsBorderColors {
   nsBorderColors* mNext;
   nscolor mColor;
 
-  nsBorderColors* CopyColors() {
-    nsBorderColors* next = nsnull;
-    if (mNext)
-      next = mNext->CopyColors();
-    return new nsBorderColors(mColor, next);
-  }
-
-  nsBorderColors() :mNext(nsnull) { mColor = NS_RGB(0,0,0); }
+  nsBorderColors() : mNext(nsnull), mColor(NS_RGB(0,0,0)) {}
+  nsBorderColors(const nscolor& aColor) : mNext(nsnull), mColor(aColor) {}
+  ~nsBorderColors();
 
-  nsBorderColors(const nscolor& aColor, nsBorderColors* aNext=nsnull) {
-    mColor = aColor;
-    mNext = aNext;
-  }
-
-  ~nsBorderColors() {
-    delete mNext;
-  }
+  nsBorderColors* Clone() const { return Clone(PR_TRUE); }
 
   static PRBool Equal(const nsBorderColors* c1,
                       const nsBorderColors* c2) {
     if (c1 == c2)
       return PR_TRUE;
     while (c1 && c2) {
       if (c1->mColor != c2->mColor)
         return PR_FALSE;
       c1 = c1->mNext;
       c2 = c2->mNext;
     }
     // both should be NULL if these are equal, otherwise one
     // has more colors than another
     return !c1 && !c2;
   }
+
+private:
+  nsBorderColors* Clone(PRBool aDeep) const;
 };
 
 struct nsCSSShadowItem {
   nscoord mXOffset;
   nscoord mYOffset;
   nscoord mRadius;
   nscoord mSpread;