Bug 569719 part 9: remove return value from css::Declaration methods that always return NS_OK, and change GetCSSDeclaration to return a css::Declaration instead of an nsresult. r=dbaron

content/html/document/src/nsHTMLFragmentContentSink.cpp

@@ -1217,10 +1217,8 @@ nsHTMLParanoidFragmentSink::SanitizeStyleRule(nsICSSStyleRule *aRule, nsAutoStri
   aRuleText.Truncate();
   css::Declaration *style = aRule->GetDeclaration();
   if (style) {
-    nsresult rv = style->RemoveProperty(eCSSProperty_binding);
+    style->RemoveProperty(eCSSProperty_binding);
-    if (NS_SUCCEEDED(rv)) {
+    style->ToString(aRuleText);
-      style->ToString(aRuleText);
-    }
   }
 }
 

content/svg/content/src/nsSVGElement.cpp

@@ -1182,11 +1182,7 @@ MappedAttrParser::CreateStyleRule()
     return nsnull; // No mapped attributes were parsed
   }
 
-  nsCOMPtr<nsICSSStyleRule> rule;
+  nsCOMPtr<nsICSSStyleRule> rule = NS_NewCSSStyleRule(nsnull, mDecl);
-  if (NS_FAILED(NS_NewCSSStyleRule(getter_AddRefs(rule), nsnull, mDecl))) {
-    NS_WARNING("could not create style rule from mapped attributes");
-    mDecl->RuleAbort(); // deletes declaration
-  }
   mDecl = nsnull; // We no longer own the declaration -- drop our pointer to it
   return rule.forget();
 }

layout/style/Declaration.cpp

@@ -87,7 +87,7 @@ Declaration::~Declaration()
   MOZ_COUNT_DTOR(mozilla::css::Declaration);
 }
 
-nsresult
+void
 Declaration::ValueAppended(nsCSSProperty aProperty)
 {
   NS_ABORT_IF_FALSE(!nsCSSProps::IsShorthand(aProperty),
@@ -95,10 +95,9 @@ Declaration::ValueAppended(nsCSSProperty aProperty)
   // order IS important for CSS, so remove and add to the end
   mOrder.RemoveElement(aProperty);
   mOrder.AppendElement(aProperty);
-  return NS_OK;
 }
 
-nsresult
+void
 Declaration::RemoveProperty(nsCSSProperty aProperty)
 {
   nsCSSExpandedDataBlock data;
@@ -116,7 +115,6 @@ Declaration::RemoveProperty(nsCSSProperty aProperty)
   }
 
   CompressFrom(&data);
-  return NS_OK;
 }
 
 PRBool Declaration::AppendValueToString(nsCSSProperty aProperty,
@@ -158,7 +156,7 @@ PRBool Declaration::AppendValueToString(nsCSSProperty aProperty,
   return PR_TRUE;
 }
 
-nsresult
+void
 Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
 {
   aValue.Truncate(0);
@@ -166,7 +164,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
   // simple properties are easy.
   if (!nsCSSProps::IsShorthand(aProperty)) {
     AppendValueToString(aProperty, aValue);
-    return NS_OK;
+    return;
   }
 
   // DOM Level 2 Style says (when describing CSS2Properties, although
@@ -208,7 +206,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
     }
     if (!storage) {
       // Case (1) above: some subproperties not specified.
-      return NS_OK;
+      return;
     }
     nsCSSUnit unit;
     switch (nsCSSProps::kTypeTable[*p]) {
@@ -245,21 +243,21 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
   }
   if (importantCount != 0 && importantCount != totalCount) {
     // Case (3), no consistent importance.
-    return NS_OK;
+    return;
   }
   if (initialCount == totalCount) {
     // Simplify serialization below by serializing initial up-front.
     nsCSSValue(eCSSUnit_Initial).AppendToString(eCSSProperty_UNKNOWN, aValue);
-    return NS_OK;
+    return;
   }
   if (inheritCount == totalCount) {
     // Simplify serialization below by serializing inherit up-front.
     nsCSSValue(eCSSUnit_Inherit).AppendToString(eCSSProperty_UNKNOWN, aValue);
-    return NS_OK;
+    return;
   }
   if (initialCount != 0 || inheritCount != 0) {
     // Case (2): partially initial or inherit.
-    return NS_OK;
+    return;
   }
 
   nsCSSCompressedDataBlock *data = importantCount ? mImportantData : mData;
@@ -465,7 +463,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
             size->mYValue.GetUnit() != eCSSUnit_Auto) {
           // Non-default background-size, so can't be serialized as shorthand.
           aValue.Truncate();
-          return NS_OK;
+          return;
         }
         image->mValue.AppendToString(eCSSProperty_background_image, aValue);
         aValue.Append(PRUnichar(' '));
