From 244bd11f3fb2b5f765b3f86e9f85cbaebe06bced Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sun, 25 Oct 2015 11:13:37 +0100 Subject: [PATCH 1/5] Privatize LayoutNode::chain. There is no reason for this field to be public. --- components/layout/wrapper.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 38c23cd0b47..61bd20204bb 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -81,7 +81,7 @@ pub struct LayoutNode<'a> { node: LayoutJS, /// Being chained to a PhantomData prevents `LayoutNode`s from escaping. - pub chain: PhantomData<&'a ()>, + chain: PhantomData<&'a ()>, } impl<'a> PartialEq for LayoutNode<'a> { From 06f9a2bc34836bee1cf8376ac8f8c1e985f00971 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sun, 25 Oct 2015 11:13:56 +0100 Subject: [PATCH 2/5] Implement a LayoutNode::new function. --- components/layout/wrapper.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 61bd20204bb..9a936433d79 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -54,6 +54,7 @@ use script::dom::htmltextareaelement::{HTMLTextAreaElement, LayoutHTMLTextAreaEl use script::dom::node::{HAS_CHANGED, HAS_DIRTY_DESCENDANTS, IS_DIRTY}; use script::dom::node::{LayoutNodeHelpers, Node, SharedLayoutData}; use script::dom::text::Text; +use script::layout_interface::TrustedNodeAddress; use selectors::matching::DeclarationBlock; use selectors::parser::{AttrSelector, NamespaceConstraint}; use smallvec::VecLike; @@ -92,6 +93,14 @@ impl<'a> PartialEq for LayoutNode<'a> { } impl<'ln> LayoutNode<'ln> { + pub unsafe fn new(address: &TrustedNodeAddress) -> LayoutNode { + let node = LayoutJS::from_trusted_node_address(*address); + LayoutNode { + node: node, + chain: PhantomData, + } + } + /// Creates a new layout node with the same lifetime as this layout node. pub unsafe fn new_with_this_lifetime(&self, node: &LayoutJS) -> LayoutNode<'ln> { LayoutNode { From ee0b0c81d1b735bea149afca4c326be360d7d9d9 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sun, 25 Oct 2015 11:22:27 +0100 Subject: [PATCH 3/5] Pass a LayoutNode to LayoutTask::dirty_all_nodes. There is no point in passing a mutable reference to what is essentially already a pointer. --- components/layout/layout_task.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 638488452f9..ad7bf3942ff 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -1144,7 +1144,7 @@ impl LayoutTask { transmute(&mut doc) }; - let mut node: LayoutNode = match doc.root_node() { + let node: LayoutNode = match doc.root_node() { None => return, Some(x) => x, }; @@ -1190,7 +1190,7 @@ impl LayoutTask { let needs_reflow = screen_size_changed && !needs_dirtying; unsafe { if needs_dirtying { - LayoutTask::dirty_all_nodes(&mut node); + LayoutTask::dirty_all_nodes(node); } } if needs_reflow { @@ -1466,7 +1466,7 @@ impl LayoutTask { } } - unsafe fn dirty_all_nodes(node: &mut LayoutNode) { + unsafe fn dirty_all_nodes(node: LayoutNode) { for node in node.traverse_preorder() { // TODO(cgaebel): mark nodes which are sensitive to media queries as // "changed": From 1eb8900f486e681888f0e5dccd7e677cd17ecadd Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 29 Oct 2015 16:54:20 +0100 Subject: [PATCH 4/5] Simplify LayoutDocument::root_node. --- components/layout/wrapper.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 9a936433d79..72973633067 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -369,11 +369,7 @@ impl<'le> LayoutDocument<'le> { } pub fn root_node(&self) -> Option> { - let mut node = self.as_node().first_child(); - while node.is_some() && !node.unwrap().is_element() { - node = node.unwrap().next_sibling(); - } - node + self.as_node().children().find(LayoutNode::is_element) } pub fn drain_event_state_changes(&self) -> Vec<(LayoutElement, EventState)> { From cce510900e9d1f95990556135b285e4b10f8eddd Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 29 Oct 2015 16:55:27 +0100 Subject: [PATCH 5/5] Use LayoutNode::new to avoid transmute calls in LayoutTask. --- components/layout/layout_task.rs | 34 +++++++++----------------------- components/layout/wrapper.rs | 9 +++++++++ 2 files changed, 18 insertions(+), 25 deletions(-) diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index ad7bf3942ff..f4ea681e24a 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -51,9 +51,7 @@ use profile_traits::time::{self, ProfilerMetadata, profile}; use query::{LayoutRPCImpl, process_content_box_request, process_content_boxes_request}; use query::{MarginPadding, MarginRetrievingFragmentBorderBoxIterator, PositionProperty}; use query::{PositionRetrievingFragmentBorderBoxIterator, Side}; -use script::dom::bindings::js::LayoutJS; -use script::dom::document::Document; -use script::dom::node::{LayoutData, Node}; +use script::dom::node::LayoutData; use script::layout_interface::Animation; use script::layout_interface::{LayoutChan, LayoutRPC, OffsetParentResponse}; use script::layout_interface::{Msg, NewLayoutTaskInfo, Reflow, ReflowGoal, ReflowQueryType}; @@ -90,7 +88,7 @@ use util::opts; use util::task::spawn_named_with_send_on_failure; use util::task_state; use util::workqueue::WorkQueue; -use wrapper::{LayoutDocument, LayoutNode, ThreadSafeLayoutNode}; +use wrapper::{LayoutNode, ThreadSafeLayoutNode}; /// The number of screens of data we're allowed to generate display lists for in each direction. pub const DISPLAY_PORT_SIZE_FACTOR: i32 = 8; @@ -895,17 +893,9 @@ impl LayoutTask { property: &Atom, layout_root: &mut FlowRef) -> Option { - // FIXME: Isolate this transmutation into a "bridge" module. - // FIXME(rust#16366): The following line had to be moved because of a - // rustc bug. It should be in the next unsafe block. - let node: LayoutJS = unsafe { - LayoutJS::from_trusted_node_address(requested_node) - }; - let node: &LayoutNode = unsafe { - transmute(&node) - }; + let node = unsafe { LayoutNode::new(&requested_node) }; - let layout_node = ThreadSafeLayoutNode::new(node); + let layout_node = ThreadSafeLayoutNode::new(&node); let layout_node = match pseudo { &Some(PseudoElement::Before) => layout_node.get_before_pseudo(), &Some(PseudoElement::After) => layout_node.get_after_pseudo(), @@ -1136,15 +1126,9 @@ impl LayoutTask { }; let _ajst = AutoJoinScriptTask { data: data }; - // FIXME: Isolate this transmutation into a "bridge" module. - let mut doc: LayoutJS = unsafe { - LayoutJS::from_trusted_node_address(data.document).downcast::().unwrap() - }; - let doc: &mut LayoutDocument = unsafe { - transmute(&mut doc) - }; - - let node: LayoutNode = match doc.root_node() { + let document = unsafe { LayoutNode::new(&data.document) }; + let document = document.as_document().unwrap(); + let node: LayoutNode = match document.root_node() { None => return, Some(x) => x, }; @@ -1199,7 +1183,7 @@ impl LayoutTask { } } - let event_state_changes = doc.drain_event_state_changes(); + let event_state_changes = document.drain_event_state_changes(); if !needs_dirtying { for &(el, state) in event_state_changes.iter() { assert!(!state.is_empty()); @@ -1233,7 +1217,7 @@ impl LayoutTask { }); // Retrieve the (possibly rebuilt) root flow. - rw_data.root_flow = self.try_get_layout_root(node.clone()); + rw_data.root_flow = self.try_get_layout_root(node); } // Send new canvas renderers to the paint task diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 72973633067..d10932530ae 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -221,6 +221,15 @@ impl<'ln> LayoutNode<'ln> { as_element(self.node) } + pub fn as_document(&self) -> Option> { + self.node.downcast().map(|document| { + LayoutDocument { + document: document, + chain: PhantomData, + } + }) + } + fn parent_node(&self) -> Option> { unsafe { self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))