Bug 1336646 - Apply selector flags during traversal. r=emilio

components/script/dom/bindings/trace.rs

@@ -76,6 +76,7 @@ use script_layout_interface::reporter::CSSErrorReporter;
 use script_layout_interface::rpc::LayoutRPC;
 use script_traits::{DocumentActivity, TimerEventId, TimerSource, TouchpadPressurePhase};
 use script_traits::{UntrustedNodeAddress, WindowSizeData, WindowSizeType};
+use selectors::matching::ElementSelectorFlags;
 use serde::{Deserialize, Serialize};
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
@@ -347,6 +348,7 @@ unsafe_no_jsmanaged_fields!(TimeProfilerChan);
 unsafe_no_jsmanaged_fields!(MemProfilerChan);
 unsafe_no_jsmanaged_fields!(PseudoElement);
 unsafe_no_jsmanaged_fields!(Length);
+unsafe_no_jsmanaged_fields!(ElementSelectorFlags);
 unsafe_no_jsmanaged_fields!(ElementState);
 unsafe_no_jsmanaged_fields!(DOMString);
 unsafe_no_jsmanaged_fields!(Mime);

components/script/dom/element.rs

@@ -83,7 +83,7 @@ use parking_lot::RwLock;
 use ref_filter_map::ref_filter_map;
 use script_layout_interface::message::ReflowQueryType;
 use script_thread::Runnable;
-use selectors::matching::{ElementFlags, MatchingReason, matches};
+use selectors::matching::{ElementSelectorFlags, matches};
 use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use servo_atoms::Atom;
@@ -95,7 +95,6 @@ use std::default::Default;
 use std::fmt;
 use std::rc::Rc;
 use std::sync::Arc;
-use std::sync::atomic::{AtomicUsize, Ordering};
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
 use style::context::{QuirksMode, ReflowGoal};
 use style::element_state::*;
@@ -109,6 +108,7 @@ use style::rule_tree::CascadeLevel;
 use style::selector_parser::{NonTSPseudoClass, RestyleDamage, SelectorImpl, SelectorParser};
 use style::sink::Push;
 use style::stylist::ApplicableDeclarationBlock;
+use style::thread_state;
 use style::values::CSSFloat;
 use style::values::specified::{self, CSSColor, CSSRGBA};
 use stylesheet_loader::StylesheetOwner;
@@ -131,7 +131,12 @@ pub struct Element {
     attr_list: MutNullableJS<NamedNodeMap>,
     class_list: MutNullableJS<DOMTokenList>,
     state: Cell<ElementState>,
-    atomic_flags: AtomicElementFlags,
+    /// These flags are set by the style system to indicate the that certain
+    /// operations may require restyling this element or its descendants. The
+    /// flags are not atomic, so the style system takes care of only set them
+    /// when it has exclusive access to the element.
+    #[ignore_heap_size_of = "bitflags defined in rust-selectors"]
+    selector_flags: Cell<ElementSelectorFlags>,
 }
 
 impl fmt::Debug for Element {
@@ -219,7 +224,7 @@ impl Element {
             attr_list: Default::default(),
             class_list: Default::default(),
             state: Cell::new(state),
-            atomic_flags: AtomicElementFlags::new(),
+            selector_flags: Cell::new(ElementSelectorFlags::empty()),
         }
     }
 
