Skip to content

Commit

Permalink
Auto merge of #18053 - bholley:descendants_bit_refactor, r=emilio
Browse files Browse the repository at this point in the history
Overhaul dirty descendants propagation in preparation for restyle roots

https://bugzilla.mozilla.org/show_bug.cgi?id=1389385

<!-- 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/18053)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Aug 12, 2017
2 parents c3bbe92 + 57d8ec9 commit faad7ff
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 162 deletions.
16 changes: 1 addition & 15 deletions components/layout_thread/dom_wrapper.rs
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
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
7 changes: 5 additions & 2 deletions components/style/gecko/generated/bindings.rs
Expand Up @@ -1082,8 +1082,11 @@ extern "C" {
pub fn Gecko_UnsetNodeFlags(node: RawGeckoNodeBorrowed, flags: u32);
}
extern "C" {
pub fn Gecko_SetOwnerDocumentNeedsStyleFlush(element:
RawGeckoElementBorrowed);
pub fn Gecko_NoteDirtyElement(element: RawGeckoElementBorrowed);
}
extern "C" {
pub fn Gecko_NoteAnimationOnlyDirtyElement(element:
RawGeckoElementBorrowed);
}
extern "C" {
pub fn Gecko_GetStyleContext(element: RawGeckoElementBorrowed,
Expand Down

0 comments on commit faad7ff

Please sign in to comment.