diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 661d3289671..9a672ae7da9 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -59,13 +59,13 @@ use std::marker::PhantomData; use std::mem::transmute; use std::sync::Arc; use std::sync::atomic::Ordering; +use style; use style::attr::AttrValue; use style::computed_values::display; use style::context::{QuirksMode, SharedStyleContext}; use style::data::ElementData; -use style::dom::{DirtyDescendants, LayoutIterator, NodeInfo, OpaqueNode, PresentationalHintsSynthetizer}; -use style::dom::{TElement, TNode}; -use style::dom::UnsafeNode; +use style::dom::{DescendantsBit, DirtyDescendants, LayoutIterator, NodeInfo, OpaqueNode}; +use style::dom::{PresentationalHintsSynthetizer, TElement, TNode, UnsafeNode}; use style::element_state::*; use style::properties::{ComputedValues, PropertyDeclarationBlock}; use style::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl}; @@ -409,6 +409,11 @@ impl<'le> TElement for ServoLayoutElement<'le> { unsafe { self.as_node().node.get_flag(HAS_DIRTY_DESCENDANTS) } } + unsafe fn note_descendants>(&self) { + debug_assert!(self.get_data().is_some()); + style::dom::raw_note_descendants::(*self); + } + unsafe fn set_dirty_descendants(&self) { debug_assert!(self.as_node().node.get_flag(IS_IN_DOC)); self.as_node().node.set_flag(HAS_DIRTY_DESCENDANTS, true) @@ -488,19 +493,22 @@ impl<'le> ServoLayoutElement<'le> { } } - // FIXME(bholley): This should be merged with TElement::note_dirty_descendants, + // FIXME(bholley): This should be merged with TElement::note_descendants, // but that requires re-testing and possibly fixing the broken callers given // the FIXME below, which I don't have time to do right now. + // + // FIXME(emilio): We'd also need to relax the invariant in note_descendants + // re. the data already available I think. pub unsafe fn note_dirty_descendant(&self) { use ::selectors::Element; let mut current = Some(*self); while let Some(el) = current { // FIXME(bholley): Ideally we'd have the invariant that any element - // with has_dirty_descendants also has the bit set on all its ancestors. - // However, there are currently some corner-cases where we get that wrong. - // I have in-flight patches to fix all this stuff up, so we just always - // propagate this bit for now. + // with has_dirty_descendants also has the bit set on all its + // ancestors. However, there are currently some corner-cases where + // we get that wrong. I have in-flight patches to fix all this + // stuff up, so we just always propagate this bit for now. el.set_dirty_descendants(); current = el.parent_element(); } diff --git a/components/style/dom.rs b/components/style/dom.rs index ab6ed9aed30..31319e3655a 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -222,6 +222,44 @@ fn fmt_subtree(f: &mut fmt::Formatter, stringify: &F, n: N, indent: Ok(()) } +/// Flag that this element has a descendant for style processing, propagating +/// the bit up to the root as needed. +/// +/// This is _not_ safe to call during the parallel traversal. +/// +/// This is intended as a helper so Servo and Gecko can override it with custom +/// stuff if needed. +/// +/// Returns whether no parent had already noted it, that is, whether we reached +/// the root during the walk up. +pub unsafe fn raw_note_descendants(element: E) -> bool + where E: TElement, + B: DescendantsBit, +{ + debug_assert!(!thread_state::get().is_worker()); + // TODO(emilio, bholley): Documenting the flags setup a bit better wouldn't + // really hurt I guess. + debug_assert!(element.get_data().is_some(), + "You should ensure you only flag styled elements"); + + let mut curr = Some(element); + while let Some(el) = curr { + if B::has(el) { + break; + } + B::set(el); + curr = el.parent_element(); + } + + // Note: We disable this assertion on servo because of bugs. See the + // comment around note_dirty_descendant in layout/wrapper.rs. + if cfg!(feature = "gecko") { + debug_assert!(element.descendants_bit_is_propagated::()); + } + + curr.is_none() +} + /// A trait used to synthesize presentational hints for HTML element attributes. pub trait PresentationalHintsSynthetizer { /// Generate the proper applicable declarations due to presentational hints, @@ -301,30 +339,19 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre /// the actual restyle traversal. fn has_dirty_descendants(&self) -> bool; + /// Flags an element and its ancestors with a given `DescendantsBit`. + /// + /// TODO(emilio): We call this conservatively from restyle_element_internal + /// because we never flag unstyled stuff. A different setup for this may be + /// a bit cleaner, but it's probably not worth to invest on it right now + /// unless necessary. + unsafe fn note_descendants>(&self); + /// Flag that this element has a descendant for style processing. /// /// Only safe to call with exclusive access to the element. unsafe fn set_dirty_descendants(&self); - /// Flag that this element has a descendant for style processing, propagating - /// the bit up to the root as needed. - /// - /// This is _not_ safe to call during the parallel traversal. - unsafe fn note_descendants>(&self) { - debug_assert!(!thread_state::get().is_worker()); - let mut curr = Some(*self); - while curr.is_some() && !B::has(curr.unwrap()) { - B::set(curr.unwrap()); - curr = curr.unwrap().parent_element(); - } - - // Note: We disable this assertion on servo because of bugs. See the - // comment around note_dirty_descendant in layout/wrapper.rs. - if cfg!(feature = "gecko") { - debug_assert!(self.descendants_bit_is_propagated::()); - } - } - /// Debug helper to be sure the bit is propagated. fn descendants_bit_is_propagated>(&self) -> bool { let mut current = Some(*self); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 2fee62cb2b6..3ec34e89927 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -17,7 +17,7 @@ use atomic_refcell::AtomicRefCell; use context::UpdateAnimationsTasks; use data::ElementData; -use dom::{AnimationRules, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; +use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthetizer}; use element_state::ElementState; use error_reporting::StdoutErrorReporter; @@ -504,10 +504,18 @@ impl<'le> TElement for GeckoElement<'le> { } unsafe fn set_dirty_descendants(&self) { + debug_assert!(self.get_data().is_some()); debug!("Setting dirty descendants: {:?}", self); self.set_flags(NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32) } + unsafe fn note_descendants>(&self) { + debug_assert!(self.get_data().is_some()); + if dom::raw_note_descendants::(*self) { + bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0); + } + } + unsafe fn unset_dirty_descendants(&self) { self.unset_flags(NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32) }