@@ -351,7 +356,8 @@ pub trait LayoutElementHelpers {
     fn get_checked_state_for_layout(&self) -> bool;
     fn get_indeterminate_state_for_layout(&self) -> bool;
     fn get_state_for_layout(&self) -> ElementState;
-    fn insert_atomic_flags(&self, flags: ElementFlags);
+    fn insert_selector_flags(&self, flags: ElementSelectorFlags);
+    fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool;
 }
 
 impl LayoutElementHelpers for LayoutJS<Element> {
@@ -720,9 +726,19 @@ impl LayoutElementHelpers for LayoutJS<Element> {
 
     #[inline]
     #[allow(unsafe_code)]
-    fn insert_atomic_flags(&self, flags: ElementFlags) {
+    fn insert_selector_flags(&self, flags: ElementSelectorFlags) {
+        debug_assert!(thread_state::get() == thread_state::LAYOUT);
         unsafe {
-            (*self.unsafe_get()).atomic_flags.insert(flags);
+            let f = &(*self.unsafe_get()).selector_flags;
+            f.set(f.get() | flags);
+        }
+    }
+
+    #[inline]
+    #[allow(unsafe_code)]
+    fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool {
+        unsafe {
+            (*self.unsafe_get()).selector_flags.get().contains(flags)
         }
     }
 }
@@ -1973,7 +1989,7 @@ impl ElementMethods for Element {
         match SelectorParser::parse_author_origin_no_namespace(&selectors) {
             Err(()) => Err(Error::Syntax),
             Ok(selectors) => {
-                Ok(matches(&selectors.0, &Root::from_ref(self), None, MatchingReason::Other))
+                Ok(matches(&selectors.0, &Root::from_ref(self), None))
             }
         }
     }
@@ -1991,7 +2007,8 @@ impl ElementMethods for Element {
                 let root = self.upcast::<Node>();
                 for element in root.inclusive_ancestors() {
                     if let Some(element) = Root::downcast::<Element>(element) {
-                        if matches(&selectors.0, &element, None, MatchingReason::Other) {
+                        if matches(&selectors.0, &element, None)
+                        {
                             return Ok(Some(element));
                         }
                     }
@@ -2231,7 +2248,7 @@ impl VirtualMethods for Element {
             s.children_changed(mutation);
         }
 
-        let flags = self.atomic_flags.get();
+        let flags = self.selector_flags.get();
         if flags.intersects(HAS_SLOW_SELECTOR) {
             // All children of this node need to be restyled when any child changes.
             self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
@@ -2744,24 +2761,6 @@ impl<'a> AttributeMutation<'a> {
     }
 }
 
-/// Thread-safe wrapper for ElementFlags set during selector matching
-#[derive(JSTraceable, HeapSizeOf)]
-struct AtomicElementFlags(AtomicUsize);
-
-impl AtomicElementFlags {
-    fn new() -> Self {
-        AtomicElementFlags(AtomicUsize::new(0))
-    }
-
-    fn get(&self) -> ElementFlags {
-        ElementFlags::from_bits_truncate(self.0.load(Ordering::Relaxed) as u8)
-    }
-
-    fn insert(&self, flags: ElementFlags) {
-        self.0.fetch_or(flags.bits() as usize, Ordering::Relaxed);
-    }
-}
-
 /// A holder for an element's "tag name", which will be lazily
 /// resolved and cached. Should be reset when the document
 /// owner changes.

components/script/dom/node.rs

@@ -67,7 +67,7 @@ use script_layout_interface::{LayoutElementType, LayoutNodeType, TrustedNodeAddr
 use script_layout_interface::message::Msg;
 use script_traits::DocumentActivity;
 use script_traits::UntrustedNodeAddress;
-use selectors::matching::{MatchingReason, matches};
+use selectors::matching::matches;
 use selectors::parser::SelectorList;
 use servo_url::ServoUrl;
 use std::borrow::ToOwned;
@@ -322,7 +322,7 @@ impl<'a> Iterator for QuerySelectorIterator {
         // (instead of passing `None`)? Probably.
         self.iterator.by_ref().filter_map(|node| {
             if let Some(element) = Root::downcast(node) {
-                if matches(selectors, &element, None, MatchingReason::Other) {
+                if matches(selectors, &element, None) {
                     return Some(Root::upcast(element));
                 }
             }
@@ -685,7 +685,7 @@ impl Node {
             // Step 3.
             Ok(selectors) => {
                 Ok(self.traverse_preorder().filter_map(Root::downcast).find(|element| {
-                    matches(&selectors.0, element, None, MatchingReason::Other)
+                    matches(&selectors.0, element, None)
                 }))
             }
         }

components/script/layout_wrapper.rs

@@ -49,7 +49,7 @@ use script_layout_interface::{HTMLCanvasData, LayoutNodeType, SVGSVGData, Truste
 use script_layout_interface::{OpaqueStyleAndLayoutData, PartialPersistentLayoutData};
 use script_layout_interface::wrapper_traits::{DangerousThreadSafeLayoutNode, GetLayoutData, LayoutNode};
 use script_layout_interface::wrapper_traits::{PseudoElementType, ThreadSafeLayoutElement, ThreadSafeLayoutNode};
-use selectors::matching::ElementFlags;
+use selectors::matching::ElementSelectorFlags;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
@@ -437,6 +437,14 @@ impl<'le> TElement for ServoLayoutElement<'le> {
     fn skip_root_and_item_based_display_fixup(&self) -> bool {
         false
     }
+
+    unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) {
+        self.element.insert_selector_flags(flags);
+    }
+
+    fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool {
+        self.element.has_selector_flags(flags)
+    }
 }
 
 impl<'le> PartialEq for ServoLayoutElement<'le> {
@@ -665,10 +673,6 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
             self.element.html_element_in_html_document_for_layout()
         }
     }
-
-    fn insert_flags(&self, flags: ElementFlags) {
-        self.element.insert_atomic_flags(flags);
-    }
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -1009,6 +1013,10 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> {
         self.as_node().type_id()
     }
 
+    unsafe fn unsafe_get(self) -> ServoLayoutElement<'le> {
+        self.element
+    }
+
     fn get_attr<'a>(&'a self, namespace: &Namespace, name: &LocalName) -> Option<&'a str> {
         self.element.get_attr(namespace, name)
     }