@@ -497,7 +495,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
           // shorthand.
           if (clip->mValue != origin->mValue) {
             aValue.Truncate();
-            return NS_OK;
+            return;
           }
 
           aValue.Append(PRUnichar(' '));
@@ -516,14 +514,14 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
           if (repeat || attachment || position || clip || origin || size) {
             // Uneven length lists, so can't be serialized as shorthand.
             aValue.Truncate();
-            return NS_OK;
+            return;
           }
           break;
         }
         if (!repeat || !attachment || !position || !clip || !origin || !size) {
           // Uneven length lists, so can't be serialized as shorthand.
           aValue.Truncate();
-          return NS_OK;
+          return;
         }
         aValue.Append(PRUnichar(','));
         aValue.Append(PRUnichar(' '));
@@ -581,7 +579,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
             featureSettings.GetUnit() != eCSSUnit_System_Font ||
             languageOverride.GetUnit() != eCSSUnit_System_Font) {
           // This can't be represented as a shorthand.
-          return NS_OK;
+          return;
         }
         systemFont->AppendToString(eCSSProperty__x_system_font, aValue);
       } else {
@@ -594,7 +592,7 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
             sizeAdjust.GetUnit() != eCSSUnit_None ||
             featureSettings.GetUnit() != eCSSUnit_Normal ||
             languageOverride.GetUnit() != eCSSUnit_Normal) {
-          return NS_OK;
+          return;
         }
 
         if (style.GetUnit() != eCSSUnit_Enumerated ||
@@ -704,7 +702,6 @@ Declaration::GetValue(nsCSSProperty aProperty, nsAString& aValue) const
       NS_NOTREACHED("no other shorthands");
       break;
   }
-  return NS_OK;
 }
 
 PRBool
@@ -769,7 +766,7 @@ Declaration::AppendPropertyAndValueToString(nsCSSProperty aProperty,
   aResult.AppendLiteral("; ");
 }
 
-nsresult
+void
 Declaration::ToString(nsAString& aString) const
 {
   nsCSSCompressedDataBlock *systemFontData =
@@ -862,7 +859,6 @@ Declaration::ToString(nsAString& aString) const
     // if the string is not empty, we have a trailing whitespace we should remove
     aString.Truncate(aString.Length() - 1);
   }
-  return NS_OK;
 }
 
 #ifdef DEBUG
@@ -878,7 +874,7 @@ void Declaration::List(FILE* out, PRInt32 aIndent) const
 }
 #endif
 
-nsresult
+void
 Declaration::GetNthProperty(PRUint32 aIndex, nsAString& aReturn) const
 {
   aReturn.Truncate();
@@ -888,8 +884,6 @@ Declaration::GetNthProperty(PRUint32 aIndex, nsAString& aReturn) const
       AppendASCIItoUTF16(nsCSSProps::GetStringValue(property), aReturn);
     }
   }
-
-  return NS_OK;
 }
 
 Declaration*
