Skip to content

Commit

Permalink
Do all descendant bit propagation from Gecko over FFI.
Browse files Browse the repository at this point in the history 
Deduplicating code is nice, and it will help us when we make the bit
propagation more complicated in upcoming patches.

MozReview-Commit-ID: KIQnNJVayrM
  • Loading branch information
@bholley
bholley committed Aug 12, 2017
1 parent 8fb7836 commit 24a52b7
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 98 deletions.
16 changes: 1 addition & 15 deletions components/layout_thread/dom_wrapper.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -469,11 +468,6 @@ impl<'le> TElement for ServoLayoutElement<'le> {
self.as_node().node.set_flag(HANDLED_SNAPSHOT, true);
}

unsafe fn note_descendants<B: DescendantsBit<Self>>(&self) {
debug_assert!(self.get_data().is_some());
style::dom::raw_note_descendants::<Self, B>(*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)
Expand Down Expand Up @@ -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;

Expand All @@ -644,8 +632,6 @@ impl<'le> ServoLayoutElement<'le> {
el.set_dirty_descendants();
current = el.parent_element();
}

debug_assert!(self.descendants_bit_is_propagated::<DirtyDescendants>());
}
}

Expand Down
58 changes: 0 additions & 58 deletions components/style/dom.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -240,44 +239,6 @@ fn fmt_subtree<F, N: TNode>(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<E, B>(element: E) -> bool
where E: TElement,
B: DescendantsBit<E>,
{
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::<B>());
}

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,
Expand Down Expand Up @@ -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<B: DescendantsBit<Self>>(&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<B: DescendantsBit<Self>>(&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.
Expand Down
32 changes: 7 additions & 25 deletions components/style/gecko/wrapper.rs
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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::<AnimationOnlyDirtyDescendants>();
bindings::Gecko_NoteAnimationOnlyDirtyDescendants(p.0);
} else {
p.note_descendants::<DirtyDescendants>();
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)
Expand Down Expand Up @@ -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<B: DescendantsBit<Self>>(&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, B>(*self) {
bindings::Gecko_SetOwnerDocumentNeedsStyleFlush(self.0);
}
}

unsafe fn unset_dirty_descendants(&self) {
self.unset_flags(ELEMENT_HAS_DIRTY_DESCENDANTS_FOR_SERVO as u32)
}
Expand Down

0 comments on commit 24a52b7

Please sign in to comment.