Bug 571635 - Make nsCSSStyleSheet::GetStyleRuleAt return a css::Rule*. r=dbaron
authorBirunthan Mohanathas <birunthan@mohanathas.com>
Sun, 07 Jul 2013 16:23:13 -0400
changeset 137910 1f030ce15fdb9f32304f4a8f5bc1b4006a0adc45
parent 137909 307ab52a74fc70a6999cf0aa34963c1fe2823390
child 137911 c69ab8cdad984534089e5669eed37495d5db89b2
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersdbaron
bugs571635
milestone25.0a1
Bug 571635 - Make nsCSSStyleSheet::GetStyleRuleAt return a css::Rule*. r=dbaron
content/base/src/nsTreeSanitizer.cpp
layout/style/nsCSSParser.cpp
layout/style/nsCSSStyleSheet.cpp
layout/style/nsCSSStyleSheet.h
--- a/content/base/src/nsTreeSanitizer.cpp
+++ b/content/base/src/nsTreeSanitizer.cpp
@@ -1106,20 +1106,19 @@ nsTreeSanitizer::SanitizeStyleSheet(cons
   NS_ENSURE_SUCCESS(rv, true);
   // Mark the sheet as complete.
   NS_ABORT_IF_FALSE(!sheet->IsModified(),
       "should not get marked modified during parsing");
   sheet->SetComplete();
   // Loop through all the rules found in the CSS text
   int32_t ruleCount = sheet->StyleRuleCount();
   for (int32_t i = 0; i < ruleCount; ++i) {
-    nsRefPtr<mozilla::css::Rule> rule;
-    rv = sheet->GetStyleRuleAt(i, *getter_AddRefs(rule));
-    if (NS_FAILED(rv))
-      continue; NS_ASSERTION(rule, "We should have a rule by now");
+    mozilla::css::Rule* rule = sheet->GetStyleRuleAt(i);
+    if (!rule)
+      continue;
     switch (rule->GetType()) {
       default:
         didSanitize = true;
         // Ignore these rule types.
         break;
       case mozilla::css::Rule::NAMESPACE_RULE:
       case mozilla::css::Rule::FONT_FACE_RULE: {
         // Append @namespace and @font-face rules verbatim.
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -947,32 +947,30 @@ CSSParserImpl::ParseSheet(const nsAStrin
 #endif
 
   nsCSSScanner scanner(aInput, aLineNumber);
   css::ErrorReporter reporter(scanner, mSheet, mChildLoader, aSheetURI);
   InitScanner(scanner, reporter, aSheetURI, aBaseURI, aSheetPrincipal);
 
   int32_t ruleCount = mSheet->StyleRuleCount();
   if (0 < ruleCount) {
-    css::Rule* lastRule = nullptr;
-    mSheet->GetStyleRuleAt(ruleCount - 1, lastRule);
+    const css::Rule* lastRule = mSheet->GetStyleRuleAt(ruleCount - 1);
     if (lastRule) {
       switch (lastRule->GetType()) {
         case css::Rule::CHARSET_RULE:
         case css::Rule::IMPORT_RULE:
           mSection = eCSSSection_Import;
           break;
         case css::Rule::NAMESPACE_RULE:
           mSection = eCSSSection_NameSpace;
           break;
         default:
           mSection = eCSSSection_General;
           break;
       }
-      NS_RELEASE(lastRule);
     }
   }
   else {
     mSection = eCSSSection_Charset; // sheet is empty, any rules are fair
   }
 
   mUnsafeRulesEnabled = aAllowUnsafeRules;
 
--- a/layout/style/nsCSSStyleSheet.cpp
+++ b/layout/style/nsCSSStyleSheet.cpp
@@ -107,36 +107,30 @@ CSSRuleListImpl::GetLength(uint32_t* aLe
   }
 
   return NS_OK;
 }
 
 nsIDOMCSSRule*    
 CSSRuleListImpl::GetItemAt(uint32_t aIndex, nsresult* aResult)
 {
-  nsresult result = NS_OK;
-
   if (mStyleSheet) {
     // ensure rules have correct parent
     if (mStyleSheet->EnsureUniqueInner() !=
           nsCSSStyleSheet::eUniqueInner_CloneFailed) {
-      nsRefPtr<css::Rule> rule;
-
-      result = mStyleSheet->GetStyleRuleAt(aIndex, *getter_AddRefs(rule));
+      css::Rule* rule = mStyleSheet->GetStyleRuleAt(aIndex);
       if (rule) {
         *aResult = NS_OK;
         return rule->GetDOMRule();
       }
-      if (result == NS_ERROR_ILLEGAL_VALUE) {
-        result = NS_OK; // per spec: "Return Value ... null if ... not a valid index."
-      }
     }
   }
 
-  *aResult = result;
+  // Per spec: "Return Value ... null if ... not a valid index."
+  *aResult = NS_OK;
   return nullptr;
 }
 
 NS_IMETHODIMP    
 CSSRuleListImpl::Item(uint32_t aIndex, nsIDOMCSSRule** aReturn)
 {
   nsresult rv;
   nsIDOMCSSRule* rule = GetItemAt(aIndex, &rv);
@@ -1497,28 +1491,22 @@ nsCSSStyleSheet::ReplaceStyleRule(css::R
 }
 
 int32_t
 nsCSSStyleSheet::StyleRuleCount() const
 {
   return mInner->mOrderedRules.Count();
 }
 
-nsresult
-nsCSSStyleSheet::GetStyleRuleAt(int32_t aIndex, css::Rule*& aRule) const
+css::Rule*
+nsCSSStyleSheet::GetStyleRuleAt(int32_t aIndex) const
 {
   // Important: If this function is ever made scriptable, we must add
   // a security check here. See GetCssRules below for an example.
-  aRule = mInner->mOrderedRules.SafeObjectAt(aIndex);
-  if (aRule) {
-    NS_ADDREF(aRule);
-    return NS_OK;
-  }
-
-  return NS_ERROR_ILLEGAL_VALUE;
+  return mInner->mOrderedRules.SafeObjectAt(aIndex);
 }
 
 int32_t
 nsCSSStyleSheet::StyleSheetCount() const
 {
   // XXX Far from an ideal way to do this, but the hope is that
   // it won't be done too often. If it is, we might want to 
   // consider storing the children in an array.
--- a/layout/style/nsCSSStyleSheet.h
+++ b/layout/style/nsCSSStyleSheet.h
@@ -142,17 +142,17 @@ public:
   void InsertStyleSheetAt(nsCSSStyleSheet* aSheet, int32_t aIndex);
 
   // XXX do these belong here or are they generic?
   void PrependStyleRule(mozilla::css::Rule* aRule);
   void AppendStyleRule(mozilla::css::Rule* aRule);
   void ReplaceStyleRule(mozilla::css::Rule* aOld, mozilla::css::Rule* aNew);
 
   int32_t StyleRuleCount() const;
-  nsresult GetStyleRuleAt(int32_t aIndex, mozilla::css::Rule*& aRule) const;
+  mozilla::css::Rule* GetStyleRuleAt(int32_t aIndex) const;
 
   nsresult DeleteRuleFromGroup(mozilla::css::GroupRule* aGroup, uint32_t aIndex);
   nsresult InsertRuleIntoGroup(const nsAString& aRule, mozilla::css::GroupRule* aGroup, uint32_t aIndex, uint32_t* _retval);
   nsresult ReplaceRuleInGroup(mozilla::css::GroupRule* aGroup, mozilla::css::Rule* aOld, mozilla::css::Rule* aNew);
 
   int32_t StyleSheetCount() const;
 
   /**