Bug 577974 - Part 4: DeCOM and de-virtualize methods in GroupRule, r=bzbarsky
authorCraig Topper <craig.topper@gmail.com>
Sat, 07 Aug 2010 22:29:28 -0700
changeset 64679 7e05a50ed66bbf83c8b124472dc70fbebe93836a
parent 64678 8bd0c633031859162173a911b2b15a722a86e294
child 64680 592aacaf042d61e425d8cf05d4339b6a0bb2b1d3
push idunknown
push userunknown
push dateunknown
reviewersbzbarsky
bugs577974
milestone2.2a1pre
Bug 577974 - Part 4: DeCOM and de-virtualize methods in GroupRule, r=bzbarsky
layout/style/GroupRule.h
layout/style/nsCSSRules.cpp
layout/style/nsCSSRules.h
layout/style/nsCSSStyleSheet.cpp
--- a/layout/style/GroupRule.h
+++ b/layout/style/GroupRule.h
@@ -78,47 +78,46 @@ public:
   DECL_STYLE_RULE_INHERIT_NO_DOMRULE
 
   // to help implement nsIStyleRule
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, PRInt32 aIndent = 0) const;
 #endif
 
 public:
-  NS_IMETHOD  AppendStyleRule(nsICSSRule* aRule);
+  void AppendStyleRule(nsICSSRule* aRule);
 
-  NS_IMETHOD  StyleRuleCount(PRInt32& aCount) const;
-  NS_IMETHOD  GetStyleRuleAt(PRInt32 aIndex, nsICSSRule*& aRule) const;
+  PRInt32 StyleRuleCount() const { return mRules.Count(); }
+  nsICSSRule* GetStyleRuleAt(PRInt32 aIndex) const;
 
   typedef nsCOMArray<nsICSSRule>::nsCOMArrayEnumFunc RuleEnumFunc;
-  NS_IMETHOD_(PRBool) EnumerateRulesForwards(RuleEnumFunc aFunc, void * aData) const;
+  PRBool EnumerateRulesForwards(RuleEnumFunc aFunc, void * aData) const;
 
   /*
    * The next three methods should never be called unless you have first
    * called WillDirty() on the parent stylesheet.  After they are
    * called, DidDirty() needs to be called on the sheet.
    */
-  NS_IMETHOD  DeleteStyleRuleAt(PRUint32 aIndex);
-  NS_IMETHOD  InsertStyleRulesAt(PRUint32 aIndex,
-                                 nsCOMArray<nsICSSRule>& aRules);
-  NS_IMETHOD  ReplaceStyleRule(nsICSSRule *aOld, nsICSSRule *aNew);
+  nsresult DeleteStyleRuleAt(PRUint32 aIndex);
+  nsresult InsertStyleRulesAt(PRUint32 aIndex,
+                              nsCOMArray<nsICSSRule>& aRules);
+  nsresult ReplaceStyleRule(nsICSSRule *aOld, nsICSSRule *aNew);
 
-  NS_IMETHOD_(PRBool) UseForPresentation(nsPresContext* aPresContext,
-                                         nsMediaQueryResultCacheKey& aKey) = 0;
+  virtual PRBool UseForPresentation(nsPresContext* aPresContext,
+                                    nsMediaQueryResultCacheKey& aKey) = 0;
 
 protected:
   // to help implement nsIDOMCSSRule
   nsresult AppendRulesToCssText(nsAString& aCssText);
   // to implement methods on nsIDOMCSSRule
-  nsresult GetParentStyleSheet(nsIDOMCSSStyleSheet** aSheet);
   nsresult GetParentRule(nsIDOMCSSRule** aParentRule);
 
   // to implement common methods on nsIDOMCSSMediaRule and
   // nsIDOMCSSMozDocumentRule
-  nsresult GetCssRules(nsIDOMCSSRuleList* *aRuleList);
+  nsIDOMCSSRuleList* GetCssRules();
   nsresult InsertRule(const nsAString & aRule, PRUint32 aIndex,
                       PRUint32* _retval);
   nsresult DeleteRule(PRUint32 aIndex);
 
   nsCOMArray<nsICSSRule> mRules;
   nsRefPtr<GroupRuleRuleList> mRuleCollection; // lazily constructed
 };
 
