Backed out changeset fc813bf68348 for failing reftest layout/reftests/bugs/272646-1.xul on OS X. r=backout

servo/components/style/dom.rs

@@ -388,15 +388,6 @@ pub trait TElement
         depth
     }
 
-    /// The style scope of this element is a node that represents which rules
-    /// apply to the element.
-    ///
-    /// In Servo, where we don't know about Shadow DOM or XBL, the style scope
-    /// is always the document.
-    fn style_scope(&self) -> Self::ConcreteNode {
-        self.as_node().owner_doc().as_node()
-    }
-
     /// Get this node's parent element from the perspective of a restyle
     /// traversal.
     fn traversal_parent(&self) -> Option<Self> {
@@ -747,16 +738,14 @@ pub trait TElement
         None
     }
 
-    /// Return the element which we can use to look up rules in the selector
-    /// maps.
-    ///
-    /// This is always the element itself, except in the case where we are an
-    /// element-backed pseudo-element, in which case we return the originating
-    /// element.
+    /// Returns the rule hash target given an element.
     fn rule_hash_target(&self) -> Self {
         let is_implemented_pseudo =
             self.implemented_pseudo_element().is_some();
 
+        // NB: This causes use to rule has pseudo selectors based on the
+        // properties of the originating element (which is fine, given the
+        // find_first_from_right usage).
         if is_implemented_pseudo {
             self.closest_non_native_anonymous_ancestor().unwrap()
         } else {

servo/components/style/gecko/wrapper.rs

@@ -518,14 +518,9 @@ impl<'le> GeckoElement<'le> {
             .and_then(|s| unsafe { s.mExtendedSlots.mPtr.as_ref() })
     }
 
-    #[inline]
-    fn may_be_in_binding_manager(&self) -> bool {
-        self.flags() & (structs::NODE_MAY_BE_IN_BINDING_MNGR as u32) != 0
-    }
-
     #[inline]
     fn get_xbl_binding(&self) -> Option<GeckoXBLBinding<'le>> {
-        if !self.may_be_in_binding_manager() {
+        if self.flags() & (structs::NODE_MAY_BE_IN_BINDING_MNGR as u32) == 0 {
             return None;
         }
 
@@ -558,11 +553,9 @@ impl<'le> GeckoElement<'le> {
         } else {
             let binding_parent = unsafe {
                 self.get_non_xul_xbl_binding_parent_raw_content().as_ref()
-            }.map(GeckoNode::from_content).and_then(|n| n.as_element());
-
-            debug_assert!(binding_parent == unsafe {
-                bindings::Gecko_GetBindingParent(self.0).map(GeckoElement)
-            });
+            }.map(GeckoNode::from_content)
+                .and_then(|n| n.as_element());
+            debug_assert!(binding_parent == unsafe { bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) });
             binding_parent
         }
     }
@@ -932,28 +925,6 @@ impl<'le> TElement for GeckoElement<'le> {
         self.get_before_or_after_pseudo(/* is_before = */ false)
     }
 
-    /// Ensure this accurately represents the rules that an element may ever
-    /// match, even in the native anonymous content case.
-    fn style_scope(&self) -> Self::ConcreteNode {
-        if self.implemented_pseudo_element().is_some() {
-            return self.closest_non_native_anonymous_ancestor().unwrap().style_scope();
-        }
-
-        if self.is_in_native_anonymous_subtree() {
-            return self.as_node().owner_doc().as_node();
-        }
-
-        if self.get_xbl_binding().is_some() {
-            return self.as_node();
-        }
-
-        if let Some(parent) = self.get_xbl_binding_parent() {
-            return parent.as_node();
-        }
-
-        self.as_node().owner_doc().as_node()
-    }
-
     /// Execute `f` for each anonymous content child element (apart from
     /// ::before and ::after) whose originating element is `self`.
     fn each_anonymous_content_child<F>(&self, mut f: F)

servo/components/style/sharing/checks.rs