@@ -898,12 +892,11 @@ Declaration::Clone() const
   return new Declaration(*this);
 }
 
-PRBool
+void
 Declaration::InitializeEmpty()
 {
   NS_ASSERTION(!mData && !mImportantData, "already initialized");
   mData = nsCSSCompressedDataBlock::CreateEmptyBlock();
-  return mData != nsnull;
 }
 
 PRBool

layout/style/Declaration.h

@@ -83,11 +83,11 @@ public:
    * |mOrder| whenever a property is parsed into an expanded data block
    * for this declaration.  aProperty must not be a shorthand.
    */
-  nsresult ValueAppended(nsCSSProperty aProperty);
+  void ValueAppended(nsCSSProperty aProperty);
 
-  nsresult RemoveProperty(nsCSSProperty aProperty);
+  void RemoveProperty(nsCSSProperty aProperty);
 
-  nsresult GetValue(nsCSSProperty aProperty, nsAString& aValue) const;
+  void GetValue(nsCSSProperty aProperty, nsAString& aValue) const;
 
   PRBool HasImportantData() const { return mImportantData != nsnull; }
   PRBool GetValueIsImportant(nsCSSProperty aProperty) const;
@@ -96,9 +96,9 @@ public:
   PRUint32 Count() const {
     return mOrder.Length();
   }
-  nsresult GetNthProperty(PRUint32 aIndex, nsAString& aReturn) const;
+  void GetNthProperty(PRUint32 aIndex, nsAString& aReturn) const;
 
-  nsresult ToString(nsAString& aString) const;
+  void ToString(nsAString& aString) const;
 
   Declaration* Clone() const;
 
@@ -106,10 +106,9 @@ public:
   nsCSSCompressedDataBlock* GetImportantBlock() const { return mImportantData; }
 
   /**
-   * Initialize this declaration as holding no data.  Return false on
+   * Initialize this declaration as holding no data.  Cannot fail.
-   * out-of-memory.
    */
