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 345681 bc3b2e7a383b6b0ee08363d642ead4188ecc8a2d
parent 345680 bdc491b9ebde3a05e8f7f4fd834f0354b4a99570
child 345682 20a1bcb47c33f96691fabd451bc54db13d3c7e26
push id31443
push usercbook@mozilla.com
push dateFri, 03 Mar 2017 12:01:25 +0000
treeherdermozilla-central@31c09bb63b69 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1342303
milestone54.0a1
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 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
  */