From 24a52b7990de55c9383e035f3eef773ec58967a7 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 18 Jul 2017 16:17:02 -0700 Subject: [PATCH] Do all descendant bit propagation from Gecko over FFI. Deduplicating code is nice, and it will help us when we make the bit propagation more complicated in upcoming patches. MozReview-Commit-ID: KIQnNJVayrM --- components/layout_thread/dom_wrapper.rs | 16 +------ components/style/dom.rs | 58 ------------------------- components/style/gecko/wrapper.rs | 32 +++----------- 3 files changed, 8 insertions(+), 98 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index 0981ec42ce27..beb929d5e166 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -63,14 +63,13 @@ use std::hash::{Hash, Hasher}; use std::marker::PhantomData; use std::mem::transmute; use std::sync::atomic::Ordering; -use style; use style::CaseSensitivityExt; use style::applicable_declarations::ApplicableDeclarationBlock; use style::attr::AttrValue; use style::computed_values::display; use style::context::{QuirksMode, SharedStyleContext}; use style::data::ElementData; -use style::dom::{DescendantsBit, DirtyDescendants, LayoutIterator, NodeInfo, OpaqueNode}; +use style::dom::{LayoutIterator, NodeInfo, OpaqueNode}; use style::dom::{PresentationalHintsSynthesizer, TElement, TNode, UnsafeNode}; use style::element_state::*; use style::font_metrics::ServoMetricsProvider; @@ -469,11 +468,6 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.as_node().node.set_flag(HANDLED_SNAPSHOT, true); } - 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) @@ -625,12 +619,6 @@ impl<'le> ServoLayoutElement<'le> { self.as_node().node.set_flag(HAS_SNAPSHOT, true); } - // 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; @@ -644,8 +632,6 @@ impl<'le> ServoLayoutElement<'le> { el.set_dirty_descendants(); current = el.parent_element(); } - - debug_assert!(self.descendants_bit_is_propagated::()); } } diff --git a/components/style/dom.rs b/components/style/dom.rs index 90644e2b9b77..d2ed5ae3d99e 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -33,7 +33,6 @@ use std::fmt::Debug; use std::hash::Hash; use std::ops::Deref; use stylist::Stylist; -use thread_state; use traversal_flags::{TraversalFlags, self}; pub use style_traits::UnsafeNode; @@ -240,44 +239,6 @@ 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.traversal_parent(); - } - - // 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 PresentationalHintsSynthesizer { /// Generate the proper applicable declarations due to presentational hints, @@ -501,30 +462,11 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone + data.has_styles() && !data.restyle.hint.has_non_animation_invalidations() } - /// 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); - /// Debug helper to be sure the bit is propagated. - fn descendants_bit_is_propagated>(&self) -> bool { - let mut current = Some(*self); - while let Some(el) = current { - if !B::has(el) { return false; } - current = el.traversal_parent(); - } - - true - } - /// Flag that this element has no descendant for style processing. /// /// Only safe to call with exclusive access to the element. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 0ecbf8abe7bc..7c31ed29e31c 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -20,7 +20,7 @@ use applicable_declarations::ApplicableDeclarationBlock; use atomic_refcell::{AtomicRefCell, AtomicRefMut}; use context::{QuirksMode, SharedStyleContext, PostAnimationTasks, UpdateAnimationsTasks}; use data::{ElementData, RestyleData}; -use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; +use dom::{LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode}; use dom::{OpaqueNode, PresentationalHintsSynthesizer}; use element_state::{ElementState, DocumentState, NS_DOCUMENT_STATE_WINDOW_INACTIVE}; use error_reporting::ParseErrorReporter; @@ -685,8 +685,6 @@ impl<'le> GeckoElement<'le> { unsafe fn maybe_restyle<'a>(&self, data: &'a mut ElementData, animation_only: bool) -> Option<&'a mut RestyleData> { - use dom::{AnimationOnlyDirtyDescendants, DirtyDescendants}; - // Don't generate a useless RestyleData if the element hasn't been styled. if !data.has_styles() { return None; @@ -695,13 +693,14 @@ impl<'le> GeckoElement<'le> { // Propagate the bit up the chain. if let Some(p) = self.traversal_parent() { if animation_only { - p.note_descendants::(); + bindings::Gecko_NoteAnimationOnlyDirtyDescendants(p.0); } else { - p.note_descendants::(); + bindings::Gecko_NoteDirtyDescendants(p.0); } - }; - - bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0); + } else { + // If there's no parent, we still need to trigger the style flush. + bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0); + } // Ensure and return the RestyleData. Some(&mut data.restyle) @@ -1042,23 +1041,6 @@ impl<'le> TElement for GeckoElement<'le> { self.set_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32) } - unsafe fn note_descendants>(&self) { - // FIXME(emilio): We seem to reach this in Gecko's - // layout/style/test/test_pseudoelement_state.html, while changing the - // state of an anonymous content element which is styled, but whose - // parent isn't, presumably because we've cleared the data and haven't - // reached it yet. - // - // Otherwise we should be able to assert this. - if self.get_data().is_none() { - return; - } - - if dom::raw_note_descendants::(*self) { - bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0); - } - } - unsafe fn unset_dirty_descendants(&self) { self.unset_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32) }