Bug 615977: Make nsCSSValue::BufferFromString() return an already_AddRefed pointer. r=dbaron a=roc
authorDaniel Holbert <dholbert@cs.stanford.edu>
Sun, 05 Dec 2010 13:17:29 +0000
changeset 58631 8c3e817ba0349b854dc7e66dab7e2fd97ebb9caf
parent 58630 3e68a0b2cdc47c59681f11705baa49380757ad6e
child 58632 20441229d0a7be4a7b4bf8b038f0f48b9e9164b6
push id17389
push userjwatt@jwatt.org
push dateSun, 05 Dec 2010 14:31:57 +0000
treeherdermozilla-central@20441229d0a7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron, roc
bugs615977
milestone2.0b8pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 615977: Make nsCSSValue::BufferFromString() return an already_AddRefed pointer. r=dbaron a=roc
content/html/content/src/nsGenericHTMLElement.cpp
content/smil/nsSMILMappedAttribute.cpp
layout/style/nsCSSParser.cpp
layout/style/nsCSSValue.cpp
layout/style/nsCSSValue.h
layout/style/nsStyleAnimation.cpp
--- a/content/html/content/src/nsGenericHTMLElement.cpp
+++ b/content/html/content/src/nsGenericHTMLElement.cpp
@@ -1952,26 +1952,25 @@ nsGenericHTMLElement::MapBackgroundInto(
         nsIDocument* doc = presContext->Document();
         nsCOMPtr<nsIURI> uri;
         nsresult rv = nsContentUtils::NewURIWithDocumentCharset(
             getter_AddRefs(uri), spec, doc, doc->GetDocBaseURI());
         if (NS_SUCCEEDED(rv)) {
           // Note that this should generally succeed here, due to the way
           // |spec| is created.  Maybe we should just add an nsStringBuffer
           // accessor on nsAttrValue?
-          nsStringBuffer* buffer = nsCSSValue::BufferFromString(spec);
-          if (NS_LIKELY(buffer != 0)) {
+          nsRefPtr<nsStringBuffer> buffer = nsCSSValue::BufferFromString(spec);
+          if (NS_LIKELY(buffer)) {
             // XXXbz it would be nice to assert that doc->NodePrincipal() is
             // the same as the principal of the node (which we'd need to store
             // in the mapped attrs or something?)
             nsCSSValue::Image *img =
               new nsCSSValue::Image(uri, buffer, doc->GetDocumentURI(),
                                     doc->NodePrincipal(), doc);
-            buffer->Release();
-            if (NS_LIKELY(img != 0)) {
+            if (NS_LIKELY(img)) {
               nsCSSValueList* list =
                 aData->mColorData->mBackImage.SetListValue();
               list->mValue.SetImageValue(img);
             }
           }
         }
       }
       else if (presContext->CompatibilityMode() == eCompatibility_NavQuirks) {
--- a/content/smil/nsSMILMappedAttribute.cpp
+++ b/content/smil/nsSMILMappedAttribute.cpp
@@ -121,17 +121,18 @@ nsSMILMappedAttribute::SetAnimValue(cons
   // Convert nsSMILValue to string
   nsAutoString valStr;
   if (!nsSMILCSSValueType::ValueToString(aValue, valStr)) {
     NS_WARNING("Failed to convert nsSMILValue for mapped attr into a string");
     return NS_ERROR_FAILURE;
   }
 
   // Set the string as this mapped attribute's animated value.
-  nsStringBuffer* valStrBuf = nsCSSValue::BufferFromString(nsString(valStr));
+  nsStringBuffer* valStrBuf =
+    nsCSSValue::BufferFromString(nsString(valStr)).get();
   nsRefPtr<nsIAtom> attrName = GetAttrNameAtom();
   nsresult rv = mElement->SetProperty(SMIL_MAPPED_ATTR_ANIMVAL,
                                       attrName, valStrBuf,
                                       ReleaseStringBufferPropertyValue);
   if (rv == NS_PROPTABLE_PROP_OVERWRITTEN) {
     rv = NS_OK;
   }
   FlushChangesToTargetAttr();
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -4648,25 +4648,26 @@ CSSParserImpl::SetValueToURL(nsCSSValue&
     return PR_FALSE;
   }
 
   // Translate url into an absolute url if the url is relative to the
   // style sheet.
   nsCOMPtr<nsIURI> uri;
   NS_NewURI(getter_AddRefs(uri), aURL, nsnull, mBaseURI);
 
-  nsStringBuffer* buffer = nsCSSValue::BufferFromString(aURL);
+  nsRefPtr<nsStringBuffer> buffer(nsCSSValue::BufferFromString(aURL));
   if (NS_UNLIKELY(!buffer)) {
     mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY);
     return PR_FALSE;
   }
+
+  // Note: urlVal retains its own reference to |buffer|.
   nsCSSValue::URL *urlVal =
     new nsCSSValue::URL(uri, buffer, mSheetURI, mSheetPrincipal);
 
-  buffer->Release();
   if (NS_UNLIKELY(!urlVal)) {
     mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY);
     return PR_FALSE;
   }
   aValue.SetURLValue(urlVal);
   return PR_TRUE;
 }
 
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -76,17 +76,17 @@ nsCSSValue::nsCSSValue(float aValue, nsC
   }
 }
 
 nsCSSValue::nsCSSValue(const nsString& aValue, nsCSSUnit aUnit)
   : mUnit(aUnit)
 {
   NS_ABORT_IF_FALSE(UnitHasStringValue(), "not a string value");
   if (UnitHasStringValue()) {
-    mValue.mString = BufferFromString(aValue);
+    mValue.mString = BufferFromString(aValue).get();
     if (NS_UNLIKELY(!mValue.mString)) {
       // XXXbz not much we can do here; just make sure that our promise of a
       // non-null mValue.mString holds for string units.
       mUnit = eCSSUnit_Null;
     }
   }
   else {
     mUnit = eCSSUnit_Null;
@@ -351,17 +351,17 @@ void nsCSSValue::SetFloatValue(float aVa
 
 void nsCSSValue::SetStringValue(const nsString& aValue,
                                 nsCSSUnit aUnit)
 {
   Reset();
   mUnit = aUnit;
   NS_ABORT_IF_FALSE(UnitHasStringValue(), "not a string unit");
   if (UnitHasStringValue()) {
-    mValue.mString = BufferFromString(aValue);
+    mValue.mString = BufferFromString(aValue).get();
     if (NS_UNLIKELY(!mValue.mString)) {
       // XXXbz not much we can do here; just make sure that our promise of a
       // non-null mValue.mString holds for string units.
       mUnit = eCSSUnit_Null;
     }
   } else
     mUnit = eCSSUnit_Null;
 }
@@ -593,26 +593,28 @@ nsCSSValue::EqualsFunction(nsCSSKeyword 
                     "illegally structured function value");
 
   nsCSSKeyword thisFunctionId =
     static_cast<nsCSSKeyword>(func->Item(0).GetIntValue());
   return thisFunctionId == aFunctionId;
 }
 
 // static
-nsStringBuffer*
+already_AddRefed<nsStringBuffer>
 nsCSSValue::BufferFromString(const nsString& aValue)
 {
   nsStringBuffer* buffer = nsStringBuffer::FromString(aValue);
   if (buffer) {
     buffer->AddRef();
     return buffer;
   }
   
   PRUnichar length = aValue.Length();
+
+  // NOTE: Alloc prouduces a new, already-addref'd (refcnt = 1) buffer.
   buffer = nsStringBuffer::Alloc((length + 1) * sizeof(PRUnichar));
   if (NS_LIKELY(buffer != 0)) {
     PRUnichar* data = static_cast<PRUnichar*>(buffer->Data());
     nsCharTraits<PRUnichar>::copy(data, aValue.get(), length);
     // Null-terminate.
     data[length] = 0;
   }
 
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -435,17 +435,18 @@ public:
 
   // Initializes as a function value with the specified function id.
   Array* InitFunction(nsCSSKeyword aFunctionId, PRUint32 aNumArgs);
   // Checks if this is a function value with the specified function id.
   PRBool EqualsFunction(nsCSSKeyword aFunctionId) const;
 
   // Returns an already addrefed buffer.  Can return null on allocation
   // failure.
-  static nsStringBuffer* BufferFromString(const nsString& aValue);
+  static already_AddRefed<nsStringBuffer>
+    BufferFromString(const nsString& aValue);
 
   struct URL {
     // Methods are not inline because using an nsIPrincipal means requiring
     // caps, which leads to REQUIRES hell, since this header is included all
     // over.
 
     // aString must not be null.
     // aOriginPrincipal must not be null.
--- a/layout/style/nsStyleAnimation.cpp
+++ b/layout/style/nsStyleAnimation.cpp
@@ -2839,17 +2839,17 @@ nsStyleAnimation::Value::SetColorValue(n
   mValue.mColor = aColor;
 }
 
 void
 nsStyleAnimation::Value::SetUnparsedStringValue(const nsString& aString)
 {
   FreeValue();
   mUnit = eUnit_UnparsedString;
-  mValue.mString = nsCSSValue::BufferFromString(aString);
+  mValue.mString = nsCSSValue::BufferFromString(aString).get();
   if (NS_UNLIKELY(!mValue.mString)) {
     // not much we can do here; just make sure that our promise of a
     // non-null mValue.mString holds for string units.
     mUnit = eUnit_Null;
   }
 }
 
 void