Auto merge of #17550 - emilio:xbl-is-a-pain-in-the, r=heycam

Fix dynamic style changes in XBL.

From bug 1375969

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17550)
<!-- Reviewable:end -->

components/style/dom.rs

@@ -31,6 +31,7 @@ use std::fmt::Debug;
 use std::hash::Hash;
 use std::ops::Deref;
 use stylearc::Arc;
+use stylist::Stylist;
 use thread_state;
 
 pub use style_traits::UnsafeNode;
@@ -623,15 +624,34 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
         }
     }
 
-    /// Gets declarations from XBL bindings from the element. Only gecko element could have this.
+    /// Implements Gecko's `nsBindingManager::WalkRules`.
-    fn get_declarations_from_xbl_bindings<V>(&self,
+    ///
-                                             _pseudo_element: Option<&PseudoElement>,
+    /// Returns whether to cut off the inheritance.
-                                             _applicable_declarations: &mut V)
+    fn each_xbl_stylist<F>(&self, _: F) -> bool
-                                             -> bool
+    where
-        where V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> {
+        F: FnMut(&Stylist),
+    {
         false
     }
 
+    /// Gets declarations from XBL bindings from the element.
+    fn get_declarations_from_xbl_bindings<V>(
+        &self,
+        pseudo_element: Option<&PseudoElement>,
+        applicable_declarations: &mut V
+    ) -> bool
+    where
+        V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock>
+    {
+        self.each_xbl_stylist(|stylist| {
+            stylist.push_applicable_declarations_as_xbl_only_stylist(
+                self,
+                pseudo_element,
+                applicable_declarations
+            );
+        })
+    }
+
     /// Gets the current existing CSS transitions, by |property, end value| pairs in a HashMap.
     #[cfg(feature = "gecko")]
     fn get_css_transitions_info(&self)

components/style/gecko/generated/bindings.rs

@@ -1936,12 +1936,15 @@ extern "C" {
 extern "C" {
     pub fn Servo_StyleSet_MightHaveAttributeDependency(set:
                                                            RawServoStyleSetBorrowed,
+                                                       element:
+                                                           RawGeckoElementBorrowed,
                                                        local_name:
                                                            *mut nsIAtom)
      -> bool;
 }
 extern "C" {
     pub fn Servo_StyleSet_HasStateDependency(set: RawServoStyleSetBorrowed,
+                                             element: RawGeckoElementBorrowed,
                                              state: u64) -> bool;
 }
 extern "C" {

components/style/gecko/wrapper.rs

@@ -80,7 +80,6 @@ use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingCo
 use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode};
 use selectors::sink::Push;
 use shared_lock::Locked;
-use smallvec::VecLike;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::fmt;
@@ -91,6 +90,7 @@ use std::ptr;
 use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use stylearc::Arc;
 use stylesheets::UrlExtraData;
+use stylist::Stylist;
 
 /// A simple wrapper over a non-null Gecko node (`nsINode`) pointer.
 ///
@@ -423,23 +423,21 @@ impl<'lb> GeckoXBLBinding<'lb> {
         }
     }
 
-    // Implements Gecko's nsXBLBinding::WalkRules().
+    fn each_xbl_stylist<F>(self, mut f: &mut F)
-    fn get_declarations_for<E, V>(&self,
+    where
-                                  element: &E,
+        F: FnMut(&Stylist),
-                                  pseudo_element: Option<&PseudoElement>,
+    {
-                                  applicable_declarations: &mut V)
+        if let Some(base) = self.base_binding() {
-        where E: TElement,
+            base.each_xbl_stylist(f);
-              V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> {
-        if let Some(base_binding) = self.base_binding() {
-            base_binding.get_declarations_for(element, pseudo_element, applicable_declarations);
         }
 
-        let raw_data = unsafe { bindings::Gecko_XBLBinding_GetRawServoStyleSet(self.0) };
+        let raw_data = unsafe {
+            bindings::Gecko_XBLBinding_GetRawServoStyleSet(self.0)
+        };
+
         if let Some(raw_data) = raw_data {
             let data = PerDocumentStyleData::from_ffi(&*raw_data).borrow();
-            data.stylist.push_applicable_declarations_as_xbl_only_stylist(element,
+            f(&data.stylist);
-                                                                          pseudo_element,
-                                                                          applicable_declarations);
         }
     }
 }
@@ -1112,30 +1110,27 @@ impl<'le> TElement for GeckoElement<'le> {
         self.may_have_animations() && unsafe { Gecko_ElementHasCSSTransitions(self.0) }
     }
 
-    // Implements Gecko's nsBindingManager::WalkRules(). Returns whether to cut off the
+    fn each_xbl_stylist<F>(&self, mut f: F) -> bool
-    // inheritance.
+    where
-    fn get_declarations_from_xbl_bindings<V>(&self,
+        F: FnMut(&Stylist),
-                                             pseudo_element: Option<&PseudoElement>,
+    {
-                                             applicable_declarations: &mut V)
+        // Walk the binding scope chain, starting with the binding attached to
-                                             -> bool
+        // our content, up till we run out of scopes or we get cut off.
-        where V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> {
+        //
-        // Walk the binding scope chain, starting with the binding attached to our content, up
+        // If we are a NAC pseudo-element, we want to get rules from our
-        // till we run out of scopes or we get cut off.
+        // rule_hash_target, that is, our originating element.
-
-        // If we are NAC, we want to get rules from our rule_hash_target.
         let mut current = Some(self.rule_hash_target());
 
         while let Some(element) = current {
             if let Some(binding) = element.get_xbl_binding() {
-                binding.get_declarations_for(self,
+                binding.each_xbl_stylist(&mut f);
-                                             pseudo_element,
-                                             applicable_declarations);
 
-                // If we're not looking at our original element, allow the binding to cut off
+                // If we're not looking at our original element, allow the
-                // style inheritance.
+                // binding to cut off style inheritance.
                 if element != *self {
                     if !binding.inherits_style() {
-                        // Go no further; we're not inheriting style from anything above here.
+                        // Go no further; we're not inheriting style from
+                        // anything above here.
                         break;
                     }
                 }
@@ -1149,8 +1144,8 @@ impl<'le> TElement for GeckoElement<'le> {
             current = element.get_xbl_binding_parent();
         }
 
-        // If current has something, this means we cut off inheritance at some point in the
+        // If current has something, this means we cut off inheritance at some
-        // loop.
+        // point in the loop.
         current.is_some()
     }
 

components/style/invalidation/element/invalidator.rs

@@ -14,7 +14,7 @@ use invalidation::element::element_wrapper::{ElementSnapshot, ElementWrapper};
 use invalidation::element::invalidation_map::*;
 use invalidation::element::restyle_hints::*;
 use selector_map::SelectorMap;
-use selector_parser::SelectorImpl;
+use selector_parser::{SelectorImpl, Snapshot};
 use selectors::attr::CaseSensitivity;
 use selectors::matching::{MatchingContext, MatchingMode, VisitedHandlingMode};
 use selectors::matching::{matches_selector, matches_compound_selector};
@@ -159,52 +159,33 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
             let mut collector = InvalidationCollector {
                 wrapper: wrapper,
                 element: self.element,
+                snapshot: &snapshot,
                 shared_context: self.shared_context,
                 lookup_element: lookup_element,
                 removed_id: id_removed.as_ref(),
+                added_id: id_added.as_ref(),
                 classes_removed: &classes_removed,
+                classes_added: &classes_added,
+                state_changes: state_changes,
                 descendant_invalidations: &mut descendant_invalidations,
                 sibling_invalidations: &mut sibling_invalidations,
                 invalidates_self: false,
             };
 
-            let map = shared_context.stylist.invalidation_map();
+            collector.collect_dependencies_in_invalidation_map(
+                shared_context.stylist.invalidation_map(),
+            );
 
-            if let Some(ref id) = id_removed {
+            // TODO(emilio): Consider storing dependencies from the UA sheet in
-                if let Some(deps) = map.id_to_selector.get(id, shared_context.quirks_mode) {
+            // a different map. If we do that, we can skip the stuff on the
-                    collector.collect_dependencies_in_map(deps)
+            // shared stylist iff cut_off_inheritance is true, and we can look
-                }
+            // just at that map.
-            }
+            let _cut_off_inheritance =
-
+                self.element.each_xbl_stylist(|stylist| {
-            if let Some(ref id) = id_added {
+                    collector.collect_dependencies_in_invalidation_map(
-                if let Some(deps) = map.id_to_selector.get(id, shared_context.quirks_mode) {
+                        stylist.invalidation_map(),
-                    collector.collect_dependencies_in_map(deps)
+                    );
-                }
+                });
-            }
-
-            for class in classes_added.iter().chain(classes_removed.iter()) {
-                if let Some(deps) = map.class_to_selector.get(class, shared_context.quirks_mode) {
-                    collector.collect_dependencies_in_map(deps)
-                }
-            }
-
-            let should_examine_attribute_selector_map =
-                snapshot.other_attr_changed() ||
-                (snapshot.class_changed() && map.has_class_attribute_selectors) ||
-                (snapshot.id_changed() && map.has_id_attribute_selectors);
-
-            if should_examine_attribute_selector_map {
-                collector.collect_dependencies_in_map(
-                    &map.other_attribute_affecting_selectors
-                )
-            }
-
-            if !state_changes.is_empty() {
-                collector.collect_state_dependencies(
-                    &map.state_affecting_selectors,
-                    state_changes,
-                )
-            }
 
             collector.invalidates_self
         };
@@ -641,10 +622,14 @@ struct InvalidationCollector<'a, 'b: 'a, E>
 {
     element: E,
     wrapper: ElementWrapper<'b, E>,
+    snapshot: &'a Snapshot,
     shared_context: &'a SharedStyleContext<'b>,
     lookup_element: E,
     removed_id: Option<&'a Atom>,
+    added_id: Option<&'a Atom>,
     classes_removed: &'a SmallVec<[Atom; 8]>,
+    classes_added: &'a SmallVec<[Atom; 8]>,
+    state_changes: ElementState,
     descendant_invalidations: &'a mut InvalidationVector,
     sibling_invalidations: &'a mut InvalidationVector,
     invalidates_self: bool,
@@ -653,6 +638,51 @@ struct InvalidationCollector<'a, 'b: 'a, E>
 impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E>
     where E: TElement,
 {
+    fn collect_dependencies_in_invalidation_map(
+        &mut self,
+        map: &InvalidationMap,
+    ) {
+        let quirks_mode = self.shared_context.quirks_mode;
+        let removed_id = self.removed_id;
+        if let Some(ref id) = removed_id {
+            if let Some(deps) = map.id_to_selector.get(id, quirks_mode) {
+                self.collect_dependencies_in_map(deps)
+            }
+        }
+
+        let added_id = self.added_id;
+        if let Some(ref id) = added_id {
+            if let Some(deps) = map.id_to_selector.get(id, quirks_mode) {
+                self.collect_dependencies_in_map(deps)
+            }
+        }
+
+        for class in self.classes_added.iter().chain(self.classes_removed.iter()) {
+            if let Some(deps) = map.class_to_selector.get(class, quirks_mode) {
+                self.collect_dependencies_in_map(deps)
+            }
+        }
+
+        let should_examine_attribute_selector_map =
+            self.snapshot.other_attr_changed() ||
+            (self.snapshot.class_changed() && map.has_class_attribute_selectors) ||
+            (self.snapshot.id_changed() && map.has_id_attribute_selectors);
+
+        if should_examine_attribute_selector_map {
+            self.collect_dependencies_in_map(
+                &map.other_attribute_affecting_selectors
+            )
+        }
+
+        let state_changes = self.state_changes;
+        if !state_changes.is_empty() {
+            self.collect_state_dependencies(
+                &map.state_affecting_selectors,
+                state_changes,
+            )
+        }
+    }
+
     fn collect_dependencies_in_map(
         &mut self,
         map: &SelectorMap<Dependency>,
@@ -671,6 +701,7 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E>
             },
         );
     }
+
     fn collect_state_dependencies(
         &mut self,
         map: &SelectorMap<StateDependency>,

ports/geckolib/glue.rs

@@ -3171,17 +3171,56 @@ pub extern "C" fn Servo_StyleSet_ResolveForDeclarations(raw_data: RawServoStyleS
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_StyleSet_MightHaveAttributeDependency(raw_data: RawServoStyleSetBorrowed,
+pub extern "C" fn Servo_StyleSet_MightHaveAttributeDependency(
-                                                              local_name: *mut nsIAtom) -> bool {
+    raw_data: RawServoStyleSetBorrowed,
+    element: RawGeckoElementBorrowed,
+    local_name: *mut nsIAtom,
+) -> bool {
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    unsafe { Atom::with(local_name, |atom| data.stylist.might_have_attribute_dependency(atom)) }
+    let element = GeckoElement(element);
+    let mut has_dep = false;
+
+    unsafe {
+        Atom::with(local_name, |atom| {
+            has_dep = data.stylist.might_have_attribute_dependency(atom);
+
+            if !has_dep {
+                // TODO(emilio): Consider optimizing this storing attribute
+                // dependencies from UA sheets separately, so we could optimize
+                // the above lookup if cut_off_inheritance is true.
+                element.each_xbl_stylist(|stylist| {
+                    has_dep =
+                        has_dep || stylist.might_have_attribute_dependency(atom);
+                });
+            }
+        })
+    }
+
+    has_dep
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_StyleSet_HasStateDependency(raw_data: RawServoStyleSetBorrowed,
+pub extern "C" fn Servo_StyleSet_HasStateDependency(
-                                                    state: u64) -> bool {
+    raw_data: RawServoStyleSetBorrowed,
+    element: RawGeckoElementBorrowed,
+    state: u64,
+) -> bool {
+    let element = GeckoElement(element);
+
+    let state = ElementState::from_bits_truncate(state);
     let data = PerDocumentStyleData::from_ffi(raw_data).borrow();
-    data.stylist.might_have_state_dependency(ElementState::from_bits_truncate(state))
+
+    let mut has_dep = data.stylist.might_have_state_dependency(state);
+    if !has_dep {
+        // TODO(emilio): Consider optimizing this storing attribute
+        // dependencies from UA sheets separately, so we could optimize
+        // the above lookup if cut_off_inheritance is true.
+        element.each_xbl_stylist(|stylist| {
+            has_dep = has_dep || stylist.might_have_state_dependency(state);
+        });
+    }
+
+    has_dep
 }
 
 #[no_mangle]