From 058bdab0e5916594a8868d0cbe792181cc0fd684 Mon Sep 17 00:00:00 2001
From: "dbaron@dbaron.org" <dbaron@dbaron.org>
Date: Mon, 27 Aug 2007 23:47:32 -0700
Subject: [PATCH] Fix accounting showing leak of CSS value types, and optimize
 CSS value constructors/destructors a bit.  b=382027  r+sr=bzbarsky  a1.9=roc

---
 layout/style/nsCSSDataBlock.cpp | 29 +++++++++++++++++++++++++----
 layout/style/nsCSSValue.cpp     | 19 ++++++++++++++-----
 layout/style/nsCSSValue.h       | 22 ++++++----------------
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/layout/style/nsCSSDataBlock.cpp b/layout/style/nsCSSDataBlock.cpp
index 3e531a38f8c6..ce2dd52a3b6c 100644
--- a/layout/style/nsCSSDataBlock.cpp
+++ b/layout/style/nsCSSDataBlock.cpp
@@ -615,23 +615,42 @@ nsCSSExpandedDataBlock::DoExpand(nsCSSCompressedDataBlock *aBlock,
         switch (nsCSSProps::kTypeTable[iProp]) {
             case eCSSType_Value: {
                 const nsCSSValue* val = ValueAtCursor(cursor);
+                nsCSSValue* dest = static_cast<nsCSSValue*>(prop);
                 NS_ASSERTION(val->GetUnit() != eCSSUnit_Null, "oops");
-                memcpy(prop, val, sizeof(nsCSSValue));
+                NS_ASSERTION(dest->GetUnit() == eCSSUnit_Null,
+                             "expanding into non-empty block");
+#ifdef NS_BUILD_REFCNT_LOGGING
+                dest->~nsCSSValue();
+#endif
+                memcpy(dest, val, sizeof(nsCSSValue));
                 cursor += CDBValueStorage_advance;
             } break;
 
             case eCSSType_Rect: {
                 const nsCSSRect* val = RectAtCursor(cursor);
+                nsCSSRect* dest = static_cast<nsCSSRect*>(prop);
                 NS_ASSERTION(val->HasValue(), "oops");
-                memcpy(prop, val, sizeof(nsCSSRect));
+                NS_ASSERTION(!dest->HasValue(),
+                             "expanding into non-empty block");
+#ifdef NS_BUILD_REFCNT_LOGGING
+                dest->~nsCSSRect();
+#endif
+                memcpy(dest, val, sizeof(nsCSSRect));
                 cursor += CDBRectStorage_advance;
             } break;
 
             case eCSSType_ValuePair: {
                 const nsCSSValuePair* val = ValuePairAtCursor(cursor);
+                nsCSSValuePair* dest = static_cast<nsCSSValuePair*>(prop);
                 NS_ASSERTION(val->mXValue.GetUnit() != eCSSUnit_Null ||
                              val->mYValue.GetUnit() != eCSSUnit_Null, "oops");
-                memcpy(prop, val, sizeof(nsCSSValuePair));
+                NS_ASSERTION(dest->mXValue.GetUnit() == eCSSUnit_Null &&
+                             dest->mYValue.GetUnit() == eCSSUnit_Null,
+                             "expanding into non-empty block");
+#ifdef NS_BUILD_REFCNT_LOGGING
+                dest->~nsCSSValuePair();
+#endif
+                memcpy(dest, val, sizeof(nsCSSValuePair));
                 cursor += CDBValuePairStorage_advance;
             } break;
 
@@ -639,8 +658,10 @@ nsCSSExpandedDataBlock::DoExpand(nsCSSCompressedDataBlock *aBlock,
             case eCSSType_CounterData:
             case eCSSType_Quotes: {
                 void* val = PointerAtCursor(cursor);
+                void** dest = static_cast<void**>(prop);
                 NS_ASSERTION(val, "oops");
-                *static_cast<void**>(prop) = val;
+                NS_ASSERTION(!*dest, "expanding into non-empty block");
+                *dest = val;
                 cursor += CDBPointerStorage_advance;
             } break;
         }
diff --git a/layout/style/nsCSSValue.cpp b/layout/style/nsCSSValue.cpp
index 977ff3678763..74d7550d6e58 100644
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -154,11 +154,6 @@ nsCSSValue::nsCSSValue(const nsCSSValue& aCopy)
   }
 }
 
