From 6c7f37061b5c356c4d045bfd23b33e84220e39f9 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 01:58:52 +0200 Subject: [PATCH 01/21] Implement Deref for JS where T: Reflectable We can only borrow JS from rooted things, so it's safe to deref it. The only types that provide mutable JS things are MutHeap> and MutNullableHeap>, which don't actually expose that they contain JS values. --- components/script/dom/bindings/js.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index aea80d88431..1ef1d93a86d 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -96,6 +96,16 @@ impl JS { } } +impl Deref for JS { + type Target = T; + + fn deref(&self) -> &T { + // We can only have &JS from a rooted thing, so it's safe to deref + // it to &T. + unsafe { &**self.ptr } + } +} + impl JSTraceable for JS { fn trace(&self, trc: *mut JSTracer) { trace_reflector(trc, "", unsafe { (**self.ptr).reflector() }); From 1f31d5b856ba33a32336f0c24e658f0bac21eede Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 02:03:55 +0200 Subject: [PATCH 02/21] Return a reference in BrowserContext::active_document() --- components/script/dom/browsercontext.rs | 7 +++---- components/script/dom/document.rs | 2 +- components/script/dom/window.rs | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs index af2bfbdc81e..87d90f8e64f 100644 --- a/components/script/dom/browsercontext.rs +++ b/components/script/dom/browsercontext.rs @@ -44,13 +44,12 @@ impl BrowsingContext { } } - pub fn active_document(&self) -> Root { - self.history[self.active_index].document.root() + pub fn active_document(&self) -> &Document { + &*self.history[self.active_index].document } pub fn active_window(&self) -> Root { - let doc = self.active_document(); - doc.r().window() + self.active_document().window() } pub fn frame_element(&self) -> Option> { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 175cc68ea9d..e0a8154de92 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -254,7 +254,7 @@ impl Document { let browsing_context = browsing_context.as_ref().unwrap(); let active_document = browsing_context.active_document(); - if self != active_document.r() { + if self != active_document { return false; } // FIXME: It should also check whether the browser context is top-level or not diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 4ada66c36cc..b5c794dcac2 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -381,7 +381,7 @@ impl WindowMethods for Window { // https://html.spec.whatwg.org/multipage/#dom-document-0 fn Document(&self) -> Root { - self.browsing_context().as_ref().unwrap().active_document() + Root::from_ref(self.browsing_context().as_ref().unwrap().active_document()) } // https://html.spec.whatwg.org/multipage/#dom-location From 264e94359712d143168ec47b3403c9285efb3354 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 02:17:25 +0200 Subject: [PATCH 03/21] Return a reference in BrowserContext::frame_element() --- components/script/dom/browsercontext.rs | 4 ++-- components/script/dom/document.rs | 8 ++++---- components/script/dom/window.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs index 87d90f8e64f..55e070ef10f 100644 --- a/components/script/dom/browsercontext.rs +++ b/components/script/dom/browsercontext.rs @@ -52,8 +52,8 @@ impl BrowsingContext { self.active_document().window() } - pub fn frame_element(&self) -> Option> { - self.frame_element.as_ref().map(JS::root) + pub fn frame_element(&self) -> Option<&Element> { + self.frame_element.as_ref().map(|element| &**element) } pub fn window_proxy(&self) -> *mut JSObject { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index e0a8154de92..52ad4dc20be 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -1923,14 +1923,14 @@ impl DocumentProgressHandler { let browsing_context = window_ref.browsing_context(); let browsing_context = browsing_context.as_ref().unwrap(); - browsing_context.frame_element().map(|frame_element| { - let frame_window = window_from_node(frame_element.r()); + if let Some(frame_element) = browsing_context.frame_element() { + let frame_window = window_from_node(frame_element); let event = Event::new(GlobalRef::Window(frame_window.r()), "load".to_owned(), EventBubbles::DoesNotBubble, EventCancelable::NotCancelable); - let target = EventTargetCast::from_ref(frame_element.r()); + let target = EventTargetCast::from_ref(frame_element); event.r().fire(target); - }); + }; document.r().notify_constellation_load(); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index b5c794dcac2..3b56b96ddb1 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -411,7 +411,7 @@ impl WindowMethods for Window { // https://html.spec.whatwg.org/multipage/#dom-frameelement fn GetFrameElement(&self) -> Option> { - self.browsing_context().as_ref().unwrap().frame_element() + self.browsing_context().as_ref().unwrap().frame_element().map(Root::from_ref) } // https://html.spec.whatwg.org/multipage/#dom-navigator @@ -1245,7 +1245,7 @@ impl Window { let browsing_context = browsing_context.as_ref().unwrap(); browsing_context.frame_element().map(|frame_element| { - let window = window_from_node(frame_element.r()); + let window = window_from_node(frame_element); // FIXME(https://github.com/rust-lang/rust/issues/23338) let r = window.r(); let context = r.browsing_context(); From e889b0914b88bbbe903bc390c444fb13f4c0dce5 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Mon, 19 Oct 2015 09:36:19 +0200 Subject: [PATCH 04/21] Do not root Document::window --- components/script/dom/document.rs | 177 +++++++++++------------------- 1 file changed, 66 insertions(+), 111 deletions(-) diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 52ad4dc20be..5aa4f257d37 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -248,9 +248,7 @@ impl Document { // https://html.spec.whatwg.org/multipage/#fully-active pub fn is_fully_active(&self) -> bool { - let window = self.window.root(); - let window = window.r(); - let browsing_context = window.browsing_context(); + let browsing_context = self.window.browsing_context(); let browsing_context = browsing_context.as_ref().unwrap(); let active_document = browsing_context.active_document(); @@ -308,9 +306,7 @@ impl Document { self.quirks_mode.set(mode); if mode == Quirks { - let window = self.window.root(); - let window = window.r(); - let LayoutChan(ref layout_chan) = window.layout_chan(); + let LayoutChan(ref layout_chan) = self.window.layout_chan(); layout_chan.send(Msg::SetQuirksMode).unwrap(); } } @@ -336,10 +332,9 @@ impl Document { } self.reflow_timeout.set(None); - let window = self.window.root(); - window.r().reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::RefreshTick); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::RefreshTick); } } @@ -449,8 +444,7 @@ impl Document { None => return None, }; let root = NodeCast::from_ref(root); - let win = self.window.root(); - let address = match win.r().layout().hit_test(root.to_trusted_node_address(), *point) { + let address = match self.window.layout().hit_test(root.to_trusted_node_address(), *point) { Ok(HitTestResponse(node_address)) => Some(node_address), Err(()) => { debug!("layout query error"); @@ -467,8 +461,7 @@ impl Document { None => return vec!(), }; let root = NodeCast::from_ref(root); - let win = self.window.root(); - match win.r().layout().mouse_over(root.to_trusted_node_address(), *point) { + match self.window.layout().mouse_over(root.to_trusted_node_address(), *point) { Ok(MouseOverResponse(node_address)) => node_address, Err(()) => vec!(), } @@ -478,8 +471,7 @@ impl Document { pub fn set_ready_state(&self, state: DocumentReadyState) { self.ready_state.set(state); - let window = self.window.root(); - let event = Event::new(GlobalRef::Window(window.r()), "readystatechange".to_owned(), + let event = Event::new(GlobalRef::Window(&self.window), "readystatechange".to_owned(), EventBubbles::DoesNotBubble, EventCancelable::NotCancelable); let target = EventTargetCast::from_ref(self); @@ -529,9 +521,8 @@ impl Document { // Update the focus state for all elements in the focus chain. // https://html.spec.whatwg.org/multipage/#focus-chain if focus_type == FocusType::Element { - let window = self.window.root(); - let ConstellationChan(ref chan) = window.r().constellation_chan(); - let event = ConstellationMsg::Focus(window.r().pipeline()); + let ConstellationChan(ref chan) = self.window.constellation_chan(); + let event = ConstellationMsg::Focus(self.window.pipeline()); chan.send(event).unwrap(); } } @@ -600,17 +591,15 @@ impl Document { self.begin_focus_transaction(); } - let window = self.window.root(); - // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#event-type-click let x = point.x as i32; let y = point.y as i32; let clickCount = 1; - let event = MouseEvent::new(window.r(), + let event = MouseEvent::new(&self.window, mouse_event_type_string, EventBubbles::Bubbles, EventCancelable::Cancelable, - Some(window.r()), + Some(&self.window), clickCount, x, y, x, y, false, false, false, false, @@ -632,7 +621,9 @@ impl Document { if let MouseEventType::Click = mouse_event_type { self.commit_focus_transaction(FocusType::Element); } - window.r().reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::MouseEvent); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::MouseEvent); } pub fn fire_mouse_event(&self, @@ -642,13 +633,11 @@ impl Document { let x = point.x.to_i32().unwrap_or(0); let y = point.y.to_i32().unwrap_or(0); - let window = self.window.root(); - - let mouse_event = MouseEvent::new(window.r(), + let mouse_event = MouseEvent::new(&self.window, event_name, EventBubbles::Bubbles, EventCancelable::Cancelable, - Some(window.r()), + Some(&self.window), 0i32, x, y, x, y, false, false, false, false, @@ -715,10 +704,9 @@ impl Document { prev_mouse_over_targets.clear(); prev_mouse_over_targets.append(&mut *mouse_over_targets); - let window = self.window.root(); - window.r().reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::MouseEvent); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::MouseEvent); } /// The entry point for all key processing for web content @@ -727,14 +715,13 @@ impl Document { state: KeyState, modifiers: KeyModifiers, compositor: &mut IpcSender) { - let window = self.window.root(); let focused = self.get_focused_element(); let body = self.GetBody(); let target = match (&focused, &body) { (&Some(ref focused), _) => EventTargetCast::from_ref(focused.r()), (&None, &Some(ref body)) => EventTargetCast::from_ref(body.r()), - (&None, &None) => EventTargetCast::from_ref(window.r()), + (&None, &None) => EventTargetCast::from_ref(&*self.window), }; let ctrl = modifiers.contains(CONTROL); @@ -751,8 +738,8 @@ impl Document { let props = KeyboardEvent::key_properties(key, modifiers); - let keyevent = KeyboardEvent::new(window.r(), ev_type, true, true, - Some(window.r()), 0, Some(key), + let keyevent = KeyboardEvent::new(&self.window, ev_type, true, true, + Some(&self.window), 0, Some(key), props.key_string.to_owned(), props.code.to_owned(), props.location, is_repeating, is_composing, ctrl, alt, shift, meta, @@ -764,8 +751,8 @@ impl Document { // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#keys-cancelable-keys if state != KeyState::Released && props.is_printable() && !prevented { // https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#keypress-event-order - let event = KeyboardEvent::new(window.r(), "keypress".to_owned(), - true, true, Some(window.r()), 0, Some(key), + let event = KeyboardEvent::new(&self.window, "keypress".to_owned(), + true, true, Some(&self.window), 0, Some(key), props.key_string.to_owned(), props.code.to_owned(), props.location, is_repeating, is_composing, ctrl, alt, shift, meta, @@ -805,7 +792,9 @@ impl Document { _ => () } - window.r().reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::KeyEvent); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::KeyEvent); } pub fn node_from_nodes_and_strings(&self, nodes: Vec) @@ -857,10 +846,8 @@ impl Document { pub fn trigger_mozbrowser_event(&self, event: MozBrowserEvent) { if htmliframeelement::mozbrowser_enabled() { - let window = self.window.root(); - - if let Some((containing_pipeline_id, subpage_id)) = window.r().parent_info() { - let ConstellationChan(ref chan) = window.r().constellation_chan(); + if let Some((containing_pipeline_id, subpage_id)) = self.window.parent_info() { + let ConstellationChan(ref chan) = self.window.constellation_chan(); let event = ConstellationMsg::MozBrowserEvent(containing_pipeline_id, subpage_id, event); @@ -871,16 +858,14 @@ impl Document { /// https://html.spec.whatwg.org/multipage/#dom-window-requestanimationframe pub fn request_animation_frame(&self, callback: Box) -> u32 { - let window = self.window.root(); - let window = window.r(); let ident = self.animation_frame_ident.get() + 1; self.animation_frame_ident.set(ident); self.animation_frame_list.borrow_mut().insert(ident, callback); // TODO: Should tick animation only when document is visible - let ConstellationChan(ref chan) = window.constellation_chan(); - let event = ConstellationMsg::ChangeRunningAnimationsState(window.pipeline(), + let ConstellationChan(ref chan) = self.window.constellation_chan(); + let event = ConstellationMsg::ChangeRunningAnimationsState(self.window.pipeline(), AnimationState::AnimationCallbacksPresent); chan.send(event).unwrap(); @@ -891,10 +876,8 @@ impl Document { pub fn cancel_animation_frame(&self, ident: u32) { self.animation_frame_list.borrow_mut().remove(&ident); if self.animation_frame_list.borrow().is_empty() { - let window = self.window.root(); - let window = window.r(); - let ConstellationChan(ref chan) = window.constellation_chan(); - let event = ConstellationMsg::ChangeRunningAnimationsState(window.pipeline(), + let ConstellationChan(ref chan) = self.window.constellation_chan(); + let event = ConstellationMsg::ChangeRunningAnimationsState(self.window.pipeline(), AnimationState::NoAnimationCallbacksPresent); chan.send(event).unwrap(); } @@ -907,16 +890,12 @@ impl Document { let mut list = self.animation_frame_list.borrow_mut(); animation_frame_list = Vec::from_iter(list.drain()); - let window = self.window.root(); - let window = window.r(); - let ConstellationChan(ref chan) = window.constellation_chan(); - let event = ConstellationMsg::ChangeRunningAnimationsState(window.pipeline(), + let ConstellationChan(ref chan) = self.window.constellation_chan(); + let event = ConstellationMsg::ChangeRunningAnimationsState(self.window.pipeline(), AnimationState::NoAnimationCallbacksPresent); chan.send(event).unwrap(); } - let window = self.window.root(); - let window = window.r(); - let performance = window.Performance(); + let performance = self.window.Performance(); let performance = performance.r(); let timing = performance.Now(); @@ -924,9 +903,9 @@ impl Document { callback(*timing); } - window.reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::RequestAnimationFrame); + self.window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::RequestAnimationFrame); } pub fn prepare_async_load(&self, load: LoadType) -> PendingAsyncLoad { @@ -950,9 +929,8 @@ impl Document { } pub fn notify_constellation_load(&self) { - let window = self.window.root(); - let pipeline_id = window.r().pipeline(); - let ConstellationChan(ref chan) = window.r().constellation_chan(); + let pipeline_id = self.window.pipeline(); + let ConstellationChan(ref chan) = self.window.constellation_chan(); let event = ConstellationMsg::DOMLoad(pipeline_id); chan.send(event).unwrap(); @@ -1093,12 +1071,11 @@ impl Document { } fn create_node_list bool>(&self, callback: F) -> Root { - let window = self.window.root(); let doc = self.GetDocumentElement(); let maybe_node = doc.r().map(NodeCast::from_ref); let iter = maybe_node.iter().flat_map(|node| node.traverse_preorder()) .filter(|node| callback(node.r())); - NodeList::new_simple_list(window.r(), iter) + NodeList::new_simple_list(&self.window, iter) } fn get_html_element(&self) -> Option> { @@ -1168,9 +1145,7 @@ impl DocumentMethods for Document { // https://html.spec.whatwg.org/multipage/#dom-document-hasfocus fn HasFocus(&self) -> bool { let target = self; // Step 1. - let window = self.window.root(); - let window = window.r(); - let browsing_context = window.browsing_context(); + let browsing_context = self.window.browsing_context(); let browsing_context = browsing_context.as_ref(); match browsing_context { @@ -1230,22 +1205,18 @@ impl DocumentMethods for Document { // https://dom.spec.whatwg.org/#dom-document-getelementsbytagname fn GetElementsByTagName(&self, tag_name: DOMString) -> Root { - let window = self.window.root(); - HTMLCollection::by_tag_name(window.r(), NodeCast::from_ref(self), tag_name) + HTMLCollection::by_tag_name(&self.window, NodeCast::from_ref(self), tag_name) } // https://dom.spec.whatwg.org/#dom-document-getelementsbytagnamens fn GetElementsByTagNameNS(&self, maybe_ns: Option, tag_name: DOMString) -> Root { - let window = self.window.root(); - HTMLCollection::by_tag_name_ns(window.r(), NodeCast::from_ref(self), tag_name, maybe_ns) + HTMLCollection::by_tag_name_ns(&self.window, NodeCast::from_ref(self), tag_name, maybe_ns) } // https://dom.spec.whatwg.org/#dom-document-getelementsbyclassname fn GetElementsByClassName(&self, classes: DOMString) -> Root { - let window = self.window.root(); - - HTMLCollection::by_class_name(window.r(), NodeCast::from_ref(self), classes) + HTMLCollection::by_class_name(&self.window, NodeCast::from_ref(self), classes) } // https://dom.spec.whatwg.org/#dom-nonelementparentnode-getelementbyid @@ -1284,13 +1255,12 @@ impl DocumentMethods for Document { return Err(Error::InvalidCharacter); } - let window = self.window.root(); let name = Atom::from_slice(&local_name); // repetition used because string_cache::atom::Atom is non-copyable let l_name = Atom::from_slice(&local_name); let value = AttrValue::String("".to_owned()); - Ok(Attr::new(window.r(), name, value, l_name, ns!(""), None, None)) + Ok(Attr::new(&self.window, name, value, l_name, ns!(""), None, None)) } // https://dom.spec.whatwg.org/#dom-document-createattributens @@ -1298,10 +1268,9 @@ impl DocumentMethods for Document { -> Fallible> { let (namespace, prefix, local_name) = try!(validate_and_extract(namespace, &qualified_name)); - let window = self.window.root(); let value = AttrValue::String("".to_owned()); let qualified_name = Atom::from_slice(&qualified_name); - Ok(Attr::new(window.r(), local_name, value, qualified_name, + Ok(Attr::new(&self.window, local_name, value, qualified_name, namespace, prefix, None)) } @@ -1369,22 +1338,20 @@ impl DocumentMethods for Document { // https://dom.spec.whatwg.org/#dom-document-createevent fn CreateEvent(&self, mut interface: DOMString) -> Fallible> { - let window = self.window.root(); - interface.make_ascii_lowercase(); match &*interface { "uievents" | "uievent" => Ok(EventCast::from_root( - UIEvent::new_uninitialized(window.r()))), + UIEvent::new_uninitialized(&self.window))), "mouseevents" | "mouseevent" => Ok(EventCast::from_root( - MouseEvent::new_uninitialized(window.r()))), + MouseEvent::new_uninitialized(&self.window))), "customevent" => Ok(EventCast::from_root( - CustomEvent::new_uninitialized(GlobalRef::Window(window.r())))), + CustomEvent::new_uninitialized(GlobalRef::Window(&self.window)))), "htmlevents" | "events" | "event" => Ok(Event::new_uninitialized( - GlobalRef::Window(window.r()))), + GlobalRef::Window(&self.window))), "keyboardevent" | "keyevents" => Ok(EventCast::from_root( - KeyboardEvent::new_uninitialized(window.r()))), + KeyboardEvent::new_uninitialized(&self.window))), "messageevent" => Ok(EventCast::from_root( - MessageEvent::new_uninitialized(GlobalRef::Window(window.r())))), + MessageEvent::new_uninitialized(GlobalRef::Window(&self.window)))), _ => Err(Error::NotSupported) } } @@ -1585,20 +1552,18 @@ impl DocumentMethods for Document { // https://html.spec.whatwg.org/multipage/#dom-document-images fn Images(&self) -> Root { self.images.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box ImagesFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } // https://html.spec.whatwg.org/multipage/#dom-document-embeds fn Embeds(&self) -> Root { self.embeds.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box EmbedsFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } @@ -1610,40 +1575,36 @@ impl DocumentMethods for Document { // https://html.spec.whatwg.org/multipage/#dom-document-links fn Links(&self) -> Root { self.links.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box LinksFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } // https://html.spec.whatwg.org/multipage/#dom-document-forms fn Forms(&self) -> Root { self.forms.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box FormsFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } // https://html.spec.whatwg.org/multipage/#dom-document-scripts fn Scripts(&self) -> Root { self.scripts.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box ScriptsFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } // https://html.spec.whatwg.org/multipage/#dom-document-anchors fn Anchors(&self) -> Root { self.anchors.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box AnchorsFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } @@ -1651,24 +1612,20 @@ impl DocumentMethods for Document { fn Applets(&self) -> Root { // FIXME: This should be return OBJECT elements containing applets. self.applets.or_init(|| { - let window = self.window.root(); let root = NodeCast::from_ref(self); let filter = box AppletsFilter; - HTMLCollection::create(window.r(), root, filter) + HTMLCollection::create(&self.window, root, filter) }) } // https://html.spec.whatwg.org/multipage/#dom-document-location fn Location(&self) -> Root { - let window = self.window.root(); - let window = window.r(); - self.location.or_init(|| Location::new(window)) + self.location.or_init(|| Location::new(&self.window)) } // https://dom.spec.whatwg.org/#dom-parentnode-children fn Children(&self) -> Root { - let window = self.window.root(); - HTMLCollection::children(window.r(), NodeCast::from_ref(self)) + HTMLCollection::children(&self.window, NodeCast::from_ref(self)) } // https://dom.spec.whatwg.org/#dom-parentnode-firstelementchild @@ -1725,9 +1682,8 @@ impl DocumentMethods for Document { if !is_scheme_host_port_tuple(&url) { return Err(Error::Security); } - let window = self.window.root(); let (tx, rx) = ipc::channel().unwrap(); - let _ = window.r().resource_task().send(GetCookiesForUrl((*url).clone(), tx, NonHTTP)); + let _ = self.window.resource_task().send(GetCookiesForUrl((*url).clone(), tx, NonHTTP)); let cookies = rx.recv().unwrap(); Ok(cookies.unwrap_or("".to_owned())) } @@ -1739,8 +1695,7 @@ impl DocumentMethods for Document { if !is_scheme_host_port_tuple(url) { return Err(Error::Security); } - let window = self.window.root(); - let _ = window.r().resource_task().send(SetCookiesForUrl((*url).clone(), cookie, NonHTTP)); + let _ = self.window.resource_task().send(SetCookiesForUrl((*url).clone(), cookie, NonHTTP)); Ok(()) } From 409b5e3695828e01fc25cbab96148c4932f07cad Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 02:23:25 +0200 Subject: [PATCH 05/21] Return a reference in Document::window() --- components/script/dom/browsercontext.rs | 2 +- components/script/dom/document.rs | 32 +++++++++----------- components/script/dom/domimplementation.rs | 6 ++-- components/script/dom/element.rs | 2 +- components/script/dom/htmlimageelement.rs | 1 - components/script/dom/htmlinputelement.rs | 2 +- components/script/dom/htmltextareaelement.rs | 2 +- components/script/dom/node.rs | 8 ++--- components/script/dom/nodeiterator.rs | 3 +- components/script/dom/range.rs | 3 +- components/script/dom/servohtmlparser.rs | 6 ++-- components/script/dom/treewalker.rs | 3 +- 12 files changed, 31 insertions(+), 39 deletions(-) diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs index 55e070ef10f..2b00f346c1e 100644 --- a/components/script/dom/browsercontext.rs +++ b/components/script/dom/browsercontext.rs @@ -49,7 +49,7 @@ impl BrowsingContext { } pub fn active_window(&self) -> Root { - self.active_document().window() + Root::from_ref(self.active_document().window()) } pub fn frame_element(&self) -> Option<&Element> { diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 5aa4f257d37..20d04269ada 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -232,8 +232,8 @@ impl Document { } #[inline] - pub fn window(&self) -> Root { - self.window.root() + pub fn window(&self) -> &Window { + &*self.window } #[inline] @@ -539,8 +539,6 @@ impl Document { /// Sends this document's title to the compositor. pub fn send_title_to_compositor(&self) { let window = self.window(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let window = window.r(); let compositor = window.compositor(); compositor.send(ScriptToCompositorMsg::SetTitle(window.pipeline(), Some(self.Title()))).unwrap(); } @@ -1094,7 +1092,7 @@ impl Document { IsHTMLDocument::NonHTMLDocument }; let new_doc = Document::new( - &*self.window(), None, doctype, None, None, + self.window(), None, doctype, None, None, DocumentSource::NotFromParser, DocumentLoader::new(&self.loader())); new_doc.appropriate_template_contents_owner_document.set(Some(&new_doc)); new_doc @@ -1789,9 +1787,8 @@ impl DocumentMethods for Document { } // Step 4. *found = true; - let window = self.window(); let filter = NamedElementFilter { name: name }; - let collection = HTMLCollection::create(window.r(), root, box filter); + let collection = HTMLCollection::create(self.window(), root, box filter); collection.r().reflector().get_jsobject().get() } @@ -1849,13 +1846,15 @@ impl DocumentProgressHandler { fn dispatch_dom_content_loaded(&self) { let document = self.addr.root(); let window = document.r().window(); - let event = Event::new(GlobalRef::Window(window.r()), "DOMContentLoaded".to_owned(), + let event = Event::new(GlobalRef::Window(window), "DOMContentLoaded".to_owned(), EventBubbles::DoesNotBubble, EventCancelable::NotCancelable); let doctarget = EventTargetCast::from_ref(document.r()); let _ = doctarget.DispatchEvent(event.r()); - window.r().reflow(ReflowGoal::ForDisplay, ReflowQueryType::NoQuery, ReflowReason::DOMContentLoaded); + window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::DOMContentLoaded); } fn set_ready_state_complete(&self) { @@ -1866,16 +1865,15 @@ impl DocumentProgressHandler { fn dispatch_load(&self) { let document = self.addr.root(); let window = document.r().window(); - let event = Event::new(GlobalRef::Window(window.r()), "load".to_owned(), + let event = Event::new(GlobalRef::Window(window), "load".to_owned(), EventBubbles::DoesNotBubble, EventCancelable::NotCancelable); - let wintarget = EventTargetCast::from_ref(window.r()); + let wintarget = EventTargetCast::from_ref(window); let doctarget = EventTargetCast::from_ref(document.r()); event.r().set_trusted(true); let _ = wintarget.dispatch_event_with_target(doctarget, event.r()); - let window_ref = window.r(); - let browsing_context = window_ref.browsing_context(); + let browsing_context = window.browsing_context(); let browsing_context = browsing_context.as_ref().unwrap(); if let Some(frame_element) = browsing_context.frame_element() { @@ -1892,9 +1890,9 @@ impl DocumentProgressHandler { // https://developer.mozilla.org/en-US/docs/Web/Events/mozbrowserloadend document.r().trigger_mozbrowser_event(MozBrowserEvent::LoadEnd); - window_ref.reflow(ReflowGoal::ForDisplay, - ReflowQueryType::NoQuery, - ReflowReason::DocumentLoaded); + window.reflow(ReflowGoal::ForDisplay, + ReflowQueryType::NoQuery, + ReflowReason::DocumentLoaded); } } @@ -1902,7 +1900,7 @@ impl Runnable for DocumentProgressHandler { fn handler(self: Box) { let document = self.addr.root(); let window = document.r().window(); - if window.r().is_alive() { + if window.is_alive() { match self.task { DocumentProgressTask::DOMContentLoaded => { self.dispatch_dom_content_loaded(); diff --git a/components/script/dom/domimplementation.rs b/components/script/dom/domimplementation.rs index d66cba67bc0..e506c7369c8 100644 --- a/components/script/dom/domimplementation.rs +++ b/components/script/dom/domimplementation.rs @@ -42,7 +42,7 @@ impl DOMImplementation { pub fn new(document: &Document) -> Root { let window = document.window(); reflect_dom_object(box DOMImplementation::new_inherited(document), - GlobalRef::Window(window.r()), + GlobalRef::Window(window), DOMImplementationBinding::Wrap) } } @@ -66,7 +66,7 @@ impl DOMImplementationMethods for DOMImplementation { let loader = DocumentLoader::new(&*doc.loader()); // Step 1. - let doc = Document::new(win.r(), None, IsHTMLDocument::NonHTMLDocument, + let doc = Document::new(win, None, IsHTMLDocument::NonHTMLDocument, None, None, DocumentSource::NotFromParser, loader); // Step 2-3. let maybe_elem = if qname.is_empty() { @@ -114,7 +114,7 @@ impl DOMImplementationMethods for DOMImplementation { let loader = DocumentLoader::new(&*document.loader()); // Step 1-2. - let doc = Document::new(win.r(), None, IsHTMLDocument::HTMLDocument, None, None, + let doc = Document::new(win, None, IsHTMLDocument::HTMLDocument, None, None, DocumentSource::NotFromParser, loader); { diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 651142df713..bc55e2ebbc2 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -1143,7 +1143,7 @@ impl ElementMethods for Element { node.owner_doc() }; let window = doc.r().window(); - NamedNodeMap::new(window.r(), self) + NamedNodeMap::new(window, self) }) } diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index c07022306d6..9e58d68a4e2 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -100,7 +100,6 @@ impl HTMLImageElement { let node = NodeCast::from_ref(self); let document = node.owner_doc(); let window = document.r().window(); - let window = window.r(); let image_cache = window.image_cache_task(); match value { None => { diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index cdfe8716775..51984c73be7 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -106,7 +106,7 @@ static DEFAULT_INPUT_SIZE: u32 = 20; impl HTMLInputElement { fn new_inherited(localName: DOMString, prefix: Option, document: &Document) -> HTMLInputElement { - let chan = document.window().r().constellation_chan(); + let chan = document.window().constellation_chan(); HTMLInputElement { htmlelement: HTMLElement::new_inherited_with_flags(NodeFlags::new() | IN_ENABLED_STATE, diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 19cd81e6519..199ddb2326e 100644 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -84,7 +84,7 @@ impl HTMLTextAreaElement { fn new_inherited(localName: DOMString, prefix: Option, document: &Document) -> HTMLTextAreaElement { - let chan = document.window().r().constellation_chan(); + let chan = document.window().constellation_chan(); HTMLTextAreaElement { htmlelement: HTMLElement::new_inherited_with_flags(NodeFlags::new() | IN_ENABLED_STATE, diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index daacdd09fa0..7674a29340b 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1341,7 +1341,7 @@ impl Node { wrap_fn: extern "Rust" fn(*mut JSContext, GlobalRef, Box) -> Root) -> Root { let window = document.window(); - reflect_dom_object(node, GlobalRef::Window(window.r()), wrap_fn) + reflect_dom_object(node, GlobalRef::Window(window), wrap_fn) } pub fn new_inherited(doc: &Document) -> Node { @@ -1688,7 +1688,7 @@ impl Node { }; let window = document.window(); let loader = DocumentLoader::new(&*document.loader()); - let document = Document::new(window.r(), Some((*document.url()).clone()), + let document = Document::new(window, Some((*document.url()).clone()), is_html_doc, None, None, DocumentSource::NotFromParser, loader); NodeCast::from_root(document) @@ -1929,7 +1929,7 @@ impl NodeMethods for Node { self.child_list.or_init(|| { let doc = self.owner_doc(); let window = doc.r().window(); - NodeList::new_child_list(window.r(), self) + NodeList::new_child_list(window, self) }) } @@ -2428,7 +2428,7 @@ pub fn document_from_node(derived: &T) -> Root(derived: &T) -> Root { let document = document_from_node(derived); - document.r().window() + Root::from_ref(document.r().window()) } impl VirtualMethods for Node { diff --git a/components/script/dom/nodeiterator.rs b/components/script/dom/nodeiterator.rs index c7152231776..0194215da2d 100644 --- a/components/script/dom/nodeiterator.rs +++ b/components/script/dom/nodeiterator.rs @@ -47,9 +47,8 @@ impl NodeIterator { root_node: &Node, what_to_show: u32, filter: Filter) -> Root { - let window = document.window(); reflect_dom_object(box NodeIterator::new_inherited(root_node, what_to_show, filter), - GlobalRef::Window(window.r()), + GlobalRef::Window(document.window()), NodeIteratorBinding::Wrap) } diff --git a/components/script/dom/range.rs b/components/script/dom/range.rs index 0b9a5cc0dbf..52ab9d24d66 100644 --- a/components/script/dom/range.rs +++ b/components/script/dom/range.rs @@ -49,10 +49,9 @@ impl Range { start_container: &Node, start_offset: u32, end_container: &Node, end_offset: u32) -> Root { - let window = document.window(); reflect_dom_object(box Range::new_inherited(start_container, start_offset, end_container, end_offset), - GlobalRef::Window(window.r()), + GlobalRef::Window(document.window()), RangeBinding::Wrap) } diff --git a/components/script/dom/servohtmlparser.rs b/components/script/dom/servohtmlparser.rs index 9b319e66a9a..6d00e702180 100644 --- a/components/script/dom/servohtmlparser.rs +++ b/components/script/dom/servohtmlparser.rs @@ -214,7 +214,6 @@ impl ServoHTMLParser { #[allow(unrooted_must_root)] pub fn new(base_url: Option, document: &Document, pipeline: Option) -> Root { - let window = document.window(); let sink = Sink { base_url: base_url, document: JS::from_ref(document), @@ -237,14 +236,13 @@ impl ServoHTMLParser { pipeline: pipeline, }; - reflect_dom_object(box parser, GlobalRef::Window(window.r()), + reflect_dom_object(box parser, GlobalRef::Window(document.window()), ServoHTMLParserBinding::Wrap) } #[allow(unrooted_must_root)] pub fn new_for_fragment(base_url: Option, document: &Document, fragment_context: FragmentContext) -> Root { - let window = document.window(); let sink = Sink { base_url: base_url, document: JS::from_ref(document), @@ -275,7 +273,7 @@ impl ServoHTMLParser { pipeline: None, }; - reflect_dom_object(box parser, GlobalRef::Window(window.r()), + reflect_dom_object(box parser, GlobalRef::Window(document.window()), ServoHTMLParserBinding::Wrap) } diff --git a/components/script/dom/treewalker.rs b/components/script/dom/treewalker.rs index 7f4646a1618..f19e8383ddf 100644 --- a/components/script/dom/treewalker.rs +++ b/components/script/dom/treewalker.rs @@ -45,9 +45,8 @@ impl TreeWalker { root_node: &Node, what_to_show: u32, filter: Filter) -> Root { - let window = document.window(); reflect_dom_object(box TreeWalker::new_inherited(root_node, what_to_show, filter), - GlobalRef::Window(window.r()), + GlobalRef::Window(document.window()), TreeWalkerBinding::Wrap) } From 6ab7f646203e168c8067acf69ad262e0f3c3fe19 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 02:33:01 +0200 Subject: [PATCH 06/21] Return a reference in BrowserContext::active_window() --- components/script/dom/browsercontext.rs | 8 ++++---- components/script/dom/window.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/script/dom/browsercontext.rs b/components/script/dom/browsercontext.rs index 2b00f346c1e..60dbdc5523e 100644 --- a/components/script/dom/browsercontext.rs +++ b/components/script/dom/browsercontext.rs @@ -48,8 +48,8 @@ impl BrowsingContext { &*self.history[self.active_index].document } - pub fn active_window(&self) -> Root { - Root::from_ref(self.active_document().window()) + pub fn active_window(&self) -> &Window { + self.active_document().window() } pub fn frame_element(&self) -> Option<&Element> { @@ -63,8 +63,8 @@ impl BrowsingContext { #[allow(unsafe_code)] pub fn create_window_proxy(&mut self) { - let win = self.active_window(); - let win = win.r(); + // We inline self.active_window() because we can't borrow *self here. + let win = self.history[self.active_index].document.window(); let WindowProxyHandler(handler) = win.windowproxy_handler(); assert!(!handler.is_null()); diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 3b56b96ddb1..22598fbf844 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1249,7 +1249,7 @@ impl Window { // FIXME(https://github.com/rust-lang/rust/issues/23338) let r = window.r(); let context = r.browsing_context(); - context.as_ref().unwrap().active_window() + Root::from_ref(context.as_ref().unwrap().active_window()) }) } } From ce6aab6cb1c1de5d77a14602a99af57a7fe8eb4f Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 03:10:16 +0200 Subject: [PATCH 07/21] Do not root DOMImplementation::document --- components/script/dom/domimplementation.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/components/script/dom/domimplementation.rs b/components/script/dom/domimplementation.rs index e506c7369c8..93eb8cf8e0a 100644 --- a/components/script/dom/domimplementation.rs +++ b/components/script/dom/domimplementation.rs @@ -53,17 +53,14 @@ impl DOMImplementationMethods for DOMImplementation { fn CreateDocumentType(&self, qualified_name: DOMString, pubid: DOMString, sysid: DOMString) -> Fallible> { try!(validate_qualified_name(&qualified_name)); - let document = self.document.root(); - Ok(DocumentType::new(qualified_name, Some(pubid), Some(sysid), document.r())) + Ok(DocumentType::new(qualified_name, Some(pubid), Some(sysid), &self.document)) } // https://dom.spec.whatwg.org/#dom-domimplementation-createdocument fn CreateDocument(&self, namespace: Option, qname: DOMString, maybe_doctype: Option<&DocumentType>) -> Fallible> { - let doc = self.document.root(); - let doc = doc.r(); - let win = doc.window(); - let loader = DocumentLoader::new(&*doc.loader()); + let win = self.document.window(); + let loader = DocumentLoader::new(&self.document.loader()); // Step 1. let doc = Document::new(win, None, IsHTMLDocument::NonHTMLDocument, @@ -108,10 +105,8 @@ impl DOMImplementationMethods for DOMImplementation { // https://dom.spec.whatwg.org/#dom-domimplementation-createhtmldocument fn CreateHTMLDocument(&self, title: Option) -> Root { - let document = self.document.root(); - let document = document.r(); - let win = document.window(); - let loader = DocumentLoader::new(&*document.loader()); + let win = self.document.window(); + let loader = DocumentLoader::new(&self.document.loader()); // Step 1-2. let doc = Document::new(win, None, IsHTMLDocument::HTMLDocument, None, None, From ff0da2f6428a95c5f10f26d7b6b86c9608f494e1 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 03:25:46 +0200 Subject: [PATCH 08/21] Do not root DOMParser::window --- components/script/dom/domparser.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/components/script/dom/domparser.rs b/components/script/dom/domparser.rs index 8ad9bfa507b..a66b8ea3962 100644 --- a/components/script/dom/domparser.rs +++ b/components/script/dom/domparser.rs @@ -49,15 +49,14 @@ impl DOMParserMethods for DOMParser { s: DOMString, ty: DOMParserBinding::SupportedType) -> Fallible> { - let window = self.window.root(); - let url = window.r().get_url(); + let url = self.window.get_url(); let content_type = DOMParserBinding::SupportedTypeValues::strings[ty as usize].to_owned(); - let doc = window.r().Document(); + let doc = self.window.Document(); let doc = doc.r(); let loader = DocumentLoader::new(&*doc.loader()); match ty { Text_html => { - let document = Document::new(window.r(), Some(url.clone()), + let document = Document::new(&self.window, Some(url.clone()), IsHTMLDocument::HTMLDocument, Some(content_type), None, @@ -69,7 +68,7 @@ impl DOMParserMethods for DOMParser { } Text_xml => { //FIXME: this should probably be FromParser when we actually parse the string (#3756). - Ok(Document::new(window.r(), Some(url.clone()), + Ok(Document::new(&self.window, Some(url.clone()), IsHTMLDocument::NonHTMLDocument, Some(content_type), None, From 822e6f0d48158950a3b9212b2b7e93478359b58c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 14:30:23 +0200 Subject: [PATCH 09/21] Do not root DOMStringMap::element --- components/script/dom/domstringmap.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/components/script/dom/domstringmap.rs b/components/script/dom/domstringmap.rs index cfa5a8e0a1c..95f727c93cb 100644 --- a/components/script/dom/domstringmap.rs +++ b/components/script/dom/domstringmap.rs @@ -37,20 +37,17 @@ impl DOMStringMap { impl DOMStringMapMethods for DOMStringMap { // https://html.spec.whatwg.org/multipage/#dom-domstringmap-removeitem fn NamedDeleter(&self, name: DOMString) { - let element = self.element.root(); - element.r().delete_custom_attr(name) + self.element.delete_custom_attr(name) } // https://html.spec.whatwg.org/multipage/#dom-domstringmap-setitem fn NamedSetter(&self, name: DOMString, value: DOMString) -> ErrorResult { - let element = self.element.root(); - element.r().set_custom_attr(name, value) + self.element.set_custom_attr(name, value) } // https://html.spec.whatwg.org/multipage/#dom-domstringmap-nameditem fn NamedGetter(&self, name: DOMString, found: &mut bool) -> DOMString { - let element = self.element.root(); - match element.r().get_custom_attr(name) { + match self.element.get_custom_attr(name) { Some(value) => { *found = true; value.clone() From 71dcabfad88d39bb9d9aa8b9264e2634216d8701 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 14:30:41 +0200 Subject: [PATCH 10/21] Do not root DOMTokenList::element --- components/script/dom/domtokenlist.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/components/script/dom/domtokenlist.rs b/components/script/dom/domtokenlist.rs index e13c1448d70..c3944b7627c 100644 --- a/components/script/dom/domtokenlist.rs +++ b/components/script/dom/domtokenlist.rs @@ -39,8 +39,7 @@ impl DOMTokenList { } fn attribute(&self) -> Option> { - let element = self.element.root(); - element.r().get_attribute(&ns!(""), &self.local_name) + self.element.get_attribute(&ns!(""), &self.local_name) } fn check_token_exceptions(&self, token: &str) -> Fallible { @@ -87,43 +86,40 @@ impl DOMTokenListMethods for DOMTokenList { // https://dom.spec.whatwg.org/#dom-domtokenlist-add fn Add(&self, tokens: Vec) -> ErrorResult { - let element = self.element.root(); - let mut atoms = element.r().get_tokenlist_attribute(&self.local_name); + let mut atoms = self.element.get_tokenlist_attribute(&self.local_name); for token in &tokens { let token = try!(self.check_token_exceptions(&token)); if !atoms.iter().any(|atom| *atom == token) { atoms.push(token); } } - element.r().set_atomic_tokenlist_attribute(&self.local_name, atoms); + self.element.set_atomic_tokenlist_attribute(&self.local_name, atoms); Ok(()) } // https://dom.spec.whatwg.org/#dom-domtokenlist-remove fn Remove(&self, tokens: Vec) -> ErrorResult { - let element = self.element.root(); - let mut atoms = element.r().get_tokenlist_attribute(&self.local_name); + let mut atoms = self.element.get_tokenlist_attribute(&self.local_name); for token in &tokens { let token = try!(self.check_token_exceptions(&token)); atoms.iter().position(|atom| *atom == token).map(|index| { atoms.remove(index) }); } - element.r().set_atomic_tokenlist_attribute(&self.local_name, atoms); + self.element.set_atomic_tokenlist_attribute(&self.local_name, atoms); Ok(()) } // https://dom.spec.whatwg.org/#dom-domtokenlist-toggle fn Toggle(&self, token: DOMString, force: Option) -> Fallible { - let element = self.element.root(); - let mut atoms = element.r().get_tokenlist_attribute(&self.local_name); + let mut atoms = self.element.get_tokenlist_attribute(&self.local_name); let token = try!(self.check_token_exceptions(&token)); match atoms.iter().position(|atom| *atom == token) { Some(index) => match force { Some(true) => Ok(true), _ => { atoms.remove(index); - element.r().set_atomic_tokenlist_attribute(&self.local_name, atoms); + self.element.set_atomic_tokenlist_attribute(&self.local_name, atoms); Ok(false) } }, @@ -131,7 +127,7 @@ impl DOMTokenListMethods for DOMTokenList { Some(false) => Ok(false), _ => { atoms.push(token); - element.r().set_atomic_tokenlist_attribute(&self.local_name, atoms); + self.element.set_atomic_tokenlist_attribute(&self.local_name, atoms); Ok(true) } } @@ -140,7 +136,7 @@ impl DOMTokenListMethods for DOMTokenList { // https://dom.spec.whatwg.org/#stringification-behavior fn Stringifier(&self) -> DOMString { - let tokenlist = self.element.root().r().get_tokenlist_attribute(&self.local_name); + let tokenlist = self.element.get_tokenlist_attribute(&self.local_name); str_join(&tokenlist, "\x20") } From e8f358d17805cfafa38313a3c2ce66c9c026682e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:04:55 +0200 Subject: [PATCH 11/21] Do not root InputActivationState::checked_radio --- components/script/dom/htmlinputelement.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 51984c73be7..d8cac420b5d 100644 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -758,17 +758,16 @@ impl Activatable for HTMLInputElement { InputType::InputRadio => { // We want to restore state only if the element had been changed in the first place if cache.was_mutable { - let old_checked = cache.checked_radio.as_ref().map(|t| t.root()); let name = self.get_radio_group_name(); - match old_checked { - Some(ref o) => { + match cache.checked_radio.as_ref().map(|t| &*t) { + Some(o) => { // Avoiding iterating through the whole tree here, instead // we can check if the conditions for radio group siblings apply - if name == o.r().get_radio_group_name() && // TODO should be compatibility caseless - self.form_owner() == o.r().form_owner() && + if name == o.get_radio_group_name() && // TODO should be compatibility caseless + self.form_owner() == o.form_owner() && // TODO Both a and b are in the same home subtree - o.r().input_type.get() == InputType::InputRadio { - o.r().SetChecked(true); + o.input_type.get() == InputType::InputRadio { + o.SetChecked(true); } else { self.SetChecked(false); } From dee3aecea1edfcf8ff86f1f52bf0d70c892e2377 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:11:45 +0200 Subject: [PATCH 12/21] Do not root HTMLScriptElement::parser_document --- components/script/dom/htmlscriptelement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index b6f9e97e712..579fe0c7c1b 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -209,7 +209,7 @@ impl HTMLScriptElement { // Step 10. let document_from_node_ref = document_from_node(self); let document_from_node_ref = document_from_node_ref.r(); - if self.parser_inserted.get() && self.parser_document.root().r() != document_from_node_ref { + if self.parser_inserted.get() && &*self.parser_document != document_from_node_ref { return NextParserState::Continue; } From 5889a75b10591172a866464cd6f349761b0741b7 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:16:19 +0200 Subject: [PATCH 13/21] Do not root Location::window --- components/script/dom/location.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/components/script/dom/location.rs b/components/script/dom/location.rs index c366139649b..230cf2748da 100644 --- a/components/script/dom/location.rs +++ b/components/script/dom/location.rs @@ -35,33 +35,31 @@ impl Location { } fn get_url(&self) -> Url { - self.window.root().get_url() + self.window.get_url() } fn set_url_component(&self, value: USVString, setter: fn(&mut Url, USVString)) { - let window = self.window.root(); - let mut url = window.get_url(); + let mut url = self.window.get_url(); setter(&mut url, value); - window.load_url(url); + self.window.load_url(url); } } impl LocationMethods for Location { // https://html.spec.whatwg.org/multipage/#dom-location-assign fn Assign(&self, url: DOMString) { - let window = self.window.root(); // TODO: per spec, we should use the _API base URL_ specified by the // _entry settings object_. - let base_url = window.get_url(); + let base_url = self.window.get_url(); if let Ok(url) = UrlParser::new().base_url(&base_url).parse(&url) { - window.load_url(url); + self.window.load_url(url); } } // https://html.spec.whatwg.org/multipage/#dom-location-reload fn Reload(&self) { - self.window.root().load_url(self.get_url()); + self.window.load_url(self.get_url()); } // https://url.spec.whatwg.org/#dom-urlutils-hash @@ -101,9 +99,8 @@ impl LocationMethods for Location { // https://url.spec.whatwg.org/#dom-urlutils-href fn SetHref(&self, value: USVString) -> ErrorResult { - let window = self.window.root(); - if let Ok(url) = UrlParser::new().base_url(&window.get_url()).parse(&value.0) { - window.load_url(url); + if let Ok(url) = UrlParser::new().base_url(&self.window.get_url()).parse(&value.0) { + self.window.load_url(url); }; Ok(()) } From 6849510526fce9b836509d24ddd9858560698cad Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:20:51 +0200 Subject: [PATCH 14/21] Do not root NamedNodeMap::owner --- components/script/dom/namednodemap.rs | 35 ++++++--------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/components/script/dom/namednodemap.rs b/components/script/dom/namednodemap.rs index 078ddcc0d84..6070c0bb0c2 100644 --- a/components/script/dom/namednodemap.rs +++ b/components/script/dom/namednodemap.rs @@ -37,57 +37,38 @@ impl NamedNodeMap { impl NamedNodeMapMethods for NamedNodeMap { // https://dom.spec.whatwg.org/#dom-namednodemap-length fn Length(&self) -> u32 { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); - let attrs = owner.attrs(); - attrs.len() as u32 + self.owner.attrs().len() as u32 } // https://dom.spec.whatwg.org/#dom-namednodemap-item fn Item(&self, index: u32) -> Option> { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); - let attrs = owner.attrs(); - attrs.get(index as usize).map(|t| t.root()) + self.owner.attrs().get(index as usize).map(JS::root) } // https://dom.spec.whatwg.org/#dom-namednodemap-getnameditem fn GetNamedItem(&self, name: DOMString) -> Option> { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); - owner.get_attribute_by_name(name) + self.owner.get_attribute_by_name(name) } // https://dom.spec.whatwg.org/#dom-namednodemap-getnameditemns fn GetNamedItemNS(&self, namespace: Option, local_name: DOMString) -> Option> { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); let ns = namespace_from_domstring(namespace); - owner.get_attribute(&ns, &Atom::from_slice(&local_name)) + self.owner.get_attribute(&ns, &Atom::from_slice(&local_name)) } // https://dom.spec.whatwg.org/#dom-namednodemap-removenameditem fn RemoveNamedItem(&self, name: DOMString) -> Fallible> { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); - let name = owner.parsed_name(name); - owner.remove_attribute_by_name(&name).ok_or(Error::NotFound) + let name = self.owner.parsed_name(name); + self.owner.remove_attribute_by_name(&name).ok_or(Error::NotFound) } // https://dom.spec.whatwg.org/#dom-namednodemap-removenameditemns fn RemoveNamedItemNS(&self, namespace: Option, local_name: DOMString) -> Fallible> { - let owner = self.owner.root(); - // FIXME(https://github.com/rust-lang/rust/issues/23338) - let owner = owner.r(); let ns = namespace_from_domstring(namespace); - owner.remove_attribute(&ns, &Atom::from_slice(&local_name)).ok_or(Error::NotFound) + self.owner.remove_attribute(&ns, &Atom::from_slice(&local_name)) + .ok_or(Error::NotFound) } // https://dom.spec.whatwg.org/#dom-namednodemap-item From b149e0c97935c407bf5bac14722c76fdd6d0e407 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:24:02 +0200 Subject: [PATCH 15/21] Do not root NodeIterator::root_node --- components/script/dom/nodeiterator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/script/dom/nodeiterator.rs b/components/script/dom/nodeiterator.rs index 0194215da2d..27a99ec3b55 100644 --- a/components/script/dom/nodeiterator.rs +++ b/components/script/dom/nodeiterator.rs @@ -121,7 +121,7 @@ impl NodeIteratorMethods for NodeIterator { } // Step 3-1. - for following_node in node.r().following_nodes(self.root_node.root().r()) { + for following_node in node.r().following_nodes(&self.root_node) { // Step 3-2. let result = try!(self.accept_node(following_node.r())); @@ -165,7 +165,7 @@ impl NodeIteratorMethods for NodeIterator { } // Step 3-1. - for preceding_node in node.r().preceding_nodes(self.root_node.root().r()) { + for preceding_node in node.r().preceding_nodes(&self.root_node) { // Step 3-2. let result = try!(self.accept_node(preceding_node.r())); From 17cd4202e7d76bf155664eae64dfd0233f3165cc Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:27:46 +0200 Subject: [PATCH 16/21] Do not root ChildrenList::node --- components/script/dom/nodelist.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/script/dom/nodelist.rs b/components/script/dom/nodelist.rs index b5db05945ca..2ed127dd2dc 100644 --- a/components/script/dom/nodelist.rs +++ b/components/script/dom/nodelist.rs @@ -109,7 +109,7 @@ impl ChildrenList { } pub fn len(&self) -> u32 { - self.node.root().children_count() + self.node.children_count() } pub fn item(&self, index: u32) -> Option> { @@ -121,7 +121,7 @@ impl ChildrenList { } if index == 0u32 { // Item is first child if any, not worth updating last visited. - return self.node.root().GetFirstChild(); + return self.node.GetFirstChild(); } let last_index = self.last_index.get(); if index == last_index { @@ -137,7 +137,7 @@ impl ChildrenList { } else if index > last_index { if index == len - 1u32 { // Item is parent's last child, not worth updating last visited. - return Some(self.node.root().GetLastChild().unwrap()); + return Some(self.node.GetLastChild().unwrap()); } if index <= last_index + (len - last_index) / 2u32 { // Item is closer to the last visited child and follows it. @@ -147,7 +147,7 @@ impl ChildrenList { } else { // Item is closer to parent's last child and obviously // precedes it. - self.node.root().GetLastChild().unwrap() + self.node.GetLastChild().unwrap() .inclusively_preceding_siblings() .nth((len - index - 1u32) as usize).unwrap() } @@ -159,7 +159,7 @@ impl ChildrenList { } else { // Item is closer to parent's first child and obviously follows it. debug_assert!(index < last_index / 2u32); - self.node.root().GetFirstChild().unwrap() + self.node.GetFirstChild().unwrap() .inclusively_following_siblings() .nth(index as usize) .unwrap() @@ -263,7 +263,7 @@ impl ChildrenList { } fn reset(&self) { - self.last_visited.set(self.node.root().GetFirstChild().as_ref().map(Root::r)); + self.last_visited.set(self.node.GetFirstChild().r()); self.last_index.set(0u32); } } From b0d1ccdf5f174ca893ffaba139e2207fdaece020 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 15:28:26 +0200 Subject: [PATCH 17/21] Do not root Performance::timing --- components/script/dom/performance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/performance.rs b/components/script/dom/performance.rs index 2d0a5990cdc..b8b6c17ef09 100644 --- a/components/script/dom/performance.rs +++ b/components/script/dom/performance.rs @@ -51,7 +51,7 @@ impl PerformanceMethods for Performance { // https://dvcs.w3.org/hg/webperf/raw-file/tip/specs/HighResolutionTime/Overview.html#dom-performance-now fn Now(&self) -> DOMHighResTimeStamp { - let navStart = self.timing.root().r().NavigationStartPrecise(); + let navStart = self.timing.NavigationStartPrecise(); let now = (time::precise_time_ns() as f64 - navStart) / 1000000 as f64; Finite::wrap(now) } From 7c47a6b78e615eaf4d896513269bb5d318396c14 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 16:00:41 +0200 Subject: [PATCH 18/21] Do not root Sink::document --- components/script/dom/servohtmlparser.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/components/script/dom/servohtmlparser.rs b/components/script/dom/servohtmlparser.rs index 6d00e702180..e527a090d13 100644 --- a/components/script/dom/servohtmlparser.rs +++ b/components/script/dom/servohtmlparser.rs @@ -48,8 +48,7 @@ impl Sink { match child { NodeOrText::AppendNode(n) => n.root(), NodeOrText::AppendText(t) => { - let doc = self.document.root(); - let text = Text::new(t.into(), &doc); + let text = Text::new(t.into(), &self.document); NodeCast::from_root(text) } } From da0634157806e50894c05dd4110717047dd67eb4 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 16:00:51 +0200 Subject: [PATCH 19/21] Do not root ServoHTMLParser::document --- components/script/dom/servohtmlparser.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/components/script/dom/servohtmlparser.rs b/components/script/dom/servohtmlparser.rs index e527a090d13..8ab97382d19 100644 --- a/components/script/dom/servohtmlparser.rs +++ b/components/script/dom/servohtmlparser.rs @@ -152,8 +152,7 @@ impl AsyncResponseListener for ParserContext { Some(parser) => parser.root(), None => return, }; - let doc = parser.r().document.root(); - doc.r().finish_load(LoadType::PageSource(self.url.clone())); + parser.document.finish_load(LoadType::PageSource(self.url.clone())); if let Err(err) = status { debug!("Failed to load page URL {}, error: {}", self.url.serialize(), err); @@ -188,7 +187,7 @@ pub struct ServoHTMLParser { impl<'a> Parser for &'a ServoHTMLParser { fn parse_chunk(self, input: String) { - self.document.root().r().set_current_parser(Some(self)); + self.document.set_current_parser(Some(self)); self.pending_input.borrow_mut().push(input); self.parse_sync(); } @@ -200,8 +199,7 @@ impl<'a> Parser for &'a ServoHTMLParser { self.tokenizer().borrow_mut().end(); debug!("finished parsing"); - let document = self.document.root(); - document.r().set_current_parser(None); + self.document.set_current_parser(None); if let Some(pipeline) = self.pipeline { ScriptTask::parsing_complete(pipeline); @@ -298,8 +296,7 @@ impl ServoHTMLParser { break; } - let document = self.document.root(); - document.r().reflow_if_reflow_timer_expired(); + self.document.reflow_if_reflow_timer_expired(); let mut pending_input = self.pending_input.borrow_mut(); if !pending_input.is_empty() { @@ -318,8 +315,7 @@ impl ServoHTMLParser { } fn window(&self) -> Root { - let doc = self.document.root(); - window_from_node(doc.r()) + window_from_node(&*self.document) } } From d28de59e4af1542b75e15420263c91b754730663 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 16:03:24 +0200 Subject: [PATCH 20/21] Return a reference in ServoHTMLParser::window() --- components/script/dom/servohtmlparser.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/script/dom/servohtmlparser.rs b/components/script/dom/servohtmlparser.rs index 8ab97382d19..2cf4bac0a4c 100644 --- a/components/script/dom/servohtmlparser.rs +++ b/components/script/dom/servohtmlparser.rs @@ -15,7 +15,7 @@ use dom::bindings::refcounted::Trusted; use dom::bindings::trace::JSTraceable; use dom::bindings::utils::{Reflector, reflect_dom_object}; use dom::document::Document; -use dom::node::{Node, window_from_node}; +use dom::node::Node; use dom::text::Text; use dom::window::Window; use encoding::all::UTF_8; @@ -108,7 +108,7 @@ impl AsyncResponseListener for ParserContext { let parser = parser.r(); let win = parser.window(); - self.parser = Some(Trusted::new(win.r().get_cx(), parser, self.script_chan.clone())); + self.parser = Some(Trusted::new(win.get_cx(), parser, self.script_chan.clone())); match content_type { Some(ContentType(Mime(TopLevel::Image, _, _))) => { @@ -314,8 +314,8 @@ impl ServoHTMLParser { } } - fn window(&self) -> Root { - window_from_node(&*self.document) + fn window(&self) -> &Window { + self.document.window() } } From 6dc42dd1d6979cea7059b055de7f0d6f93c14338 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Sat, 17 Oct 2015 16:46:32 +0200 Subject: [PATCH 21/21] Do not root XMLHttpRequest::upload --- components/script/dom/xmlhttprequest.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 3373865fa5f..400b06ca95f 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -493,8 +493,7 @@ impl XMLHttpRequestMethods for XMLHttpRequest { if !self.sync.get() { // Step 8 - let upload_target = self.upload.root(); - let event_target = EventTargetCast::from_ref(upload_target.r()); + let event_target = EventTargetCast::from_ref(&*self.upload); if event_target.has_handlers() { self.upload_events.set(true); } @@ -917,13 +916,12 @@ impl XMLHttpRequest { fn dispatch_progress_event(&self, upload: bool, type_: DOMString, loaded: u64, total: Option) { let global = self.global.root(); - let upload_target = self.upload.root(); let progressevent = ProgressEvent::new(global.r(), type_, EventBubbles::DoesNotBubble, EventCancelable::NotCancelable, total.is_some(), loaded, total.unwrap_or(0)); let target = if upload { - EventTargetCast::from_ref(upload_target.r()) + EventTargetCast::from_ref(&*self.upload) } else { EventTargetCast::from_ref(self) };