-  PRBool InitializeEmpty();
+  void InitializeEmpty();
 
   /**
    * Transfer all of the state from |aExpandedData| into this declaration.

layout/style/nsCSSDataBlock.cpp

@@ -560,10 +560,7 @@ nsCSSCompressedDataBlock::Destroy()
 nsCSSCompressedDataBlock::CreateEmptyBlock()
 {
     nsCSSCompressedDataBlock *result = new(0) nsCSSCompressedDataBlock();
-    if (!result)
-        return nsnull;
     result->mBlockEnd = result->Block();
-
     result->AddRef();
     return result;
 }

layout/style/nsCSSParser.cpp

@@ -1006,16 +1006,8 @@ CSSParserImpl::ParseStyleAttribute(const nsAString& aAttributeValue,
   css::Declaration* declaration = ParseDeclarationBlock(haveBraces);
   if (declaration) {
     // Create a style rule for the declaration
-    nsICSSStyleRule* rule = nsnull;
+    *aResult = NS_NewCSSStyleRule(nsnull, declaration).get();
-    nsresult rv = NS_NewCSSStyleRule(&rule, nsnull, declaration);
+  } else {
-    if (NS_FAILED(rv)) {
-      declaration->RuleAbort();
-      ReleaseScanner();
-      return rv;
-    }
-    *aResult = rule;
-  }
-  else {
     *aResult = nsnull;
   }
 
@@ -2436,13 +2428,7 @@ CSSParserImpl::ParseRuleSet(RuleAppendFunc aAppendFunc, void* aData,
 
   // Translate the selector list and declaration block into style data
 
-  nsCOMPtr<nsICSSStyleRule> rule;
+  nsCOMPtr<nsICSSStyleRule> rule = NS_NewCSSStyleRule(slist, declaration);
-  NS_NewCSSStyleRule(getter_AddRefs(rule), slist, declaration);
-  if (!rule) {
-    mScanner.SetLowLevelError(NS_ERROR_OUT_OF_MEMORY);
-    delete slist;
-    return PR_FALSE;
-  }
   rule->SetLineNumber(linenum);
   (*aAppendFunc)(rule, aData);
 

layout/style/nsCSSStyleRule.cpp

@@ -959,8 +959,7 @@ public:
 
   NS_IMETHOD GetParentRule(nsIDOMCSSRule **aParent);
   void DropReference(void);
-  virtual nsresult GetCSSDeclaration(css::Declaration **aDecl,
+  virtual mozilla::css::Declaration* GetCSSDeclaration(PRBool aAllocate);
-                                     PRBool aAllocate);
   virtual nsresult GetCSSParsingEnvironment(nsIURI** aSheetURI,
                                             nsIURI** aBaseURI,
                                             nsIPrincipal** aSheetPrincipal,
@@ -1048,18 +1047,14 @@ DOMCSSDeclarationImpl::DropReference(void)
   mRule = nsnull;
 }
 
-nsresult
+css::Declaration*
-DOMCSSDeclarationImpl::GetCSSDeclaration(css::Declaration **aDecl,
+DOMCSSDeclarationImpl::GetCSSDeclaration(PRBool aAllocate)
-                                         PRBool aAllocate)
 {
   if (mRule) {
-    *aDecl = mRule->GetDeclaration();
+    return mRule->GetDeclaration();
+  } else {
+    return nsnull;
   }
-  else {
-    *aDecl = nsnull;
-  }
-
-  return NS_OK;
 }
 
 /*
@@ -1643,17 +1638,12 @@ CSSStyleRuleImpl::SetSelectorText(const nsAString& aSelectorText)
   return NS_OK;
 }
 
-nsresult
+already_AddRefed<nsICSSStyleRule>
-NS_NewCSSStyleRule(nsICSSStyleRule** aInstancePtrResult,
+NS_NewCSSStyleRule(nsCSSSelectorList* aSelector,
-                   nsCSSSelectorList* aSelector,
                    css::Declaration* aDeclaration)
 {
   NS_PRECONDITION(aDeclaration, "must have a declaration");
   CSSStyleRuleImpl *it = new CSSStyleRuleImpl(aSelector, aDeclaration);
-  if (!it) {
+  NS_ADDREF(it);
-    return NS_ERROR_OUT_OF_MEMORY;
+  return it;
-  }
-
-  NS_ADDREF(*aInstancePtrResult = it);
-  return NS_OK;
 }

layout/style/nsComputedDOMStyle.cpp

@@ -410,11 +410,11 @@ nsComputedDOMStyle::GetPresShellForContent(nsIContent* aContent)
 // nsDOMCSSDeclaration abstract methods which should never be called
 // on a nsComputedDOMStyle object, but must be defined to avoid
 // compile errors.
-nsresult
+css::Declaration*
-nsComputedDOMStyle::GetCSSDeclaration(css::Declaration**, PRBool)
+nsComputedDOMStyle::GetCSSDeclaration(PRBool)
 {
   NS_RUNTIMEABORT("called nsComputedDOMStyle::GetCSSDeclaration");
-  return NS_ERROR_FAILURE;
+  return nsnull;
 }
 
 nsresult

layout/style/nsComputedDOMStyle.h

@@ -104,7 +104,7 @@ public:
   // nsDOMCSSDeclaration abstract methods which should never be called
   // on a nsComputedDOMStyle object, but must be defined to avoid
   // compile errors.
-  virtual nsresult GetCSSDeclaration(mozilla::css::Declaration**, PRBool);
+  virtual mozilla::css::Declaration* GetCSSDeclaration(PRBool);
   virtual nsresult DeclarationChanged();
   virtual nsIDocument* DocToUpdate();
   virtual nsresult GetCSSParsingEnvironment(nsIURI**, nsIURI**, nsIPrincipal**,

layout/style/nsDOMCSSAttrDeclaration.cpp

@@ -128,51 +128,46 @@ nsDOMCSSAttributeDeclaration::DocToUpdate()
   return mContent->GetOwnerDoc();
 }
 
-nsresult
+css::Declaration*
-nsDOMCSSAttributeDeclaration::GetCSSDeclaration(css::Declaration **aDecl,
+nsDOMCSSAttributeDeclaration::GetCSSDeclaration(PRBool aAllocate)
-                                                PRBool aAllocate)
 {
-  nsresult result = NS_OK;
+  if (!mContent)
+    return nsnull;
 
-  *aDecl = nsnull;
+  nsICSSStyleRule* cssRule;
-  if (mContent) {
-    nsICSSStyleRule* cssRule =
 #ifdef MOZ_SMIL
-      mIsSMILOverride ? mContent->GetSMILOverrideStyleRule() :
+  if (mIsSMILOverride)
+    cssRule = mContent->GetSMILOverrideStyleRule();
+  else
 #endif // MOZ_SMIL
-      mContent->GetInlineStyleRule();
+    cssRule = mContent->GetInlineStyleRule();
-    if (cssRule) {
-      *aDecl = cssRule->GetDeclaration();
-    }
-    else if (aAllocate) {
-      css::Declaration *decl = new css::Declaration();
-      if (!decl)
-        return NS_ERROR_OUT_OF_MEMORY;
-      if (!decl->InitializeEmpty()) {
-        decl->RuleAbort();
-        return NS_ERROR_OUT_OF_MEMORY;
-      }
 
-      nsCOMPtr<nsICSSStyleRule> newRule;
+  if (cssRule) {
-      result = NS_NewCSSStyleRule(getter_AddRefs(newRule), nsnull, decl);
+    return cssRule->GetDeclaration();
-      if (NS_FAILED(result)) {
+  }
-        decl->RuleAbort();
+  if (!aAllocate) {
-        return result;
+    return nsnull;
-      }
-
-      result =
-#ifdef MOZ_SMIL
-        mIsSMILOverride ?
-          mContent->SetSMILOverrideStyleRule(newRule, PR_FALSE) :
-#endif // MOZ_SMIL
-          mContent->SetInlineStyleRule(newRule, PR_FALSE);
-      if (NS_SUCCEEDED(result)) {
-        *aDecl = decl;
-      }
-    }
   }
 
-  return result;
+  // cannot fail
+  css::Declaration *decl = new css::Declaration();
+  decl->InitializeEmpty();
+  nsCOMPtr<nsICSSStyleRule> newRule = NS_NewCSSStyleRule(nsnull, decl);
+
+  // this *can* fail (inside SetAttrAndNotify, at least).
+  nsresult rv;
+#ifdef MOZ_SMIL
+  if (mIsSMILOverride)
+    rv = mContent->SetSMILOverrideStyleRule(newRule, PR_FALSE);
+  else
+#endif // MOZ_SMIL
+    rv = mContent->SetInlineStyleRule(newRule, PR_FALSE);
+
+  if (NS_FAILED(rv)) {
+    return nsnull; // the decl will be destroyed along with the style rule
+  }
+
+  return decl;
 }
 
 /*

layout/style/nsDOMCSSAttrDeclaration.h

@@ -69,8 +69,7 @@ public:
 
   // If GetCSSDeclaration returns non-null, then the decl it returns
   // is owned by our current style rule.
-  virtual nsresult GetCSSDeclaration(mozilla::css::Declaration **aDecl,
+  virtual mozilla::css::Declaration* GetCSSDeclaration(PRBool aAllocate);
-                                     PRBool aAllocate);
   virtual nsresult GetCSSParsingEnvironment(nsIURI** aSheetURI,
                                             nsIURI** aBaseURI,
                                             nsIPrincipal** aSheetPrincipal,

layout/style/nsDOMCSSDeclaration.cpp

@@ -79,15 +79,13 @@ nsDOMCSSDeclaration::GetPropertyValue(const nsCSSProperty aPropID,
   NS_PRECONDITION(aPropID != eCSSProperty_UNKNOWN,
                   "Should never pass eCSSProperty_UNKNOWN around");
 
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
-  nsresult result = GetCSSDeclaration(&decl, PR_FALSE);
 
   aValue.Truncate();
   if (decl) {
-    result = decl->GetValue(aPropID, aValue);
+    decl->GetValue(aPropID, aValue);
   }
-
+  return NS_OK;
-  return result;
 }
 
 NS_IMETHODIMP
@@ -107,9 +105,8 @@ nsDOMCSSDeclaration::SetPropertyValue(const nsCSSProperty aPropID,
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetCssText(nsAString& aCssText)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
   aCssText.Truncate();
-  GetCSSDeclaration(&decl, PR_FALSE);
 
   if (decl) {
     decl->ToString(aCssText);
@@ -121,12 +118,12 @@ nsDOMCSSDeclaration::GetCssText(nsAString& aCssText)
 NS_IMETHODIMP
 nsDOMCSSDeclaration::SetCssText(const nsAString& aCssText)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_TRUE);
-  nsresult result = GetCSSDeclaration(&decl, PR_TRUE);
   if (!decl) {
-    return result;
+    return NS_ERROR_FAILURE;
   }
 
+  nsresult result;
   nsRefPtr<css::Loader> cssLoader;
   nsCOMPtr<nsIURI> baseURI, sheetURI;
   nsCOMPtr<nsIPrincipal> sheetPrincipal;
@@ -151,19 +148,17 @@ nsDOMCSSDeclaration::SetCssText(const nsAString& aCssText)
   PRBool changed;
   result = cssParser.ParseDeclarations(aCssText, sheetURI, baseURI,
                                        sheetPrincipal, decl, &changed);
-
+  if (NS_FAILED(result) || !changed) {
-  if (NS_SUCCEEDED(result) && changed) {
+    return result;
-    result = DeclarationChanged();
   }
 
-  return result;
+  return DeclarationChanged();
 }
 
 NS_IMETHODIMP
 nsDOMCSSDeclaration::GetLength(PRUint32* aLength)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
-  nsresult result = GetCSSDeclaration(&decl, PR_FALSE);
 
   if (decl) {
     *aLength = decl->Count();
@@ -171,7 +166,7 @@ nsDOMCSSDeclaration::GetLength(PRUint32* aLength)
     *aLength = 0;
   }
 
-  return result;
+  return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -189,19 +184,18 @@ nsDOMCSSDeclaration::GetPropertyCSSValue(const nsAString& aPropertyName,
 NS_IMETHODIMP
 nsDOMCSSDeclaration::Item(PRUint32 aIndex, nsAString& aReturn)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
-  nsresult result = GetCSSDeclaration(&decl, PR_FALSE);
 
   aReturn.SetLength(0);
   if (decl) {
-    result = decl->GetNthProperty(aIndex, aReturn);
+    decl->GetNthProperty(aIndex, aReturn);
   }
 
-  return result;
+  return NS_OK;
 }
 
-NS_IMETHODIMP    
+NS_IMETHODIMP
-nsDOMCSSDeclaration::GetPropertyValue(const nsAString& aPropertyName, 
+nsDOMCSSDeclaration::GetPropertyValue(const nsAString& aPropertyName,
                                       nsAString& aReturn)
 {
   const nsCSSProperty propID = nsCSSProps::LookupProperty(aPropertyName);
@@ -209,28 +203,27 @@ nsDOMCSSDeclaration::GetPropertyValue(const nsAString& aPropertyName,
     aReturn.Truncate();
     return NS_OK;
   }
-  
+
   return GetPropertyValue(propID, aReturn);
 }
 
-NS_IMETHODIMP    
+NS_IMETHODIMP
-nsDOMCSSDeclaration::GetPropertyPriority(const nsAString& aPropertyName, 
+nsDOMCSSDeclaration::GetPropertyPriority(const nsAString& aPropertyName,
                                          nsAString& aReturn)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
-  nsresult result = GetCSSDeclaration(&decl, PR_FALSE);
 
   aReturn.Truncate();
   if (decl && decl->GetValueIsImportant(aPropertyName)) {
-    aReturn.AssignLiteral("important");    
+    aReturn.AssignLiteral("important");
   }
 
-  return result;
+  return NS_OK;
 }
 
-NS_IMETHODIMP    
+NS_IMETHODIMP
-nsDOMCSSDeclaration::SetProperty(const nsAString& aPropertyName, 
+nsDOMCSSDeclaration::SetProperty(const nsAString& aPropertyName,
-                                 const nsAString& aValue, 
+                                 const nsAString& aValue,
                                  const nsAString& aPriority)
 {
   // In the common (and fast) cases we can use the property id
@@ -267,12 +260,11 @@ nsDOMCSSDeclaration::RemoveProperty(const nsAString& aPropertyName,
     aReturn.Truncate();
     return NS_OK;
   }
-  
+
   nsresult rv = GetPropertyValue(propID, aReturn);
   NS_ENSURE_SUCCESS(rv, rv);
-  
+
-  rv = RemoveProperty(propID);
+  return RemoveProperty(propID);
-  return rv;
 }
 
 nsresult
@@ -280,12 +272,12 @@ nsDOMCSSDeclaration::ParsePropertyValue(const nsCSSProperty aPropID,
                                         const nsAString& aPropValue,
                                         PRBool aIsImportant)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_TRUE);
-  nsresult result = GetCSSDeclaration(&decl, PR_TRUE);
   if (!decl) {
-    return result;
+    return NS_ERROR_FAILURE;
   }
 
+  nsresult result;
   nsRefPtr<css::Loader> cssLoader;
   nsCOMPtr<nsIURI> baseURI, sheetURI;
   nsCOMPtr<nsIPrincipal> sheetPrincipal;
@@ -310,20 +302,19 @@ nsDOMCSSDeclaration::ParsePropertyValue(const nsCSSProperty aPropID,
   result = cssParser.ParseProperty(aPropID, aPropValue, sheetURI, baseURI,
                                    sheetPrincipal, decl, &changed,
                                    aIsImportant);
-  if (NS_SUCCEEDED(result) && changed) {
+  if (NS_FAILED(result) || !changed) {
-    result = DeclarationChanged();
+    return result;
   }
 
-  return result;
+  return DeclarationChanged();
 }
 
 nsresult
 nsDOMCSSDeclaration::RemoveProperty(const nsCSSProperty aPropID)
 {
-  css::Declaration* decl;
+  css::Declaration* decl = GetCSSDeclaration(PR_FALSE);
-  nsresult rv = GetCSSDeclaration(&decl, PR_FALSE);
   if (!decl) {
-    return rv;
+    return NS_OK; // no decl, so nothing to remove
   }
 
   // For nsDOMCSSAttributeDeclaration, DeclarationChanged will lead to
@@ -333,18 +324,8 @@ nsDOMCSSDeclaration::RemoveProperty(const nsCSSProperty aPropID)
   // rule (see stack in bug 209575).
   mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), PR_TRUE);
 
-  rv = decl->RemoveProperty(aPropID);
+  decl->RemoveProperty(aPropID);
-
+  return DeclarationChanged();
-  if (NS_SUCCEEDED(rv)) {
-    rv = DeclarationChanged();
-  } else {
-    // RemoveProperty used to throw in all sorts of situations -- e.g.
-    // if the property was a shorthand one.  Do not propagate its return
-    // value to callers.  (XXX or should we propagate it again now?)
-    rv = NS_OK;
-  }
-
-  return rv;
 }
 
 // nsIDOMCSS2Properties

layout/style/nsDOMCSSDeclaration.h

@@ -91,11 +91,10 @@ public:
   NS_DECL_NSIDOMNSCSS2PROPERTIES
 
 protected:
-  // Always fills in the out parameter, even on failure, and if the out
+  // This method can return null regardless of the value of aAllocate;
-  // parameter is null the nsresult will be the correct thing to
+  // however, a null return should only be considered a failure
-  // propagate.
+  // if aAllocate is true.
-  virtual nsresult GetCSSDeclaration(mozilla::css::Declaration **aDecl,
+  virtual mozilla::css::Declaration* GetCSSDeclaration(PRBool aAllocate) = 0;
-                                     PRBool aAllocate) = 0;
   virtual nsresult DeclarationChanged() = 0;
   // Document that we must call BeginUpdate/EndUpdate on around the
   // calls to DeclarationChanged and the style rule mutation that leads

layout/style/nsICSSStyleRule.h

@@ -339,9 +339,8 @@ public:
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsICSSStyleRule, NS_ICSS_STYLE_RULE_IID)
 
-nsresult
+already_AddRefed<nsICSSStyleRule>
-NS_NewCSSStyleRule(nsICSSStyleRule** aInstancePtrResult,
+NS_NewCSSStyleRule(nsCSSSelectorList* aSelector,
-                   nsCSSSelectorList* aSelector,
                    mozilla::css::Declaration* aDeclaration);
 
 #endif /* nsICSSStyleRule_h___ */

