Skip to content

Commit

Permalink
style: Distinguish between the tree structures used for traversal and…
Browse files Browse the repository at this point in the history
… selector matching.

This patch renames TNode::parent_element to traversal_parent, since it returns
the parent from the perspective of traversal (which in Gecko uses the
flattened tree).  It also renames TNode::children to traversal_children
for the saem reason.

We keep parent_element and children functions on TNode to use for selector
matching, which must be done on the real DOM tree structure.
  • Loading branch information
heycam committed Jun 9, 2017
1 parent c465dd0 commit c533097
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 67 deletions.
22 changes: 15 additions & 7 deletions components/script/layout_wrapper.rs
Expand Up @@ -165,12 +165,26 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
transmute(node)
}

fn children(self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
fn parent_node(&self) -> Option<Self> {
unsafe {
self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn children(&self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
LayoutIterator(ServoChildrenIterator {
current: self.first_child(),
})
}

fn traversal_parent(&self) -> Option<ServoLayoutElement<'ln>> {
self.parent_element()
}

fn traversal_children(&self) -> LayoutIterator<ServoChildrenIterator<'ln>> {
self.children()
}

fn opaque(&self) -> OpaqueNode {
unsafe { self.get_jsmanaged().opaque() }
}
Expand Down Expand Up @@ -199,12 +213,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> {
self.node.set_flag(CAN_BE_FRAGMENTED, value)
}

fn parent_node(&self) -> Option<ServoLayoutNode<'ln>> {
unsafe {
self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn is_in_doc(&self) -> bool {
unsafe { (*self.node.unsafe_get()).is_in_doc() }
}
Expand Down
18 changes: 9 additions & 9 deletions components/style/bloom.rs
Expand Up @@ -89,7 +89,7 @@ impl<E: TElement> StyleBloom<E> {
pub fn push(&mut self, element: E) {
if cfg!(debug_assertions) {
if self.elements.is_empty() {
assert!(element.parent_element().is_none());
assert!(element.traversal_parent().is_none());
}
}
self.push_internal(element);
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<E: TElement> StyleBloom<E> {
pub fn rebuild(&mut self, mut element: E) {
self.clear();

while let Some(parent) = element.parent_element() {
while let Some(parent) = element.traversal_parent() {
self.push_internal(parent);
element = parent;
}
Expand All @@ -155,7 +155,7 @@ impl<E: TElement> StyleBloom<E> {
pub fn assert_complete(&self, mut element: E) {
if cfg!(debug_assertions) {
let mut checked = 0;
while let Some(parent) = element.parent_element() {
while let Some(parent) = element.traversal_parent() {
assert_eq!(parent, *self.elements[self.elements.len() - 1 - checked]);
element = parent;
checked += 1;
Expand Down Expand Up @@ -190,7 +190,7 @@ impl<E: TElement> StyleBloom<E> {
return;
}

let parent_element = match element.parent_element() {
let traversal_parent = match element.traversal_parent() {
Some(parent) => parent,
None => {
// Yay, another easy case.
Expand All @@ -199,7 +199,7 @@ impl<E: TElement> StyleBloom<E> {
}
};

if self.current_parent() == Some(parent_element) {
if self.current_parent() == Some(traversal_parent) {
// Ta da, cache hit, we're all done.
return;
}
Expand Down Expand Up @@ -232,8 +232,8 @@ impl<E: TElement> StyleBloom<E> {
}

// Now let's try to find a common parent in the bloom filter chain,
// starting with parent_element.
let mut common_parent = parent_element;
// starting with traversal_parent.
let mut common_parent = traversal_parent;
let mut common_parent_depth = element_depth - 1;

// Let's collect the parents we are going to need to insert once we've
Expand All @@ -247,7 +247,7 @@ impl<E: TElement> StyleBloom<E> {
// reverse the slice.
parents_to_insert.push(common_parent);
common_parent =
common_parent.parent_element().expect("We were lied");
common_parent.traversal_parent().expect("We were lied to");
common_parent_depth -= 1;
}

Expand All @@ -269,7 +269,7 @@ impl<E: TElement> StyleBloom<E> {
while **self.elements.last().unwrap() != common_parent {
parents_to_insert.push(common_parent);
self.pop().unwrap();
common_parent = match common_parent.parent_element() {
common_parent = match common_parent.traversal_parent() {
Some(parent) => parent,
None => {
debug_assert!(self.elements.is_empty());
Expand Down
42 changes: 28 additions & 14 deletions components/style/dom.rs
Expand Up @@ -108,17 +108,27 @@ pub trait TNode : Sized + Copy + Clone + Debug + NodeInfo {
/// Get a node back from an `UnsafeNode`.
unsafe fn from_unsafe(n: &UnsafeNode) -> Self;

/// Returns an iterator over this node's children.
fn children(self) -> LayoutIterator<Self::ConcreteChildrenIterator>;

/// Converts self into an `OpaqueNode`.
fn opaque(&self) -> OpaqueNode;
/// Get this node's parent node.
fn parent_node(&self) -> Option<Self>;

/// Get this node's parent element if present.
fn parent_element(&self) -> Option<Self::ConcreteElement> {
self.parent_node().and_then(|n| n.as_element())
}

/// Returns an iterator over this node's children.
fn children(&self) -> LayoutIterator<Self::ConcreteChildrenIterator>;

/// Get this node's parent element from the perspective of a restyle
/// traversal.
fn traversal_parent(&self) -> Option<Self::ConcreteElement>;

/// Get this node's children from the perspective of a restyle traversal.
fn traversal_children(&self) -> LayoutIterator<Self::ConcreteChildrenIterator>;

/// Converts self into an `OpaqueNode`.
fn opaque(&self) -> OpaqueNode;

/// A debug id, only useful, mm... for debugging.
fn debug_id(self) -> usize;

Expand All @@ -138,9 +148,6 @@ pub trait TNode : Sized + Copy + Clone + Debug + NodeInfo {
/// Set whether this node can be fragmented.
unsafe fn set_can_be_fragmented(&self, value: bool);

/// Get this node's parent node.
fn parent_node(&self) -> Option<Self>;

/// Whether this node is in the document right now needed to clear the
/// restyle data appropriately on some forced restyles.
fn is_in_doc(&self) -> bool;
Expand Down Expand Up @@ -222,7 +229,7 @@ fn fmt_subtree<F, N: TNode>(f: &mut fmt::Formatter, stringify: &F, n: N, indent:
try!(write!(f, " "));
}
try!(stringify(f, n));
for kid in n.children() {
for kid in n.traversal_children() {
try!(writeln!(f, ""));
try!(fmt_subtree(f, stringify, kid, indent + 1));
}
Expand Down Expand Up @@ -256,7 +263,7 @@ pub unsafe fn raw_note_descendants<E, B>(element: E) -> bool
break;
}
B::set(el);
curr = el.parent_element();
curr = el.traversal_parent();
}

// Note: We disable this assertion on servo because of bugs. See the
Expand Down Expand Up @@ -297,7 +304,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
fn depth(&self) -> usize {
let mut depth = 0;
let mut curr = *self;
while let Some(parent) = curr.parent_element() {
while let Some(parent) = curr.traversal_parent() {
depth += 1;
curr = parent;
}
Expand All @@ -315,11 +322,18 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
}
}

/// Get this node's parent element from the perspective of a restyle
/// traversal.
fn traversal_parent(&self) -> Option<Self> {
self.as_node().traversal_parent()
}

/// Returns the parent element we should inherit from.
///
/// This is pretty much always the parent element itself, except in the case
/// of Gecko's Native Anonymous Content, which may need to find the closest
/// non-NAC ancestor.
/// of Gecko's Native Anonymous Content, which uses the traversal parent
/// (i.e. the flattened tree parent) and which also may need to find the
/// closest non-NAC ancestor.
fn inheritance_parent(&self) -> Option<Self> {
self.parent_element()
}
Expand Down Expand Up @@ -442,7 +456,7 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
let mut current = Some(*self);
while let Some(el) = current {
if !B::has(el) { return false; }
current = el.parent_element();
current = el.traversal_parent();
}

true
Expand Down
41 changes: 27 additions & 14 deletions components/style/gecko/wrapper.rs
Expand Up @@ -204,6 +204,16 @@ impl<'ln> GeckoNode<'ln> {
true
}

fn flattened_tree_parent(&self) -> Option<Self> {
let fast_path = self.flattened_tree_parent_is_parent();
debug_assert!(fast_path == unsafe { bindings::Gecko_FlattenedTreeParentIsParent(self.0) });
if fast_path {
unsafe { self.0.mParent.as_ref().map(GeckoNode) }
} else {
unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).map(GeckoNode) }
}
}

/// This logic is duplicated in Gecko's nsIContent::IsRootOfNativeAnonymousSubtree.
fn is_root_of_native_anonymous_subtree(&self) -> bool {
use gecko_bindings::structs::NODE_IS_NATIVE_ANONYMOUS_ROOT;
Expand Down Expand Up @@ -240,12 +250,24 @@ impl<'ln> TNode for GeckoNode<'ln> {
GeckoNode(&*(n.0 as *mut RawGeckoNode))
}

fn children(self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
fn parent_node(&self) -> Option<Self> {
unsafe { self.0.mParent.as_ref().map(GeckoNode) }
}

fn children(&self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
LayoutIterator(self.dom_children())
}

fn traversal_parent(&self) -> Option<GeckoElement<'ln>> {
self.flattened_tree_parent().and_then(|n| n.as_element())
}

fn traversal_children(&self) -> LayoutIterator<GeckoChildrenIterator<'ln>> {
let maybe_iter = unsafe { Gecko_MaybeCreateStyleChildrenIterator(self.0) };
if let Some(iter) = maybe_iter.into_owned_opt() {
LayoutIterator(GeckoChildrenIterator::GeckoIterator(iter))
} else {
LayoutIterator(GeckoChildrenIterator::Current(self.first_child()))
LayoutIterator(self.dom_children())
}
}

Expand Down Expand Up @@ -277,16 +299,6 @@ impl<'ln> TNode for GeckoNode<'ln> {
// Maybe this isn’t useful for Gecko?
}

fn parent_node(&self) -> Option<Self> {
let fast_path = self.flattened_tree_parent_is_parent();
debug_assert!(fast_path == unsafe { bindings::Gecko_FlattenedTreeParentIsParent(self.0) });
if fast_path {
unsafe { self.0.mParent.as_ref().map(GeckoNode) }
} else {
unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).map(GeckoNode) }
}
}

fn is_in_doc(&self) -> bool {
unsafe { bindings::Gecko_IsInDocument(self.0) }
}
Expand Down Expand Up @@ -644,9 +656,10 @@ impl<'le> TElement for GeckoElement<'le> {

fn inheritance_parent(&self) -> Option<Self> {
if self.is_native_anonymous() {
return self.closest_non_native_anonymous_ancestor();
self.closest_non_native_anonymous_ancestor()
} else {
self.as_node().flattened_tree_parent().and_then(|n| n.as_element())
}
return self.parent_element();
}

fn closest_non_native_anonymous_ancestor(&self) -> Option<Self> {
Expand Down
2 changes: 1 addition & 1 deletion components/style/invalidation/stylesheets.rs
Expand Up @@ -166,7 +166,7 @@ impl StylesheetInvalidationSet {

let mut any_children_invalid = false;

for child in element.as_node().children() {
for child in element.as_node().traversal_children() {
let child = match child.as_element() {
Some(e) => e,
None => continue,
Expand Down
2 changes: 1 addition & 1 deletion components/style/matching.rs
Expand Up @@ -230,7 +230,7 @@ trait PrivateMatchMethods: TElement {
fn layout_parent(&self) -> Self {
let mut current = self.clone();
loop {
current = match current.parent_element() {
current = match current.traversal_parent() {
Some(el) => el,
None => return current,
};
Expand Down
2 changes: 1 addition & 1 deletion components/style/parallel.rs
Expand Up @@ -64,7 +64,7 @@ pub fn traverse_dom<E, D>(traversal: &D,
// it on the context to make it available to the bottom-up phase.
let depth = if token.traverse_unstyled_children_only() {
debug_assert!(!D::needs_postorder_traversal());
for kid in root.as_node().children() {
for kid in root.as_node().traversal_children() {
if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
nodes.push(unsafe { SendNode::new(kid) });
}
Expand Down
2 changes: 1 addition & 1 deletion components/style/sequential.rs
Expand Up @@ -32,7 +32,7 @@ pub fn traverse_dom<E, D>(traversal: &D,
let root_depth = root.depth();

if token.traverse_unstyled_children_only() {
for kid in root.as_node().children() {
for kid in root.as_node().traversal_children() {
if kid.as_element().map_or(false, |el| el.get_data().is_none()) {
discovered.push_back(WorkItem(kid, root_depth + 1));
}
Expand Down
14 changes: 7 additions & 7 deletions components/style/sharing/mod.rs
Expand Up @@ -185,10 +185,10 @@ impl ValidationData {
// filter, to avoid thrashing the filter.
let bloom_to_use = if bloom_known_valid {
debug_assert_eq!(bloom.current_parent(),
element.parent_element());
element.traversal_parent());
Some(bloom.filter())
} else {
if bloom.current_parent() == element.parent_element() {
if bloom.current_parent() == element.traversal_parent() {
Some(bloom.filter())
} else {
None
Expand Down Expand Up @@ -344,7 +344,7 @@ impl<E: TElement> StyleSharingTarget<E> {
return StyleSharingResult::CannotShare;
}
debug_assert_eq!(bloom_filter.current_parent(),
self.element.parent_element());
self.element.traversal_parent());

let result = cache
.share_style_if_possible(shared_context,
Expand Down Expand Up @@ -500,7 +500,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
dom_depth: usize) {
use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS;

let parent = match element.parent_element() {
let parent = match element.traversal_parent() {
Some(element) => element,
None => {
debug!("Failing to insert to the cache: no parent element");
Expand Down Expand Up @@ -572,7 +572,7 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
return StyleSharingResult::CannotShare
}

if target.parent_element().is_none() {
if target.traversal_parent().is_none() {
debug!("{:?} Cannot share style: element has no parent",
target.element);
return StyleSharingResult::CannotShare
Expand Down Expand Up @@ -644,8 +644,8 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
// Check that we have the same parent, or at least the same pointer
// identity for parent computed style. The latter check allows us to
// share style between cousins if the parents shared style.
let parent = target.parent_element();
let candidate_parent = candidate.element.parent_element();
let parent = target.traversal_parent();
let candidate_parent = candidate.element.traversal_parent();
if parent != candidate_parent &&
!checks::same_computed_values(parent, candidate_parent) {
miss!(Parent)
Expand Down

0 comments on commit c533097

Please sign in to comment.