Bug 1342303 part 4 - Remove nsCOMArray::EnumerateForwards uses in layout/style. r=heycam
☠☠ backed out by b0b40ce3dfe5 ☠ ☠
authorXidorn Quan <me@upsuper.org>
Mon, 27 Feb 2017 10:45:45 +1100
changeset 374708 bc3b2e7a383b6b0ee08363d642ead4188ecc8a2d
parent 374707 bdc491b9ebde3a05e8f7f4fd834f0354b4a99570
child 374709 20a1bcb47c33f96691fabd451bc54db13d3c7e26
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1342303
milestone54.0a1
Bug 1342303 part 4 - Remove nsCOMArray::EnumerateForwards uses in layout/style. r=heycam MozReview-Commit-ID: UjIZgeWRwh
layout/style/CSSStyleSheet.cpp
layout/style/CSSStyleSheet.h
layout/style/GroupRule.h
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsCSSRules.cpp
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -160,51 +160,49 @@ struct ChildSheetListBuilder {
       child->mParent = aPrimarySheet;
       child->SetAssociatedDocument(aPrimarySheet->mDocument,
                                    aPrimarySheet->mDocumentAssociationMode);
     }
   }
 };
 
 bool
-CSSStyleSheet::RebuildChildList(css::Rule* aRule, void* aBuilder)
+CSSStyleSheet::RebuildChildList(css::Rule* aRule,
+                                ChildSheetListBuilder* aBuilder)
 {
   int32_t type = aRule->GetType();
   if (type < css::Rule::IMPORT_RULE) {
     // Keep going till we get to the import rules.
     return true;
   }
 
   if (type != css::Rule::IMPORT_RULE) {
     // We're past all the import rules; stop the enumeration.
     return false;
   }
 
-  ChildSheetListBuilder* builder =
-    static_cast<ChildSheetListBuilder*>(aBuilder);
-
   // XXXbz We really need to decomtaminate all this stuff.  Is there a reason
   // that I can't just QI to ImportRule and get a CSSStyleSheet
   // directly from it?
   nsCOMPtr<nsIDOMCSSImportRule> importRule(do_QueryInterface(aRule));
   NS_ASSERTION(importRule, "GetType lied");
 
   nsCOMPtr<nsIDOMCSSStyleSheet> childSheet;
   importRule->GetStyleSheet(getter_AddRefs(childSheet));
 
   // Have to do this QI to be safe, since XPConnect can fake
   // nsIDOMCSSStyleSheets
   RefPtr<CSSStyleSheet> sheet = do_QueryObject(childSheet);
   if (!sheet) {
     return true;
   }
 
-  (*builder->sheetSlot) = sheet;
-  builder->SetParentLinks(*builder->sheetSlot);
-  builder->sheetSlot = &(*builder->sheetSlot)->mNext;
+  (*aBuilder->sheetSlot) = sheet;
+  aBuilder->SetParentLinks(*aBuilder->sheetSlot);
+  aBuilder->sheetSlot = &(*aBuilder->sheetSlot)->mNext;
   return true;
 }
 
 size_t
 CSSStyleSheet::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
   size_t n = StyleSheet::SizeOfIncludingThis(aMallocSizeOf);
   const CSSStyleSheet* s = this;