-nsCSSValue::~nsCSSValue()
-{
-  Reset();
-}
-
 nsCSSValue& nsCSSValue::operator=(const nsCSSValue& aCopy)
 {
   if (this != &aCopy) {
@@ -241,6 +236,20 @@ nscoord nsCSSValue::GetLengthTwips() const
   return 0;
 }
 
+void nsCSSValue::DoReset()
+{
+  if (eCSSUnit_String <= mUnit && mUnit <= eCSSUnit_Attr) {
+    mValue.mString->Release();
+  } else if (eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Counters) {
+    mValue.mArray->Release();
+  } else if (eCSSUnit_URL == mUnit) {
+    mValue.mURL->Release();
+  } else if (eCSSUnit_Image == mUnit) {
+    mValue.mImage->Release();
+  }
+  mUnit = eCSSUnit_Null;
+}
+
 void nsCSSValue::SetIntValue(PRInt32 aValue, nsCSSUnit aUnit)
 {
   NS_ASSERTION((eCSSUnit_Integer == aUnit) ||
diff --git a/layout/style/nsCSSValue.h b/layout/style/nsCSSValue.h
index c9f7d2a5d697..e48ac82612d8 100644
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -137,10 +137,6 @@ public:
     : mUnit(aUnit)
   {
     NS_ASSERTION(aUnit <= eCSSUnit_System_Font, "not a valueless unit");
-    if (aUnit > eCSSUnit_System_Font) {
-      mUnit = eCSSUnit_Null;
-    }
-    mValue.mInt = 0;
   }
 
   nsCSSValue(PRInt32 aValue, nsCSSUnit aUnit) NS_HIDDEN;
@@ -151,7 +147,7 @@ public:
   explicit nsCSSValue(URL* aValue) NS_HIDDEN;
   explicit nsCSSValue(Image* aValue) NS_HIDDEN;
   nsCSSValue(const nsCSSValue& aCopy) NS_HIDDEN;
-  NS_CONSTRUCTOR_FASTCALL ~nsCSSValue() NS_HIDDEN;
+  ~nsCSSValue() { Reset(); }
 
   NS_HIDDEN_(nsCSSValue&)  operator=(const nsCSSValue& aCopy);
   NS_HIDDEN_(PRBool)      operator==(const nsCSSValue& aOther) const;
@@ -258,19 +254,13 @@ public:
 
   NS_HIDDEN_(void)  Reset()  // sets to null
   {
-    if (eCSSUnit_String <= mUnit && mUnit <= eCSSUnit_Attr) {
-      mValue.mString->Release();
-    } else if (eCSSUnit_Array <= mUnit && mUnit <= eCSSUnit_Counters) {
-      mValue.mArray->Release();
-    } else if (eCSSUnit_URL == mUnit) {
-      mValue.mURL->Release();
-    } else if (eCSSUnit_Image == mUnit) {
-      mValue.mImage->Release();
-    }
-    mUnit = eCSSUnit_Null;
-    mValue.mInt = 0;
+    if (mUnit != eCSSUnit_Null)
+      DoReset();
   }
+private:
+  NS_HIDDEN_(void)  DoReset();
 
+public:
   NS_HIDDEN_(void)  SetIntValue(PRInt32 aValue, nsCSSUnit aUnit);
   NS_HIDDEN_(void)  SetPercentValue(float aValue);
   NS_HIDDEN_(void)  SetFloatValue(float aValue, nsCSSUnit aUnit);