--- a/layout/style/nsCSSRules.cpp
+++ b/layout/style/nsCSSRules.cpp
@@ -131,45 +131,36 @@ NS_INTERFACE_MAP_END
 
 NS_IMPL_ADDREF(GroupRuleRuleList)
 NS_IMPL_RELEASE(GroupRuleRuleList)
 
 NS_IMETHODIMP
 GroupRuleRuleList::GetLength(PRUint32* aLength)
 {
   if (mGroupRule) {
-    PRInt32 count;
-    mGroupRule->StyleRuleCount(count);
-    *aLength = (PRUint32)count;
+    *aLength = (PRUint32)mGroupRule->StyleRuleCount();
   } else {
     *aLength = 0;
   }
 
   return NS_OK;
 }
 
 nsIDOMCSSRule*
 GroupRuleRuleList::GetItemAt(PRUint32 aIndex, nsresult* aResult)
 {
-  nsresult result = NS_OK;
+  *aResult = NS_OK;
 
   if (mGroupRule) {
-    nsCOMPtr<nsICSSRule> rule;
-
-    result = mGroupRule->GetStyleRuleAt(aIndex, *getter_AddRefs(rule));
+    nsCOMPtr<nsICSSRule> rule = mGroupRule->GetStyleRuleAt(aIndex);
     if (rule) {
       return rule->GetDOMRuleWeak(aResult);
     }
-    if (result == NS_ERROR_ILLEGAL_VALUE) {
-      result = NS_OK; // per spec: "Return Value ... null if ... not a valid index."
-    }
   }
 
-  *aResult = result;
-
   return nsnull;
 }
 
 NS_IMETHODIMP
 GroupRuleRuleList::Item(PRUint32 aIndex, nsIDOMCSSRule** aReturn)
 {
   nsresult rv;
   nsIDOMCSSRule* rule = GetItemAt(aIndex, &rv);
@@ -643,86 +634,72 @@ GroupRule::List(FILE* out, PRInt32 aInde
 
   for (PRInt32 index = 0, count = mRules.Count(); index < count; ++index) {
     mRules.ObjectAt(index)->List(out, aIndent + 1);
   }
   fputs("}\n", out);
 }
 #endif
 
-NS_IMETHODIMP
+void
 GroupRule::AppendStyleRule(nsICSSRule* aRule)
 {
   mRules.AppendObject(aRule);
   aRule->SetStyleSheet(mSheet);
   aRule->SetParentRule(this);
   if (mSheet) {
     // XXXldb Shouldn't we be using |WillDirty| and |DidDirty| (and
     // shouldn't |SetModified| be removed?
     mSheet->SetModified(PR_TRUE);
   }
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-GroupRule::StyleRuleCount(PRInt32& aCount) const
-{
-  aCount = mRules.Count();
-  return NS_OK;
 }
 
-NS_IMETHODIMP
-GroupRule::GetStyleRuleAt(PRInt32 aIndex, nsICSSRule*& aRule) const
+nsICSSRule*
+GroupRule::GetStyleRuleAt(PRInt32 aIndex) const
 {
-  if (aIndex < 0 || aIndex >= mRules.Count()) {
-    aRule = nsnull;
-    return NS_ERROR_ILLEGAL_VALUE;
-  }
-
-  NS_ADDREF(aRule = mRules.ObjectAt(aIndex));
-  return NS_OK;
+  return mRules.SafeObjectAt(aIndex);
 }
 
-NS_IMETHODIMP_(PRBool)
+PRBool
 GroupRule::EnumerateRulesForwards(RuleEnumFunc aFunc, void * aData) const
 {
   return
     const_cast<GroupRule*>(this)->mRules.EnumerateForwards(aFunc, aData);
 }
 
 /*
  * The next two methods (DeleteStyleRuleAt and InsertStyleRulesAt)
  * should never be called unless you have first called WillDirty() on
  * the parents tylesheet.  After they are called, DidDirty() needs to
  * be called on the sheet
  */
-NS_IMETHODIMP
+nsresult
 GroupRule::DeleteStyleRuleAt(PRUint32 aIndex)
 {
   nsICSSRule* rule = mRules.SafeObjectAt(aIndex);
   if (rule) {
     rule->SetStyleSheet(nsnull);
     rule->SetParentRule(nsnull);
   }
   return mRules.RemoveObjectAt(aIndex) ? NS_OK : NS_ERROR_ILLEGAL_VALUE;
 }
 
-NS_IMETHODIMP
+nsresult
 GroupRule::InsertStyleRulesAt(PRUint32 aIndex,
                               nsCOMArray<nsICSSRule>& aRules)
 {
   aRules.EnumerateForwards(SetStyleSheetReference, mSheet);
   aRules.EnumerateForwards(SetParentRuleReference, this);
   if (! mRules.InsertObjectsAt(aRules, aIndex)) {
     return NS_ERROR_FAILURE;
   }
   return NS_OK;
 }
 
-NS_IMETHODIMP
+nsresult
 GroupRule::ReplaceStyleRule(nsICSSRule* aOld, nsICSSRule* aNew)
 {
   PRInt32 index = mRules.IndexOf(aOld);
   NS_ENSURE_TRUE(index != -1, NS_ERROR_UNEXPECTED);
   mRules.ReplaceObjectAt(aNew, index);
   aNew->SetStyleSheet(mSheet);
   aNew->SetParentRule(this);
   aOld->SetStyleSheet(nsnull);
@@ -750,42 +727,34 @@ GroupRule::AppendRulesToCssText(nsAStrin
   }
 
   aCssText.AppendLiteral("}");
   
   return NS_OK;
 }
 
 nsresult
-GroupRule::GetParentStyleSheet(nsIDOMCSSStyleSheet** aSheet)
-{
-  NS_IF_ADDREF(*aSheet = mSheet);
-  return NS_OK;
-}
-
-nsresult
 GroupRule::GetParentRule(nsIDOMCSSRule** aParentRule)
 {
   if (mParentRule) {
     return mParentRule->GetDOMRule(aParentRule);
   }
   *aParentRule = nsnull;
   return NS_OK;
 }
 
 // nsIDOMCSSMediaRule or nsIDOMCSSMozDocumentRule methods
-nsresult
-GroupRule::GetCssRules(nsIDOMCSSRuleList* *aRuleList)
+nsIDOMCSSRuleList*
+GroupRule::GetCssRules()
 {
   if (!mRuleCollection) {
     mRuleCollection = new css::GroupRuleRuleList(this);
   }
 
-  NS_ADDREF(*aRuleList = mRuleCollection);
-  return NS_OK;
+  return mRuleCollection;
 }
 
 nsresult
 GroupRule::InsertRule(const nsAString & aRule, PRUint32 aIndex, PRUint32* _retval)
 {
   NS_ENSURE_TRUE(mSheet, NS_ERROR_FAILURE);
   
   if (aIndex > PRUint32(mRules.Count()))
@@ -931,17 +900,18 @@ NS_IMETHODIMP
 MediaRule::SetCssText(const nsAString& aCssText)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 MediaRule::GetParentStyleSheet(nsIDOMCSSStyleSheet** aSheet)
 {
-  return GroupRule::GetParentStyleSheet(aSheet);
+  NS_IF_ADDREF(*aSheet = mSheet);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 MediaRule::GetParentRule(nsIDOMCSSRule** aParentRule)
 {
   return GroupRule::GetParentRule(aParentRule);
 }
 
@@ -952,33 +922,34 @@ MediaRule::GetMedia(nsIDOMMediaList* *aM
   NS_ENSURE_ARG_POINTER(aMedia);
   NS_IF_ADDREF(*aMedia = mMedia);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 MediaRule::GetCssRules(nsIDOMCSSRuleList* *aRuleList)
 {
-  return GroupRule::GetCssRules(aRuleList);
+  NS_ADDREF(*aRuleList = GroupRule::GetCssRules());
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 MediaRule::InsertRule(const nsAString & aRule, PRUint32 aIndex, PRUint32* _retval)
 {
   return GroupRule::InsertRule(aRule, aIndex, _retval);
 }
 
 NS_IMETHODIMP
 MediaRule::DeleteRule(PRUint32 aIndex)
 {
   return GroupRule::DeleteRule(aIndex);
 }
 
 // GroupRule interface
-NS_IMETHODIMP_(PRBool)
+/* virtual */ PRBool
 MediaRule::UseForPresentation(nsPresContext* aPresContext,
                                    nsMediaQueryResultCacheKey& aKey)
 {
   if (mMedia) {
     return mMedia->Matches(aPresContext, aKey);
   }
   return PR_TRUE;
 }
@@ -1102,45 +1073,47 @@ NS_IMETHODIMP
 DocumentRule::SetCssText(const nsAString& aCssText)
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 DocumentRule::GetParentStyleSheet(nsIDOMCSSStyleSheet** aSheet)
 {
-  return GroupRule::GetParentStyleSheet(aSheet);
+  NS_IF_ADDREF(*aSheet = mSheet);
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 DocumentRule::GetParentRule(nsIDOMCSSRule** aParentRule)
 {
   return GroupRule::GetParentRule(aParentRule);
 }
 
 NS_IMETHODIMP
 DocumentRule::GetCssRules(nsIDOMCSSRuleList* *aRuleList)
 {
-  return GroupRule::GetCssRules(aRuleList);
+  NS_ADDREF(*aRuleList = GroupRule::GetCssRules());
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 DocumentRule::InsertRule(const nsAString & aRule, PRUint32 aIndex, PRUint32* _retval)
 {
   return GroupRule::InsertRule(aRule, aIndex, _retval);
 }
 
 NS_IMETHODIMP
 DocumentRule::DeleteRule(PRUint32 aIndex)
 {
   return GroupRule::DeleteRule(aIndex);
 }
 
 // GroupRule interface
-NS_IMETHODIMP_(PRBool)
+/* virtual */ PRBool
 DocumentRule::UseForPresentation(nsPresContext* aPresContext,
                                       nsMediaQueryResultCacheKey& aKey)
 {
   nsIURI *docURI = aPresContext->Document()->GetDocumentURI();
   nsCAutoString docURISpec;
   if (docURI)
     docURI->GetSpec(docURISpec);
 
--- a/layout/style/nsCSSRules.h
+++ b/layout/style/nsCSSRules.h
@@ -74,31 +74,31 @@ public:
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, PRInt32 aIndent = 0) const;
 #endif
 
   // nsICSSRule methods
   virtual void SetStyleSheet(nsCSSStyleSheet* aSheet); //override GroupRule
   virtual PRInt32 GetType() const;
   virtual already_AddRefed<nsICSSRule> Clone() const;
-  nsIDOMCSSRule* GetDOMRuleWeak(nsresult *aResult)
+  virtual nsIDOMCSSRule* GetDOMRuleWeak(nsresult *aResult)
   {
     *aResult = NS_OK;
     return this;
   }
 
   // nsIDOMCSSRule interface
   NS_DECL_NSIDOMCSSRULE
 
   // nsIDOMCSSMediaRule interface
   NS_DECL_NSIDOMCSSMEDIARULE
 
   // rest of GroupRule
-  NS_IMETHOD_(PRBool) UseForPresentation(nsPresContext* aPresContext,
-                                         nsMediaQueryResultCacheKey& aKey);
+  virtual PRBool UseForPresentation(nsPresContext* aPresContext,
+                                    nsMediaQueryResultCacheKey& aKey);
 
   // @media rule methods
   nsresult SetMedia(nsMediaList* aMedia);
   
 protected:
   nsRefPtr<nsMediaList> mMedia;
 };
 
@@ -117,31 +117,31 @@ public:
   // nsIStyleRule methods
 #ifdef DEBUG
   virtual void List(FILE* out = stdout, PRInt32 aIndent = 0) const;
 #endif
 
   // nsICSSRule methods
   virtual PRInt32 GetType() const;
   virtual already_AddRefed<nsICSSRule> Clone() const;
-  nsIDOMCSSRule* GetDOMRuleWeak(nsresult *aResult)
+  virtual nsIDOMCSSRule* GetDOMRuleWeak(nsresult *aResult)
   {
     *aResult = NS_OK;
     return this;
   }
 
   // nsIDOMCSSRule interface
   NS_DECL_NSIDOMCSSRULE
 
   // nsIDOMCSSMozDocumentRule interface
   NS_DECL_NSIDOMCSSMOZDOCUMENTRULE
 
   // rest of GroupRule
-  NS_IMETHOD_(PRBool) UseForPresentation(nsPresContext* aPresContext,
-                                         nsMediaQueryResultCacheKey& aKey);
+  virtual PRBool UseForPresentation(nsPresContext* aPresContext,
+                                    nsMediaQueryResultCacheKey& aKey);
 
   enum Function {
     eURL,
     eURLPrefix,
     eDomain
   };
 
   struct URL {
--- a/layout/style/nsCSSStyleSheet.cpp
+++ b/layout/style/nsCSSStyleSheet.cpp
@@ -1934,20 +1934,19 @@ nsCSSStyleSheet::DeleteRule(PRUint32 aIn
 }
 
 nsresult
 nsCSSStyleSheet::DeleteRuleFromGroup(css::GroupRule* aGroup, PRUint32 aIndex)
 {
   NS_ENSURE_ARG_POINTER(aGroup);
   NS_ASSERTION(mInner->mComplete, "No deleting from an incomplete sheet!");
   nsresult result;
-  nsCOMPtr<nsICSSRule> rule;
-  result = aGroup->GetStyleRuleAt(aIndex, *getter_AddRefs(rule));
-  NS_ENSURE_SUCCESS(result, result);
-  
+  nsCOMPtr<nsICSSRule> rule = aGroup->GetStyleRuleAt(aIndex);
+  NS_ENSURE_TRUE(rule, NS_ERROR_ILLEGAL_VALUE);
+
   // check that the rule actually belongs to this sheet!
   nsCOMPtr<nsIStyleSheet> ruleSheet = rule->GetStyleSheet();
   if (this != ruleSheet) {
     return NS_ERROR_INVALID_ARG;
   }
 
   mozAutoDocUpdate updateBatch(mDocument, UPDATE_STYLE, PR_TRUE);