@@ -6,6 +6,7 @@
 //! quickly whether it's worth to share style, and whether two different
 //! elements can indeed share the same style.
 
+use Atom;
 use bloom::StyleBloom;
 use context::{SelectorFlagsMap, SharedStyleContext};
 use dom::TElement;
@@ -95,16 +96,14 @@ pub fn have_same_class<E>(target: &mut StyleSharingTarget<E>,
 /// :first-child, etc, or on attributes that we don't check off-hand (pretty
 /// much every attribute selector except `id` and `class`.
 #[inline]
-pub fn revalidate<E>(
-    target: &mut StyleSharingTarget<E>,
-    candidate: &mut StyleSharingCandidate<E>,
-    shared_context: &SharedStyleContext,
-    bloom: &StyleBloom<E>,
-    nth_index_cache: &mut NthIndexCache,
-    selector_flags_map: &mut SelectorFlagsMap<E>,
-) -> bool
-where
-    E: TElement,
+pub fn revalidate<E>(target: &mut StyleSharingTarget<E>,
+                     candidate: &mut StyleSharingCandidate<E>,
+                     shared_context: &SharedStyleContext,
+                     bloom: &StyleBloom<E>,
+                     nth_index_cache: &mut NthIndexCache,
+                     selector_flags_map: &mut SelectorFlagsMap<E>)
+                     -> bool
+    where E: TElement,
 {
     let stylist = &shared_context.stylist;
 
@@ -131,25 +130,16 @@ where
 
 /// Checks whether we might have rules for either of the two ids.
 #[inline]
-pub fn may_match_different_id_rules<E>(
-    shared_context: &SharedStyleContext,
-    element: E,
-    candidate: E,
-) -> bool
-where
-    E: TElement,
+pub fn may_have_rules_for_ids(shared_context: &SharedStyleContext,
+                              element_id: Option<&Atom>,
+                              candidate_id: Option<&Atom>) -> bool
 {
-    let element_id = element.get_id();
-    let candidate_id = candidate.get_id();
-
-    if element_id == candidate_id {
-        return false;
-    }
-
+    // We shouldn't be called unless the ids are different.
+    debug_assert!(element_id.is_some() || candidate_id.is_some());
     let stylist = &shared_context.stylist;
 
     let may_have_rules_for_element = match element_id {
-        Some(ref id) => stylist.may_have_rules_for_id(id, element),
+        Some(id) => stylist.may_have_rules_for_id(id),
         None => false
     };
 
@@ -158,7 +148,7 @@ where
     }
 
     match candidate_id {
-        Some(ref id) => stylist.may_have_rules_for_id(id, candidate),
+        Some(id) => stylist.may_have_rules_for_id(id),
         None => false
     }
 }

servo/components/style/sharing/mod.rs

@@ -206,11 +206,10 @@ impl ValidationData {
         bloom: &StyleBloom<E>,
         nth_index_cache: &mut NthIndexCache,
         bloom_known_valid: bool,
-        flags_setter: &mut F,
+        flags_setter: &mut F
     ) -> &SmallBitVec
-    where
-        E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
+        where E: TElement,
+              F: FnMut(&E, ElementSelectorFlags),
     {
         if self.revalidation_match_results.is_none() {
             // The bloom filter may already be set up for our element.
@@ -231,12 +230,10 @@ impl ValidationData {
                 }
             };
             self.revalidation_match_results =
-                Some(stylist.match_revalidation_selectors(
-                    element,
-                    bloom_to_use,
-                    nth_index_cache,
-                    flags_setter,
-                ));
+                Some(stylist.match_revalidation_selectors(&element,
+                                                          bloom_to_use,
+                                                          nth_index_cache,
+                                                          flags_setter));
         }
 
         self.revalidation_match_results.as_ref().unwrap()
@@ -664,15 +661,6 @@ impl<E: TElement> StyleSharingCache<E> {
             return None;
         }
 
-        // Note that in the XBL case, we should be able to assert that the
-        // scopes are different, since two elements with different XBL bindings
-        // need to necessarily have different style (and thus children of them
-        // would never pass the parent check).
-        if target.element.style_scope() != candidate.element.style_scope() {
-            trace!("Miss: Different style scopes");
-            return None;
-        }
-
         if *target.get_local_name() != *candidate.element.get_local_name() {
             trace!("Miss: Local Name");
             return None;
@@ -702,17 +690,15 @@ impl<E: TElement> StyleSharingCache<E> {
             return None;
         }
 
-        // It's possible that there are no styles for either id.
-        let may_match_different_id_rules =
-            checks::may_match_different_id_rules(
-                shared,
-                target.element,
-                candidate.element,
-            );
-
-        if may_match_different_id_rules {
-            trace!("Miss: ID Attr");
-            return None;
+        let element_id = target.element.get_id();
+        let candidate_id = candidate.element.get_id();
+        if element_id != candidate_id {
+            // It's possible that there are no styles for either id.
+            if checks::may_have_rules_for_ids(shared, element_id.as_ref(),
+                                              candidate_id.as_ref()) {
+                trace!("Miss: ID Attr");
+                return None;
+            }
         }
 
         if !checks::have_same_style_attribute(target, candidate) {

servo/components/style/stylist.rs

@@ -1409,29 +1409,10 @@ impl Stylist {
     /// Given an id, returns whether there might be any rules for that id in any
     /// of our rule maps.
     #[inline]
-    pub fn may_have_rules_for_id<E>(
-        &self,
-        id: &Atom,
-        element: E,
-    ) -> bool
-    where
-        E: TElement,
-    {
-        let hash = id.get_hash();
-        for (data, _) in self.cascade_data.iter_origins() {
-            if data.mapped_ids.might_contain_hash(hash) {
-                return true;
-            }
-        }
-
-        let mut xbl_rules_may_contain = false;
-
-        element.each_xbl_stylist(|stylist| {
-            xbl_rules_may_contain = xbl_rules_may_contain ||
-                stylist.cascade_data.author.mapped_ids.might_contain_hash(hash)
-        });
-
-        xbl_rules_may_contain
+    pub fn may_have_rules_for_id(&self, id: &Atom) -> bool {
+        self.cascade_data
+            .iter_origins()
+            .any(|(d, _)| d.mapped_ids.might_contain_hash(id.get_hash()))
     }
 
     /// Returns the registered `@keyframes` animation for the specified name.
@@ -1447,7 +1428,7 @@ impl Stylist {
     /// revalidation selectors.
     pub fn match_revalidation_selectors<E, F>(
         &self,
-        element: E,
+        element: &E,
         bloom: Option<&BloomFilter>,
         nth_index_cache: &mut NthIndexCache,
         flags_setter: &mut F
@@ -1473,14 +1454,14 @@ impl Stylist {
         let mut results = SmallBitVec::new();
         for (data, _) in self.cascade_data.iter_origins() {
             data.selectors_for_cache_revalidation.lookup(
-                element,
+                *element,
                 self.quirks_mode,
                 &mut |selector_and_hashes| {
                     results.push(matches_selector(
                         &selector_and_hashes.selector,
                         selector_and_hashes.selector_offset,
                         Some(&selector_and_hashes.hashes),
-                        &element,
+                        element,
                         &mut matching_context,
                         flags_setter
                     ));
@@ -1489,24 +1470,6 @@ impl Stylist {
             );
         }
 
-        element.each_xbl_stylist(|stylist| {
-            stylist.cascade_data.author.selectors_for_cache_revalidation.lookup(
-                element,
-                stylist.quirks_mode,
-                &mut |selector_and_hashes| {
-                    results.push(matches_selector(
-                        &selector_and_hashes.selector,
-                        selector_and_hashes.selector_offset,
-                        Some(&selector_and_hashes.hashes),
-                        &element,
-                        &mut matching_context,
-                        flags_setter
-                    ));
-                    true
-                }
-            );
-        });
-
         results
     }