From 470368ecce2308938c4270d1d892ddc1e8f73d28 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 10 Aug 2016 13:34:31 -0700 Subject: [PATCH 1/4] Move rev_children and traverse_preorder to LayoutNode. These impose additional requirements on the traversal code that we want to avoid for Gecko (for now). --- .../script_layout_interface/wrapper_traits.rs | 53 +++++++++++++++++++ components/style/dom.rs | 52 ------------------ 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index bc64480b424..3c603e83396 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -80,8 +80,61 @@ pub trait LayoutNode: TNode { fn init_style_and_layout_data(&self, data: OpaqueStyleAndLayoutData); fn get_style_and_layout_data(&self) -> Option; + + fn rev_children(self) -> ReverseChildrenIterator { + ReverseChildrenIterator { + current: self.last_child(), + } + } + + fn traverse_preorder(self) -> TreeIterator { + TreeIterator::new(self) + } } +pub struct ReverseChildrenIterator where ConcreteNode: TNode { + current: Option, +} + +impl Iterator for ReverseChildrenIterator + where ConcreteNode: TNode { + type Item = ConcreteNode; + fn next(&mut self) -> Option { + let node = self.current; + self.current = node.and_then(|node| node.prev_sibling()); + node + } +} + +pub struct TreeIterator where ConcreteNode: TNode { + stack: Vec, +} + +impl TreeIterator where ConcreteNode: LayoutNode { + fn new(root: ConcreteNode) -> TreeIterator { + let mut stack = vec![]; + stack.push(root); + TreeIterator { + stack: stack, + } + } + + pub fn next_skipping_children(&mut self) -> Option { + self.stack.pop() + } +} + +impl Iterator for TreeIterator + where ConcreteNode: LayoutNode { + type Item = ConcreteNode; + fn next(&mut self) -> Option { + let ret = self.stack.pop(); + ret.map(|node| self.stack.extend(node.rev_children())); + ret + } +} + + /// A thread-safe version of `LayoutNode`, used during flow construction. This type of layout /// node does not allow any parents or siblings of nodes to be accessed, to avoid races. pub trait ThreadSafeLayoutNode: Clone + Copy + Sized + PartialEq { diff --git a/components/style/dom.rs b/components/style/dom.rs index 3b0940c59ef..feae36d8ebf 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -83,10 +83,6 @@ pub trait TNode : Sized + Copy + Clone { fn dump_style(self); - fn traverse_preorder(self) -> TreeIterator { - TreeIterator::new(self) - } - /// Returns an iterator over this node's children. fn children(self) -> ChildrenIterator { ChildrenIterator { @@ -94,12 +90,6 @@ pub trait TNode : Sized + Copy + Clone { } } - fn rev_children(self) -> ReverseChildrenIterator { - ReverseChildrenIterator { - current: self.last_child(), - } - } - /// Converts self into an `OpaqueNode`. fn opaque(&self) -> OpaqueNode; @@ -255,34 +245,6 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre } } -pub struct TreeIterator where ConcreteNode: TNode { - stack: Vec, -} - -impl TreeIterator where ConcreteNode: TNode { - fn new(root: ConcreteNode) -> TreeIterator { - let mut stack = vec![]; - stack.push(root); - TreeIterator { - stack: stack, - } - } - - pub fn next_skipping_children(&mut self) -> Option { - self.stack.pop() - } -} - -impl Iterator for TreeIterator - where ConcreteNode: TNode { - type Item = ConcreteNode; - fn next(&mut self) -> Option { - let ret = self.stack.pop(); - ret.map(|node| self.stack.extend(node.rev_children())); - ret - } -} - pub struct ChildrenIterator where ConcreteNode: TNode { current: Option, } @@ -296,17 +258,3 @@ impl Iterator for ChildrenIterator node } } - -pub struct ReverseChildrenIterator where ConcreteNode: TNode { - current: Option, -} - -impl Iterator for ReverseChildrenIterator - where ConcreteNode: TNode { - type Item = ConcreteNode; - fn next(&mut self) -> Option { - let node = self.current; - self.current = node.and_then(|node| node.prev_sibling()); - node - } -} From b56297f2a57eb79ed47b3aa3a1daf82f29d36b4b Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 10 Aug 2016 15:07:36 -0700 Subject: [PATCH 2/4] Make ChildrenIterator concrete. This will allow us to specialize ChildrenIterator in the Gecko case to do something more interesting in some cases. --- components/script/layout_wrapper.rs | 20 ++++++++++++++++++++ components/style/dom.rs | 21 ++------------------- ports/geckolib/wrapper.rs | 20 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index b4e54fe5bd9..47ecc910345 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -115,6 +115,7 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { type ConcreteElement = ServoLayoutElement<'ln>; type ConcreteDocument = ServoLayoutDocument<'ln>; type ConcreteRestyleDamage = RestyleDamage; + type ConcreteChildrenIterator = ServoChildrenIterator<'ln>; fn to_unsafe(&self) -> UnsafeNode { unsafe { @@ -147,6 +148,12 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.dump_style_indent(0); } + fn children(self) -> ServoChildrenIterator<'ln> { + ServoChildrenIterator { + current: self.first_child(), + } + } + fn opaque(&self) -> OpaqueNode { unsafe { self.get_jsmanaged().opaque() } } @@ -280,6 +287,19 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { } } +pub struct ServoChildrenIterator<'a> { + current: Option>, +} + +impl<'a> Iterator for ServoChildrenIterator<'a> { + type Item = ServoLayoutNode<'a>; + fn next(&mut self) -> Option> { + let node = self.current; + self.current = node.and_then(|node| node.next_sibling()); + node + } +} + impl<'ln> LayoutNode for ServoLayoutNode<'ln> { type ConcreteThreadSafeLayoutNode = ServoThreadSafeLayoutNode<'ln>; diff --git a/components/style/dom.rs b/components/style/dom.rs index feae36d8ebf..41bab451b76 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -68,6 +68,7 @@ pub trait TNode : Sized + Copy + Clone { type ConcreteElement: TElement; type ConcreteDocument: TDocument; type ConcreteRestyleDamage: TRestyleDamage; + type ConcreteChildrenIterator: Iterator; fn to_unsafe(&self) -> UnsafeNode; unsafe fn from_unsafe(n: &UnsafeNode) -> Self; @@ -84,11 +85,7 @@ pub trait TNode : Sized + Copy + Clone { fn dump_style(self); /// Returns an iterator over this node's children. - fn children(self) -> ChildrenIterator { - ChildrenIterator { - current: self.first_child(), - } - } + fn children(self) -> Self::ConcreteChildrenIterator; /// Converts self into an `OpaqueNode`. fn opaque(&self) -> OpaqueNode; @@ -244,17 +241,3 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre } } } - -pub struct ChildrenIterator where ConcreteNode: TNode { - current: Option, -} - -impl Iterator for ChildrenIterator - where ConcreteNode: TNode { - type Item = ConcreteNode; - fn next(&mut self) -> Option { - let node = self.current; - self.current = node.and_then(|node| node.next_sibling()); - node - } -} diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index b5b57d27c88..961fc5b06ef 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -134,6 +134,7 @@ impl<'ln> TNode for GeckoNode<'ln> { type ConcreteDocument = GeckoDocument<'ln>; type ConcreteElement = GeckoElement<'ln>; type ConcreteRestyleDamage = GeckoRestyleDamage; + type ConcreteChildrenIterator = GeckoChildrenIterator<'ln>; fn to_unsafe(&self) -> UnsafeNode { (self.node as usize, 0) @@ -163,6 +164,12 @@ impl<'ln> TNode for GeckoNode<'ln> { unimplemented!() } + fn children(self) -> GeckoChildrenIterator<'ln> { + GeckoChildrenIterator { + current: self.first_child(), + } + } + fn opaque(&self) -> OpaqueNode { let ptr: uintptr_t = self.node as uintptr_t; OpaqueNode(ptr) @@ -341,6 +348,19 @@ impl<'ln> TNode for GeckoNode<'ln> { unsafe fn set_dirty_on_viewport_size_changed(&self) {} } +pub struct GeckoChildrenIterator<'a> { + current: Option>, +} + +impl<'a> Iterator for GeckoChildrenIterator<'a> { + type Item = GeckoNode<'a>; + fn next(&mut self) -> Option> { + let node = self.current; + self.current = node.and_then(|node| node.next_sibling()); + node + } +} + #[derive(Clone, Copy)] pub struct GeckoDocument<'ld> { document: *mut RawGeckoDocument, From 1799b0a5dfd0923977cad3f5179d287918d371d3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 10 Aug 2016 15:17:16 -0700 Subject: [PATCH 3/4] Remove unused children_count method from TNode. The semantics of this method with respect to anonymous children are unclear. Thankfully it's unused, so we can remove it. --- components/script/layout_wrapper.rs | 4 ---- components/style/dom.rs | 2 -- ports/geckolib/wrapper.rs | 7 ------- 3 files changed, 13 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 47ecc910345..906bde60550 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -170,10 +170,6 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.opaque().0 } - fn children_count(&self) -> u32 { - unsafe { self.node.children_count() } - } - fn as_element(&self) -> Option> { as_element(self.node) } diff --git a/components/style/dom.rs b/components/style/dom.rs index 41bab451b76..ce7ba70a1bf 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -100,8 +100,6 @@ pub trait TNode : Sized + Copy + Clone { fn as_document(&self) -> Option; - fn children_count(&self) -> u32; - fn has_changed(&self) -> bool; unsafe fn set_changed(&self, value: bool); diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 961fc5b06ef..5258caa36b4 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -5,7 +5,6 @@ #![allow(unsafe_code)] use gecko_bindings::bindings; -use gecko_bindings::bindings::Gecko_ChildrenCount; use gecko_bindings::bindings::Gecko_ClassOrClassList; use gecko_bindings::bindings::Gecko_GetNodeData; use gecko_bindings::bindings::Gecko_GetStyleContext; @@ -187,12 +186,6 @@ impl<'ln> TNode for GeckoNode<'ln> { unimplemented!() } - fn children_count(&self) -> u32 { - unsafe { - Gecko_ChildrenCount(self.node) - } - } - fn as_element(&self) -> Option> { if self.is_element() { unsafe { Some(GeckoElement::from_raw(self.node as *mut RawGeckoElement)) } From 122df8ca6052c356fca901d8043d29e358a1a0fb Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 10 Aug 2016 17:52:19 -0700 Subject: [PATCH 4/4] Implement GeckoChildrenIterator to traverse anonymous children. --- ports/geckolib/gecko_bindings/bindings.rs | 6 ++++ ports/geckolib/wrapper.rs | 41 ++++++++++++++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/ports/geckolib/gecko_bindings/bindings.rs b/ports/geckolib/gecko_bindings/bindings.rs index 544a0f70285..2b208d751d5 100644 --- a/ports/geckolib/gecko_bindings/bindings.rs +++ b/ports/geckolib/gecko_bindings/bindings.rs @@ -168,6 +168,7 @@ pub enum RawServoStyleSheet { } pub enum RawServoStyleSet { } pub enum nsHTMLCSSStyleSheet { } pub enum ServoDeclarationBlock { } +pub enum StyleChildrenIterator { } pub type ThreadSafePrincipalHolder = nsMainThreadPtrHolder; pub type ThreadSafeURIHolder = nsMainThreadPtrHolder; extern "C" { @@ -190,6 +191,11 @@ extern "C" { -> *mut RawGeckoElement; pub fn Gecko_GetDocumentElement(document: *mut RawGeckoDocument) -> *mut RawGeckoElement; + pub fn Gecko_MaybeCreateStyleChildrenIterator(node: *mut RawGeckoNode) + -> *mut StyleChildrenIterator; + pub fn Gecko_DropStyleChildrenIterator(it: *mut StyleChildrenIterator); + pub fn Gecko_GetNextStyleChild(it: *mut StyleChildrenIterator) + -> *mut RawGeckoNode; pub fn Gecko_ElementState(element: *mut RawGeckoElement) -> u8; pub fn Gecko_IsHTMLElementInHTMLDocument(element: *mut RawGeckoElement) -> bool; diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 5258caa36b4..c168b1dcaaa 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -10,10 +10,11 @@ use gecko_bindings::bindings::Gecko_GetNodeData; use gecko_bindings::bindings::Gecko_GetStyleContext; use gecko_bindings::bindings::ServoNodeData; use gecko_bindings::bindings::{Gecko_CalcStyleDifference, Gecko_StoreStyleDifference}; +use gecko_bindings::bindings::{Gecko_DropStyleChildrenIterator, Gecko_MaybeCreateStyleChildrenIterator}; use gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentElement}; use gecko_bindings::bindings::{Gecko_GetFirstChild, Gecko_GetFirstChildElement}; use gecko_bindings::bindings::{Gecko_GetLastChild, Gecko_GetLastChildElement}; -use gecko_bindings::bindings::{Gecko_GetNextSibling, Gecko_GetNextSiblingElement}; +use gecko_bindings::bindings::{Gecko_GetNextSibling, Gecko_GetNextSiblingElement, Gecko_GetNextStyleChild}; use gecko_bindings::bindings::{Gecko_GetNodeFlags, Gecko_SetNodeFlags, Gecko_UnsetNodeFlags}; use gecko_bindings::bindings::{Gecko_GetParentElement, Gecko_GetParentNode}; use gecko_bindings::bindings::{Gecko_GetPrevSibling, Gecko_GetPrevSiblingElement}; @@ -164,8 +165,11 @@ impl<'ln> TNode for GeckoNode<'ln> { } fn children(self) -> GeckoChildrenIterator<'ln> { - GeckoChildrenIterator { - current: self.first_child(), + let maybe_iter = unsafe { Gecko_MaybeCreateStyleChildrenIterator(self.node) }; + if !maybe_iter.is_null() { + GeckoChildrenIterator::GeckoIterator(maybe_iter) + } else { + GeckoChildrenIterator::Current(self.first_child()) } } @@ -341,16 +345,37 @@ impl<'ln> TNode for GeckoNode<'ln> { unsafe fn set_dirty_on_viewport_size_changed(&self) {} } -pub struct GeckoChildrenIterator<'a> { - current: Option>, +// We generally iterate children by traversing the siblings of the first child +// like Servo does. However, for nodes with anonymous children, we use a custom +// (heavier-weight) Gecko-implemented iterator. +pub enum GeckoChildrenIterator<'a> { + Current(Option>), + GeckoIterator(*mut bindings::StyleChildrenIterator), +} + +impl<'a> Drop for GeckoChildrenIterator<'a> { + fn drop(&mut self) { + if let GeckoChildrenIterator::GeckoIterator(it) = *self { + unsafe { + Gecko_DropStyleChildrenIterator(it); + } + } + } } impl<'a> Iterator for GeckoChildrenIterator<'a> { type Item = GeckoNode<'a>; fn next(&mut self) -> Option> { - let node = self.current; - self.current = node.and_then(|node| node.next_sibling()); - node + match *self { + GeckoChildrenIterator::Current(curr) => { + let next = curr.and_then(|node| node.next_sibling()); + *self = GeckoChildrenIterator::Current(next); + curr + }, + GeckoChildrenIterator::GeckoIterator(it) => unsafe { + Gecko_GetNextStyleChild(it).as_ref().map(|n| GeckoNode::from_ref(n)) + } + } } }