layout/style/nsStyleAnimation.cpp

@@ -1610,11 +1610,11 @@ BuildStyleRule(nsCSSProperty aProperty,
 {
   // Set up an empty CSS Declaration
   css::Declaration* declaration = new css::Declaration();
+  declaration->InitializeEmpty();
 
   PRBool changed; // ignored, but needed as outparam for ParseProperty
   nsIDocument* doc = aTargetElement->GetOwnerDoc();
   nsCOMPtr<nsIURI> baseURI = aTargetElement->GetBaseURI();
-  nsCOMPtr<nsICSSStyleRule> styleRule;
   nsCSSParser parser(doc->CSSLoader());
 
   if (aUseSVGMode) {
@@ -1628,26 +1628,21 @@ BuildStyleRule(nsCSSProperty aProperty,
   nsCSSProperty propertyToCheck = nsCSSProps::IsShorthand(aProperty) ?
     nsCSSProps::SubpropertyEntryFor(aProperty)[0] : aProperty;
 
-  // The next clause performs the following, in sequence: Initialize our
+  // Get a parser, parse the property, and check for CSS parsing errors.
-  // declaration, get a parser, parse property, check that parsing succeeded,
+  // If any of these steps fails, we bail out and delete the declaration.
-  // and build a rule for the resulting declaration.  If any of these steps
+  if (!parser ||
-  // fails, we bail out and delete the declaration.
-  if (!declaration->InitializeEmpty() ||
-      !parser ||
       NS_FAILED(parser.ParseProperty(aProperty, aSpecifiedValue,
                                      doc->GetDocumentURI(), baseURI,
                                      aTargetElement->NodePrincipal(),
                                      declaration, &changed, PR_FALSE)) ||
       // check whether property parsed without CSS parsing errors
-      !declaration->HasNonImportantValueFor(propertyToCheck) ||
+      !declaration->HasNonImportantValueFor(propertyToCheck)) {
-      NS_FAILED(NS_NewCSSStyleRule(getter_AddRefs(styleRule), nsnull,
-                                   declaration))) {
     NS_WARNING("failure in BuildStyleRule");
     declaration->RuleAbort();  // deletes declaration
     return nsnull;
   }
 
-  return styleRule.forget();
+  return NS_NewCSSStyleRule(nsnull, declaration);
 }
 
 inline