Issue #1593 - Part 2: Account for Shadow DOM v1 and iterator in nsBindingManager.

dom/xbl/nsBindingManager.cpp

@@ -669,25 +669,13 @@ nsBindingManager::WalkRules(nsIStyleRuleProcessor::EnumFunc aFunc,
   aData->mElementIsFeatureless = true;
 
   ShadowRoot* currentShadow = aData->mElement->GetShadowRoot();
-  // XXX: Obviously not the right approach for Shadow DOM v1.
-  // Per spec, rules from younger shadow DOM win over rules from older shadow 
-  // DOM. Iterate to the oldest shadow root on the host then walk back to the 
-  // youngest when walking rules.
-  while (currentShadow) {
-    ShadowRoot* olderShadow = currentShadow->GetOlderShadowRoot();
-    if (!olderShadow) {
-      break;
-    }
-    currentShadow = olderShadow;
-  }
 
-  while (currentShadow) {
+  if (currentShadow) {
     nsXBLBinding* associatedBinding = currentShadow->GetAssociatedBinding();
     if (associatedBinding) {
       aData->mTreeMatchContext.mScopedRoot = aData->mElement;
       associatedBinding->WalkRules(aFunc, aData);
     }
-    currentShadow = currentShadow->GetYoungerShadowRoot();
   }
 
   aData->mElementIsFeatureless = false;
@@ -733,24 +721,10 @@ nsBindingManager::WalkRules(nsIStyleRuleProcessor::EnumFunc aFunc,
 
 typedef nsTHashtable<nsPtrHashKey<nsIStyleRuleProcessor> > RuleProcessorSet;
 
-// XXX: Figure out what the anonymous namespace is for. Is this even appropriate now that we're using iterators instead of enumerators?
-
-namespace {
-
-struct RuleProcessorsData {
-  RuleProcessorSet* mSet;
-  bool mOnlyWalkShadowHosts;
-};
-
-} // anonymous namespace
-
-
 static RuleProcessorSet*
-GetContentSetRuleProcessors(nsTHashtable<nsRefPtrHashKey<nsIContent>>* aContentSet)
+GetContentSetRuleProcessors(nsTHashtable<nsRefPtrHashKey<nsIContent>>* aContentSet, bool aOnlyWalkShadowRootRules)
 {
-  RuleProcessorsData* data = nullptr;	
+  RuleProcessorSet* set = nullptr;
-  RuleProcessorSet* set = data->mSet;
-
 
   for (auto iter = aContentSet->Iter(); !iter.Done(); iter.Next()) {
     nsIContent* boundContent = iter.Get()->GetKey();
@@ -758,7 +732,7 @@ GetContentSetRuleProcessors(nsTHashtable<nsRefPtrHashKey<nsIContent>>* aContentS
   // If we are only walking rules for shadow root hosts, skip other types
   // of bound content.
   ShadowRoot *shadowRoot = boundContent->GetShadowRoot();
-  if (data->mOnlyWalkShadowHosts && !shadowRoot) {
+  if (aOnlyWalkShadowRootRules && !shadowRoot) {
     return set;
   }
 
@@ -767,7 +741,7 @@ GetContentSetRuleProcessors(nsTHashtable<nsRefPtrHashKey<nsIContent>>* aContentS
   // inheritance chain. Additionally, a bound content may host multiple
   // shadow roots, each with its own rule processor.
   nsXBLBinding *binding = boundContent->GetXBLBinding();
-  while (binding) {
+  if (binding) {
       nsIStyleRuleProcessor* ruleProc =
         binding->PrototypeBinding()->GetRuleProcessor();
       if (ruleProc) {
@@ -778,24 +752,15 @@ GetContentSetRuleProcessors(nsTHashtable<nsRefPtrHashKey<nsIContent>>* aContentS
       }
 
       binding = binding->GetBaseBinding();
-      // XXX: Obviously not the right approach for Shadow DOM v1.
+      if (shadowRoot) {
-      // If there isn't a base binding, start walking the binding of the
+        binding = shadowRoot->GetAssociatedBinding();
-      // next older shadow root hosted by the bound content (if any).
-      if (!binding && shadowRoot) {
-        shadowRoot = shadowRoot->GetOlderShadowRoot();
-        if (shadowRoot) {
-          binding = shadowRoot->GetAssociatedBinding();
         }
       }
     }
-  }
 
   return set;
 }
 
-// XXX: I think since this calls GetContentSetRuleProcessors directly now, no changes should be needed here? The original enumerator patch used the RuleProcessorsData function from the anonymous namespace here.
-
-
 void
 nsBindingManager::WalkAllRules(nsIStyleRuleProcessor::EnumFunc aFunc,
                                ElementDependentRuleProcessorData* aData,
@@ -806,7 +771,7 @@ nsBindingManager::WalkAllRules(nsIStyleRuleProcessor::EnumFunc aFunc,
   }
 
   nsAutoPtr<RuleProcessorSet> set;
-  set = GetContentSetRuleProcessors(mBoundContentSet);
+  set = GetContentSetRuleProcessors(mBoundContentSet, aOnlyWalkShadowRootRules);
   if (!set) {
     return;
   }
@@ -828,8 +793,6 @@ nsBindingManager::WalkAllShadowRootHostRules(nsIStyleRuleProcessor::EnumFunc aFu
    aData->mTreeMatchContext.mOnlyMatchHostPseudo = false;
  }
 
-//XXX: I think since this calls GetContentSetRuleProcessors directly now, no changes should be needed here? The original enumerator patch used the RuleProcessorsData function from the anonymous namespace here.
-
 nsresult
 nsBindingManager::MediumFeaturesChanged(nsPresContext* aPresContext,
                                         bool* aRulesChanged)
@@ -840,7 +803,7 @@ nsBindingManager::MediumFeaturesChanged(nsPresContext* aPresContext,
   }
 
   nsAutoPtr<RuleProcessorSet> set;
-  set = GetContentSetRuleProcessors(mBoundContentSet);
+  set = GetContentSetRuleProcessors(mBoundContentSet, false);
   if (!set) {
     return NS_OK;
   }