From bf7c791e3aa28746fe1fcee61c34e30098676851 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sun, 5 Apr 2015 04:38:25 +0200 Subject: [PATCH] Hold a Temporary in AncestorIterator --- components/script/dom/document.rs | 22 +++---- components/script/dom/element.rs | 35 ++++++---- components/script/dom/eventdispatcher.rs | 3 +- components/script/dom/htmlbuttonelement.rs | 2 +- components/script/dom/htmlformelement.rs | 11 +++- components/script/dom/htmlinputelement.rs | 2 +- components/script/dom/htmlselectelement.rs | 2 +- components/script/dom/htmltextareaelement.rs | 2 +- components/script/dom/node.rs | 69 ++++++++++++-------- components/script/parse/html.rs | 6 +- 10 files changed, 91 insertions(+), 63 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 7e8a570960d..bd4c1c5393b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -506,20 +506,17 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { }.root(); let el = match ElementCast::to_ref(node.r()) { - Some(el) => el, + Some(el) => Temporary::from_rooted(el), None => { - let ancestor = node.r() - .ancestors() - .filter_map(ElementCast::to_ref) - .next(); - match ancestor { - Some(ancestor) => ancestor, + let parent = node.r().parent_node(); + match parent.and_then(ElementCast::to_temporary) { + Some(parent) => parent, None => return, } }, - }; + }.root(); - let node: JSRef = NodeCast::from_ref(el); + let node: JSRef = NodeCast::from_ref(el.r()); debug!("clicked on {:?}", node.debug_str()); // Prevent click event if form control element is disabled. if node.click_event_filter_by_disabled_state() { @@ -548,7 +545,7 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#trusted-events event.set_trusted(true); // https://html.spec.whatwg.org/multipage/interaction.html#run-authentic-click-activation-steps - el.authentic_click_activation(event); + el.r().authentic_click_activation(event); self.commit_focus_transaction(); window.r().reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::MouseEvent); @@ -563,7 +560,10 @@ impl<'a> DocumentHelpers<'a> for JSRef<'a, Document> { let mouse_over_targets: Vec> = mouse_over_addresses.iter() .filter_map(|node_address| { let node = node::from_untrusted_node_address(js_runtime, *node_address); - node.root().r().inclusive_ancestors().find(|node| node.is_element()).map(JS::from_rooted) + node.root().r().inclusive_ancestors() + .map(|node| node.root()) + .find(|node| node.r().is_element()) + .map(|node| JS::from_rooted(node.r())) }).collect(); // Remove hover from any elements in the previous list that are no longer diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 4cfa77fdb4b..c3bf678a48d 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -28,7 +28,7 @@ use dom::bindings::error::Error; use dom::bindings::error::Error::{InvalidCharacter, Syntax}; use dom::bindings::error::Error::NoModificationAllowed; use dom::bindings::js::{MutNullableJS, JS, JSRef, LayoutJS, Temporary, TemporaryPushable}; -use dom::bindings::js::OptionalRootable; +use dom::bindings::js::{OptionalRootable, RootedReference}; use dom::bindings::trace::RootedVec; use dom::bindings::utils::xml_name_type; use dom::bindings::utils::XMLName::{QName, Name, InvalidXMLName}; @@ -595,8 +595,8 @@ impl<'a> ElementHelpers<'a> for JSRef<'a, Element> { // https://html.spec.whatwg.org/multipage/infrastructure.html#root-element fn get_root_element(self) -> Option> { let node: JSRef = NodeCast::from_ref(self); - match node.ancestors().last().map(ElementCast::to_ref) { - Some(n) => n.map(Temporary::from_rooted), + match node.ancestors().last().map(ElementCast::to_temporary) { + Some(n) => n, None => Some(self).map(Temporary::from_rooted), } } @@ -1258,10 +1258,15 @@ impl<'a> ElementMethods for JSRef<'a, Element> { Err(()) => Err(Syntax), Ok(ref selectors) => { let root: JSRef = NodeCast::from_ref(self); - Ok(root.inclusive_ancestors() - .filter_map(ElementCast::to_ref) - .find(|element| matches(selectors, &NodeCast::from_ref(*element), &mut None)) - .map(Temporary::from_rooted)) + for element in root.inclusive_ancestors() { + let element = element.root(); + if let Some(element) = ElementCast::to_ref(element.r()) { + if matches(selectors, &NodeCast::from_ref(element), &mut None) { + return Ok(Some(Temporary::from_rooted(element))); + } + } + } + Ok(None) } } } @@ -1599,13 +1604,15 @@ impl<'a> ActivationElementHelpers<'a> for JSRef<'a, Element> { Some(el) => Some(Temporary::from_rooted(el.as_element().root().r())), None => { let node: JSRef = NodeCast::from_ref(self); - node.ancestors() - .filter_map(|node| { - let e: Option> = ElementCast::to_ref(node); - e - }) - .filter(|e| e.as_maybe_activatable().is_some()).next() - .map(|r| Temporary::from_rooted(r)) + for node in node.ancestors() { + let node = node.root(); + if let Some(node) = ElementCast::to_ref(node.r()) { + if node.as_maybe_activatable().is_some() { + return Some(Temporary::from_rooted(node)) + } + } + } + None } } } diff --git a/components/script/dom/eventdispatcher.rs b/components/script/dom/eventdispatcher.rs index 029691f702c..6882c3f4599 100644 --- a/components/script/dom/eventdispatcher.rs +++ b/components/script/dom/eventdispatcher.rs @@ -31,7 +31,8 @@ pub fn dispatch_event<'a, 'b>(target: JSRef<'a, EventTarget>, let mut chain: RootedVec> = RootedVec::new(); if let Some(target_node) = NodeCast::to_ref(target) { for ancestor in target_node.ancestors() { - let ancestor_target: JSRef = EventTargetCast::from_ref(ancestor); + let ancestor = ancestor.root(); + let ancestor_target = EventTargetCast::from_ref(ancestor.r()); chain.push(JS::from_rooted(ancestor_target)) } } diff --git a/components/script/dom/htmlbuttonelement.rs b/components/script/dom/htmlbuttonelement.rs index e1d74d7a3a6..aaf725d1d9e 100644 --- a/components/script/dom/htmlbuttonelement.rs +++ b/components/script/dom/htmlbuttonelement.rs @@ -161,7 +161,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLButtonElement> { } let node: JSRef = NodeCast::from_ref(*self); - if node.ancestors().any(|ancestor| ancestor.is_htmlfieldsetelement()) { + if node.ancestors().any(|ancestor| ancestor.root().r().is_htmlfieldsetelement()) { node.check_ancestors_disabled_state_for_form_control(); } else { node.check_disabled_attribute(); diff --git a/components/script/dom/htmlformelement.rs b/components/script/dom/htmlformelement.rs index ebe033cea37..395a9ab8522 100644 --- a/components/script/dom/htmlformelement.rs +++ b/components/script/dom/htmlformelement.rs @@ -258,7 +258,7 @@ impl<'a> HTMLFormElementHelpers for JSRef<'a, HTMLFormElement> { if child.get_disabled_state() { return None; } - if child.ancestors().any(|a| a.type_id() == NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLDataListElement))) { + if child.ancestors().any(|a| a.root().r().type_id() == NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLDataListElement))) { return None; } // XXXManishearth don't include it if it is a button but not the submitter @@ -531,8 +531,13 @@ pub trait FormControl<'a> : Copy + Sized { } } let node: JSRef = NodeCast::from_ref(elem); - node.ancestors().filter_map(|a| HTMLFormElementCast::to_ref(a)).next() - .map(Temporary::from_rooted) + for ancestor in node.ancestors() { + let ancestor = ancestor.root(); + if let Some(ancestor) = HTMLFormElementCast::to_ref(ancestor.r()) { + return Some(Temporary::from_rooted(ancestor)) + } + } + None } fn get_form_attribute(self, diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 9223ac7ad60..cb3eff8c59b 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -583,7 +583,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLInputElement> { } let node: JSRef = NodeCast::from_ref(*self); - if node.ancestors().any(|ancestor| ancestor.is_htmlfieldsetelement()) { + if node.ancestors().any(|ancestor| ancestor.root().r().is_htmlfieldsetelement()) { node.check_ancestors_disabled_state_for_form_control(); } else { node.check_disabled_attribute(); diff --git a/components/script/dom/htmlselectelement.rs b/components/script/dom/htmlselectelement.rs index 14459ca841b..12f88281eba 100644 --- a/components/script/dom/htmlselectelement.rs +++ b/components/script/dom/htmlselectelement.rs @@ -129,7 +129,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLSelectElement> { } let node: JSRef = NodeCast::from_ref(*self); - if node.ancestors().any(|ancestor| ancestor.is_htmlfieldsetelement()) { + if node.ancestors().any(|ancestor| ancestor.root().r().is_htmlfieldsetelement()) { node.check_ancestors_disabled_state_for_form_control(); } else { node.check_disabled_attribute(); diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 2b53e09a1a2..f45bedd791d 100644 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -319,7 +319,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLTextAreaElement> { } let node: JSRef = NodeCast::from_ref(*self); - if node.ancestors().any(|ancestor| ancestor.is_htmlfieldsetelement()) { + if node.ancestors().any(|ancestor| ancestor.root().r().is_htmlfieldsetelement()) { node.check_ancestors_disabled_state_for_form_control(); } else { node.check_disabled_attribute(); diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index e4dcdae5ee4..98656887bef 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -409,8 +409,8 @@ impl<'a> Iterator for QuerySelectorIterator<'a> { } pub trait NodeHelpers<'a> { - fn ancestors(self) -> AncestorIterator<'a>; - fn inclusive_ancestors(self) -> AncestorIterator<'a>; + fn ancestors(self) -> AncestorIterator; + fn inclusive_ancestors(self) -> AncestorIterator; fn children(self) -> NodeChildrenIterator; fn rev_children(self) -> ReverseChildrenIterator; fn child_elements(self) -> ChildElementIterator<'a>; @@ -735,8 +735,9 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { // 4. Dirty ancestors. for ancestor in self.ancestors() { - if !force_ancestors && ancestor.get_has_dirty_descendants() { break } - ancestor.set_has_dirty_descendants(true); + let ancestor = ancestor.root(); + if !force_ancestors && ancestor.r().get_has_dirty_descendants() { break } + ancestor.r().set_has_dirty_descendants(true); } } @@ -752,7 +753,7 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { } fn is_inclusive_ancestor_of(self, parent: JSRef) -> bool { - self == parent || parent.ancestors().any(|ancestor| ancestor == self) + self == parent || parent.ancestors().any(|ancestor| ancestor.root().r() == self) } fn following_siblings(self) -> NodeChildrenIterator { @@ -788,7 +789,8 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { Err(()) => return Err(Syntax), // Step 3. Ok(ref selectors) => { - let root = self.ancestors().last().unwrap_or(self.clone()); + let root = self.ancestors().last().root(); + let root = root.r().unwrap_or(self.clone()); Ok(root.traverse_preorder() .filter_map(ElementCast::to_ref) .find(|element| matches(selectors, &NodeCast::from_ref(*element), &mut None)) @@ -805,7 +807,9 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { -> Fallible> { // Step 1. let nodes; - let root = self.ancestors().last().unwrap_or(self.clone()); + let root = self.ancestors().last().root() + .map(|node| node.get_unsound_ref_forever()) + .unwrap_or(self.clone()); match parse_author_origin_selector_list_from_str(selectors.as_slice()) { // Step 2. Err(()) => return Err(Syntax), @@ -830,15 +834,15 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> { } - fn ancestors(self) -> AncestorIterator<'a> { + fn ancestors(self) -> AncestorIterator { AncestorIterator { - current: self.parent_node.get().map(|node| node.root().get_unsound_ref_forever()), + current: self.parent_node() } } - fn inclusive_ancestors(self) -> AncestorIterator<'a> { + fn inclusive_ancestors(self) -> AncestorIterator { AncestorIterator { - current: Some(self.clone()) + current: Some(Temporary::from_rooted(self)) } } @@ -1148,17 +1152,20 @@ impl Iterator for ReverseChildrenIterator { } } -pub struct AncestorIterator<'a> { - current: Option>, +pub struct AncestorIterator { + current: Option>, } -impl<'a> Iterator for AncestorIterator<'a> { - type Item = JSRef<'a, Node>; +impl Iterator for AncestorIterator { + type Item = Temporary; - fn next(&mut self) -> Option> { - let node = self.current; - self.current = node.and_then(|node| node.parent_node().map(|node| node.root().get_unsound_ref_forever())); - node + fn next(&mut self) -> Option> { + let current = match self.current.take() { + None => return None, + Some(current) => current, + }.root(); + self.current = current.r().parent_node(); + Some(Temporary::from_rooted(current.r())) } } @@ -2174,23 +2181,25 @@ impl<'a> NodeMethods for JSRef<'a, Node> { // step 2. 0 } else { - let mut lastself = self.clone(); - let mut lastother = other.clone(); + let mut lastself = Temporary::from_rooted(self.clone()); + let mut lastother = Temporary::from_rooted(other.clone()); for ancestor in self.ancestors() { - if ancestor == other { + let ancestor = ancestor.root(); + if ancestor.r() == other { // step 4. return NodeConstants::DOCUMENT_POSITION_CONTAINS + NodeConstants::DOCUMENT_POSITION_PRECEDING; } - lastself = ancestor.clone(); + lastself = Temporary::from_rooted(ancestor.r()); } for ancestor in other.ancestors() { - if ancestor == self { + let ancestor = ancestor.root(); + if ancestor.r() == self { // step 5. return NodeConstants::DOCUMENT_POSITION_CONTAINED_BY + NodeConstants::DOCUMENT_POSITION_FOLLOWING; } - lastother = ancestor.clone(); + lastother = Temporary::from_rooted(ancestor.r()); } if lastself != lastother { @@ -2208,7 +2217,8 @@ impl<'a> NodeMethods for JSRef<'a, Node> { NodeConstants::DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC; } - for child in lastself.traverse_preorder() { + let lastself = lastself.root(); + for child in lastself.r().traverse_preorder() { if child == other { // step 6. return NodeConstants::DOCUMENT_POSITION_PRECEDING; @@ -2418,7 +2428,10 @@ pub trait DisabledStateHelpers { impl<'a> DisabledStateHelpers for JSRef<'a, Node> { fn check_ancestors_disabled_state_for_form_control(self) { if self.get_disabled_state() { return; } - for ancestor in self.ancestors().filter(|ancestor| ancestor.is_htmlfieldsetelement()) { + for ancestor in self.ancestors() { + let ancestor = ancestor.root(); + let ancestor = ancestor.r(); + if !ancestor.is_htmlfieldsetelement() { continue; } if !ancestor.get_disabled_state() { continue; } if ancestor.is_parent_of(self) { self.set_disabled_state(true); @@ -2431,7 +2444,7 @@ impl<'a> DisabledStateHelpers for JSRef<'a, Node> { { Some(legend) => { // XXXabinader: should we save previous ancestor to avoid this iteration? - if self.ancestors().any(|ancestor| ancestor == legend.r()) { continue; } + if self.ancestors().any(|ancestor| ancestor.root().r() == legend.r()) { continue; } }, None => () } diff --git a/components/script/parse/html.rs b/components/script/parse/html.rs index 2aecd9264e9..66a95f2e879 100644 --- a/components/script/parse/html.rs +++ b/components/script/parse/html.rs @@ -12,6 +12,7 @@ use dom::bindings::codegen::InheritTypes::{DocumentTypeCast, TextCast, CommentCa use dom::bindings::codegen::InheritTypes::ProcessingInstructionCast; use dom::bindings::codegen::InheritTypes::HTMLFormElementDerived; use dom::bindings::js::{JS, JSRef, Temporary, OptionalRootable, Root}; +use dom::bindings::js::RootedReference; use dom::bindings::trace::RootedVec; use dom::comment::Comment; use dom::document::{Document, DocumentHelpers}; @@ -350,10 +351,11 @@ pub fn parse_html_fragment(context_node: JSRef, // Step 11. let form = context_node.inclusive_ancestors() - .find(|element| element.is_htmlformelement()); + .map(|element| element.root()) + .find(|element| element.r().is_htmlformelement()); let fragment_context = FragmentContext { context_elem: context_node, - form_elem: form, + form_elem: form.r(), }; parse_html(document.r(), HTMLInput::InputString(input), &url, Some(fragment_context));