@@ -233,42 +231,54 @@ CSSStyleSheet::SizeOfIncludingThis(Mallo
   return n;
 }
 
 CSSStyleSheetInner::CSSStyleSheetInner(CSSStyleSheetInner& aCopy,
                                        CSSStyleSheet* aPrimarySheet)
   : StyleSheetInfo(aCopy, aPrimarySheet)
 {
   MOZ_COUNT_CTOR(CSSStyleSheetInner);
-  aCopy.mOrderedRules.EnumerateForwards(css::GroupRule::CloneRuleInto, &mOrderedRules);
-  mOrderedRules.EnumerateForwards(SetStyleSheetReference, aPrimarySheet);
+  for (css::Rule* rule : aCopy.mOrderedRules) {
+    RefPtr<css::Rule> clone = rule->Clone();
+    mOrderedRules.AppendObject(clone);
+    clone->SetStyleSheet(aPrimarySheet);
+  }
 
   ChildSheetListBuilder builder = { &mFirstChild, aPrimarySheet };
-  mOrderedRules.EnumerateForwards(CSSStyleSheet::RebuildChildList, &builder);
+  for (css::Rule* rule : mOrderedRules) {
+    if (!CSSStyleSheet::RebuildChildList(rule, &builder)) {
+      break;
+    }
+  }
 
   RebuildNameSpaces();
 }
 
 CSSStyleSheetInner::~CSSStyleSheetInner()
 {
   MOZ_COUNT_DTOR(CSSStyleSheetInner);
-  mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
+  for (css::Rule* rule : mOrderedRules) {
+    rule->SetStyleSheet(nullptr);
+  }
 }
 
 CSSStyleSheetInner*
 CSSStyleSheetInner::CloneFor(CSSStyleSheet* aPrimarySheet)
 {
   return new CSSStyleSheetInner(*this, aPrimarySheet);
 }
 
 void
 CSSStyleSheetInner::RemoveSheet(StyleSheet* aSheet)
 {
-  if ((aSheet == mSheets.ElementAt(0)) && (mSheets.Length() > 1)) {
-    mOrderedRules.EnumerateForwards(SetStyleSheetReference, mSheets[1]);
+  if (aSheet == mSheets.ElementAt(0) && mSheets.Length() > 1) {
+    StyleSheet* sheet = mSheets[1];
+    for (css::Rule* rule : mOrderedRules) {
+      rule->SetStyleSheet(sheet);
+    }
 
     ChildSheetListBuilder::ReparentChildList(mSheets[1], mFirstChild);
   }
 
   // Don't do anything after this call, because superclass implementation
   // may delete this.
   StyleSheetInfo::RemoveSheet(aSheet);
 }
@@ -300,17 +310,27 @@ CreateNameSpace(css::Rule* aRule, void* 
   return (css::Rule::CHARSET_RULE == type || css::Rule::IMPORT_RULE == type);
 }
 
 void 
 CSSStyleSheetInner::RebuildNameSpaces()
 {
   // Just nuke our existing namespace map, if any
   if (NS_SUCCEEDED(CreateNamespaceMap())) {
-    mOrderedRules.EnumerateForwards(CreateNameSpace, mNameSpaceMap);
+    for (css::Rule* rule : mOrderedRules) {
+      switch (rule->GetType()) {
+        case css::Rule::NAMESPACE_RULE:
+          AddNamespaceRuleToMap(rule, mNameSpaceMap);
+          continue;
+        case css::Rule::CHARSET_RULE:
+        case css::Rule::IMPORT_RULE:
+          continue;
+      }
+      break;
+    }
   }
 }
 
 nsresult
 CSSStyleSheetInner::CreateNamespaceMap()
 {
   mNameSpaceMap = nsXMLNameSpaceMap::Create(false);
   NS_ENSURE_TRUE(mNameSpaceMap, NS_ERROR_OUT_OF_MEMORY);
@@ -432,17 +452,19 @@ void
 CSSStyleSheet::UnlinkInner()
 {
   // We can only have a cycle through our inner if we have a unique inner,
   // because otherwise there are no JS wrappers for anything in the inner.
   if (mInner->mSheets.Length() != 1) {
     return;
   }
 
-  Inner()->mOrderedRules.EnumerateForwards(SetStyleSheetReference, nullptr);
+  for (css::Rule* rule : Inner()->mOrderedRules) {
+    rule->SetStyleSheet(nullptr);
+  }
   Inner()->mOrderedRules.Clear();
 
   StyleSheet::UnlinkInner();
 }
 
 void
 CSSStyleSheet::TraverseInner(nsCycleCollectionTraversalCallback &cb)
 {
--- a/layout/style/CSSStyleSheet.h
+++ b/layout/style/CSSStyleSheet.h
@@ -33,16 +33,17 @@ class nsCSSRuleProcessor;
 class nsIURI;
 class nsMediaQueryResultCacheKey;
 class nsStyleSet;
 class nsPresContext;
 class nsXMLNameSpaceMap;
 
 namespace mozilla {
 class CSSStyleSheet;
+struct ChildSheetListBuilder;
 
 namespace css {
 class GroupRule;
 } // namespace css
 namespace dom {
 class CSSRuleList;
 } // namespace dom
 
@@ -159,17 +160,18 @@ public:
                             nsMediaQueryResultCacheKey& aKey) const;
 
   nsresult ReparseSheet(const nsAString& aInput);
 
   void SetInRuleProcessorCache() { mInRuleProcessorCache = true; }
 
   // Function used as a callback to rebuild our inner's child sheet
   // list after we clone a unique inner for ourselves.
-  static bool RebuildChildList(css::Rule* aRule, void* aBuilder);
+  static bool RebuildChildList(css::Rule* aRule,
+                               ChildSheetListBuilder* aBuilder);
 
   size_t SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const override;
 
   dom::Element* GetScopeElement() const { return mScopeElement; }
   void SetScopeElement(dom::Element* aScopeElement)
   {
     mScopeElement = aScopeElement;
   }
--- a/layout/style/GroupRule.h
+++ b/layout/style/GroupRule.h
@@ -71,24 +71,16 @@ public:
 
   virtual bool UseForPresentation(nsPresContext* aPresContext,
                                     nsMediaQueryResultCacheKey& aKey) = 0;
 
   // non-virtual -- it is only called by subclasses
   size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
   virtual size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const override = 0;
 
-  static bool
-  CloneRuleInto(Rule* aRule, void* aArray)
-  {
-    RefPtr<Rule> clone = aRule->Clone();
-    static_cast<IncrementalClearCOMRuleArray*>(aArray)->AppendObject(clone);
-    return true;
-  }
-
   // WebIDL API
   dom::CSSRuleList* CssRules();
   uint32_t InsertRule(const nsAString& aRule, uint32_t aIndex,
                       ErrorResult& aRv);
   void DeleteRule(uint32_t aIndex, ErrorResult& aRv);
 
 protected:
   // to help implement nsIDOMCSSRule
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -3668,19 +3668,21 @@ nsCSSRuleProcessor::CascadeSheet(CSSStyl
 
     StyleSheet* child = aSheet->GetFirstChild();
     while (child) {
       CascadeSheet(child->AsGecko(), aData);
 
       child = child->mNext;
     }
 
-    if (!aSheet->Inner()->mOrderedRules.EnumerateForwards(CascadeRuleEnumFunc,
-                                                         aData))
-      return false;
+    for (css::Rule* rule : aSheet->Inner()->mOrderedRules) {
+      if (!CascadeRuleEnumFunc(rule, aData)) {
+        return false;
+      }
+    }
   }
   return true;
 }
 
 static int CompareWeightData(const void* aArg1, const void* aArg2,
                              void* closure)
 {
   const PerWeightData* arg1 = static_cast<const PerWeightData*>(aArg1);
--- a/layout/style/nsCSSRules.cpp
+++ b/layout/style/nsCSSRules.cpp
@@ -426,35 +426,32 @@ ImportRule::WrapObject(JSContext* aCx,
   return CSSImportRuleBinding::Wrap(aCx, this, aGivenProto);
 }
 
 GroupRule::GroupRule(uint32_t aLineNumber, uint32_t aColumnNumber)
   : Rule(aLineNumber, aColumnNumber)
 {
 }
 
-static bool
-SetParentRuleReference(Rule* aRule, void* aParentRule)
-{
-  GroupRule* parentRule = static_cast<GroupRule*>(aParentRule);
-  aRule->SetParentRule(parentRule);
-  return true;
-}
-
 GroupRule::GroupRule(const GroupRule& aCopy)
   : Rule(aCopy)
 {
-  const_cast<GroupRule&>(aCopy).mRules.EnumerateForwards(GroupRule::CloneRuleInto, &mRules);
-  mRules.EnumerateForwards(SetParentRuleReference, this);
+  for (const Rule* rule : aCopy.mRules) {
+    RefPtr<Rule> clone = rule->Clone();
+    mRules.AppendObject(clone);
+    clone->SetParentRule(this);
+  }
 }
 
 GroupRule::~GroupRule()
 {
   MOZ_ASSERT(!mSheet, "SetStyleSheet should have been called");
-  mRules.EnumerateForwards(SetParentRuleReference, nullptr);
+  for (Rule* rule : mRules) {
+    rule->SetParentRule(nullptr);
+  }
   if (mRuleCollection) {
     mRuleCollection->DropReference();
   }
 }
 
 NS_IMPL_ADDREF_INHERITED(GroupRule, Rule)
 NS_IMPL_RELEASE_INHERITED(GroupRule, Rule)
 
@@ -463,35 +460,32 @@ NS_INTERFACE_MAP_END_INHERITING(Rule)
 
 bool
 GroupRule::IsCCLeaf() const
 {
   // Let's not worry for now about sorting out whether we're a leaf or not.
   return false;
 }
 
-static bool
-SetStyleSheetReference(Rule* aRule, void* aSheet)
-{
-  aRule->SetStyleSheet(reinterpret_cast<StyleSheet*>(aSheet));
-  return true;
-}
-
 NS_IMPL_CYCLE_COLLECTION_CLASS(GroupRule)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(GroupRule, Rule)
-  tmp->mRules.EnumerateForwards(SetParentRuleReference, nullptr);
+  for (Rule* rule : tmp->mRules) {
+    rule->SetParentRule(nullptr);
+  }
   // If tmp does not have a stylesheet, neither do its descendants.  In that
   // case, don't try to null out their stylesheet, to avoid O(N^2) behavior in
   // depth of group rule nesting.  But if tmp _does_ have a stylesheet (which
   // can happen if it gets unlinked earlier than its owning stylesheet), then we
   // need to null out the stylesheet pointer on descendants now, before we clear
   // tmp->mRules.
   if (tmp->GetStyleSheet()) {
-    tmp->mRules.EnumerateForwards(SetStyleSheetReference, nullptr);
+    for (Rule* rule : tmp->mRules) {
+      rule->SetStyleSheet(nullptr);
+    }
   }
   tmp->mRules.Clear();
   if (tmp->mRuleCollection) {
     tmp->mRuleCollection->DropReference();
     tmp->mRuleCollection = nullptr;
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -509,17 +503,19 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 /* virtual */ void
 GroupRule::SetStyleSheet(StyleSheet* aSheet)
 {
   // Don't set the sheet on the kids if it's already the same as the sheet we
   // already have.  This is needed to avoid O(N^2) behavior in group nesting
   // depth when seting the sheet to null during unlink, if we happen to unlin in
   // order from most nested rule up to least nested rule.
   if (aSheet != GetStyleSheet()) {
-    mRules.EnumerateForwards(SetStyleSheetReference, aSheet);
+    for (Rule* rule : mRules) {
+      rule->SetStyleSheet(aSheet);
+    }
     Rule::SetStyleSheet(aSheet);
   }
 }
 
 #ifdef DEBUG
 /* virtual */ void
 GroupRule::List(FILE* out, int32_t aIndent) const
 {
@@ -545,18 +541,22 @@ Rule*
 GroupRule::GetStyleRuleAt(int32_t aIndex) const
 {
   return mRules.SafeObjectAt(aIndex);
 }
 
 bool
 GroupRule::EnumerateRulesForwards(RuleEnumFunc aFunc, void * aData) const
 {
-  return
-    const_cast<GroupRule*>(this)->mRules.EnumerateForwards(aFunc, aData);
+  for (const Rule* rule : mRules) {
+    if (!aFunc(const_cast<Rule*>(rule), aData)) {
+      return false;
+    }
+  }
+  return true;
 }
 
 /*
  * The next two methods (DeleteStyleRuleAt and InsertStyleRuleAt)
  * should never be called unless you have first called WillDirty() on
  * the parents stylesheet.  After they are called, DidDirty() needs to
  * be called on the sheet
  */