Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Convert TNode / TElement to use self parameters rather than &self #3540

Merged
merged 8 commits into from Oct 1, 2014
Merged

Convert TNode / TElement to use self parameters rather than &self #3540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ef0f39
Remove a prefix from a method name by manually resolving methods
Oct 1, 2014
be9618d
Add a lifetime parameter to the ElementHelper trait
Oct 1, 2014
e894499
Disambiguate methods without using trait objects
Oct 1, 2014
ccc5d97
Refine a lifetime parameter on TElement's get_local_name
Oct 1, 2014
1581c38
Convert the remaining ElementHelpers methods to take self
Oct 1, 2014
6331420
Add Copy bounds to TNode / TElement
Oct 1, 2014
79b4ec9
Convert TNode / TElement to use self methods rather than &self
Oct 1, 2014
8b0361e
Remove an unused lifetime parameter
Oct 1, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 25 additions & 19 deletions components/layout/wrapper.rs
View file
Open in desktop
Expand Up @@ -213,8 +213,14 @@ impl<'ln> LayoutNode<'ln> {

/// Returns an iterator over this node's children.
pub fn children(&self) -> LayoutNodeChildrenIterator<'ln> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn first_child<T: TLayoutNode>(this: &T) -> Option<T> {
this.first_child()
}

LayoutNodeChildrenIterator {
current_node: self.first_child(),
current_node: first_child(self),
}
}

Expand All @@ -241,33 +247,33 @@ impl<'ln> LayoutNode<'ln> {
}

impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
fn parent_node(&self) -> Option<LayoutNode<'ln>> {
fn parent_node(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.parent_node_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn tnode_first_child(&self) -> Option<LayoutNode<'ln>> {
fn first_child(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.first_child_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn prev_sibling(&self) -> Option<LayoutNode<'ln>> {
fn prev_sibling(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn next_sibling(&self) -> Option<LayoutNode<'ln>> {
fn next_sibling(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

/// If this is an element, accesses the element data. Fails if this is not an element node.
#[inline]
fn as_element(&self) -> LayoutElement<'ln> {
fn as_element(self) -> LayoutElement<'ln> {
unsafe {
assert!(self.node.is_element_for_layout());
let elem: JS<Element> = self.node.transmute_copy();
Expand All @@ -278,15 +284,15 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
}
}

fn is_element(&self) -> bool {
fn is_element(self) -> bool {
self.node_is_element()
}

fn is_document(&self) -> bool {
fn is_document(self) -> bool {
self.node_is_document()
}

fn match_attr(&self, attr: &AttrSelector, test: |&str| -> bool) -> bool {
fn match_attr(self, attr: &AttrSelector, test: |&str| -> bool) -> bool {
assert!(self.is_element())
let name = if self.is_html_element_in_html_document() {
attr.lower_name.as_slice()
Expand All @@ -304,7 +310,7 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
}
}

fn is_html_element_in_html_document(&self) -> bool {
fn is_html_element_in_html_document(self) -> bool {
unsafe {
self.is_element() && {
let element: JS<Element> = self.node.transmute_copy();
Expand Down Expand Up @@ -389,21 +395,21 @@ impl<'le> LayoutElement<'le> {

impl<'le> TElement<'le> for LayoutElement<'le> {
#[inline]
fn get_local_name<'a>(&'a self) -> &'a Atom {
fn get_local_name(self) -> &'le Atom {
&self.element.local_name
}

#[inline]
fn get_namespace<'a>(&'a self) -> &'a Namespace {
fn get_namespace(self) -> &'le Namespace {
&self.element.namespace
}

#[inline]
fn get_attr(&self, namespace: &Namespace, name: &str) -> Option<&'le str> {
fn get_attr(self, namespace: &Namespace, name: &str) -> Option<&'le str> {
unsafe { self.element.get_attr_val_for_layout(namespace, name) }
}

fn get_link(&self) -> Option<&'le str> {
fn get_link(self) -> Option<&'le str> {
// FIXME: This is HTML only.
match self.element.node.type_id_for_layout() {
// http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#
Expand All @@ -417,30 +423,30 @@ impl<'le> TElement<'le> for LayoutElement<'le> {
}
}

fn get_hover_state(&self) -> bool {
fn get_hover_state(self) -> bool {
unsafe {
self.element.node.get_hover_state_for_layout()
}
}

#[inline]
fn get_id(&self) -> Option<Atom> {
fn get_id(self) -> Option<Atom> {
unsafe { self.element.get_attr_atom_for_layout(&ns!(""), "id") }
}

fn get_disabled_state(&self) -> bool {
fn get_disabled_state(self) -> bool {
unsafe {
self.element.node.get_disabled_state_for_layout()
}
}

fn get_enabled_state(&self) -> bool {
fn get_enabled_state(self) -> bool {
unsafe {
self.element.node.get_enabled_state_for_layout()
}
}

fn has_class(&self, name: &str) -> bool {
fn has_class(self, name: &str) -> bool {
unsafe {
self.element.has_class_for_layout(name)
}
Expand Down
66 changes: 42 additions & 24 deletions components/script/dom/element.rs
View file
Open in desktop
Expand Up @@ -237,26 +237,26 @@ impl LayoutElementHelpers for JS<Element> {
}
}

pub trait ElementHelpers {
pub trait ElementHelpers<'a> {
fn html_element_in_html_document(self) -> bool;
fn get_local_name<'a>(&'a self) -> &'a Atom;
fn get_namespace<'a>(&'a self) -> &'a Namespace;
fn get_local_name(self) -> &'a Atom;
fn get_namespace(self) -> &'a Namespace;
fn summarize(self) -> Vec<AttrInfo>;
fn is_void(self) -> bool;
}

impl<'a> ElementHelpers for JSRef<'a, Element> {
impl<'a> ElementHelpers<'a> for JSRef<'a, Element> {
fn html_element_in_html_document(self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(self);
self.namespace == ns!(HTML) && node.is_in_html_doc()
}

fn get_local_name<'a>(&'a self) -> &'a Atom {
&self.deref().local_name
fn get_local_name(self) -> &'a Atom {
&self.extended_deref().local_name
}

fn get_namespace<'a>(&'a self) -> &'a Namespace {
&self.deref().namespace
fn get_namespace(self) -> &'a Namespace {
&self.extended_deref().namespace
}

fn summarize(self) -> Vec<AttrInfo> {
Expand Down Expand Up @@ -948,14 +948,14 @@ impl<'a> VirtualMethods for JSRef<'a, Element> {
}

impl<'a> style::TElement<'a> for JSRef<'a, Element> {
fn get_attr(&self, namespace: &Namespace, attr: &str) -> Option<&'a str> {
fn get_attr(self, namespace: &Namespace, attr: &str) -> Option<&'a str> {
self.get_attribute(namespace.clone(), attr).root().map(|attr| {
unsafe { mem::transmute(attr.deref().value().as_slice()) }
})
}
fn get_link(&self) -> Option<&'a str> {
fn get_link(self) -> Option<&'a str> {
// FIXME: This is HTML only.
let node: JSRef<Node> = NodeCast::from_ref(*self);
let node: JSRef<Node> = NodeCast::from_ref(self);
match node.type_id() {
// http://www.whatwg.org/specs/web-apps/current-work/multipage/selectors.html#
// selector-link
Expand All @@ -965,17 +965,29 @@ impl<'a> style::TElement<'a> for JSRef<'a, Element> {
_ => None,
}
}
fn get_local_name<'b>(&'b self) -> &'b Atom {
(self as &ElementHelpers).get_local_name()
fn get_local_name(self) -> &'a Atom {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn get_local_name<'a, T: ElementHelpers<'a>>(this: T) -> &'a Atom {
this.get_local_name()
}

get_local_name(self)
}
fn get_namespace<'b>(&'b self) -> &'b Namespace {
(self as &ElementHelpers).get_namespace()
fn get_namespace(self) -> &'a Namespace {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn get_namespace<'a, T: ElementHelpers<'a>>(this: T) -> &'a Namespace {
this.get_namespace()
}

get_namespace(self)
}
fn get_hover_state(&self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(*self);
fn get_hover_state(self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(self);
node.get_hover_state()
}
fn get_id<'a>(&self) -> Option<Atom> {
fn get_id(self) -> Option<Atom> {
self.get_attribute(ns!(""), "id").map(|attr| {
let attr = attr.root();
match *attr.value() {
Expand All @@ -984,15 +996,21 @@ impl<'a> style::TElement<'a> for JSRef<'a, Element> {
}
})
}
fn get_disabled_state(&self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(*self);
fn get_disabled_state(self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(self);
node.get_disabled_state()
}
fn get_enabled_state(&self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(*self);
fn get_enabled_state(self) -> bool {
let node: JSRef<Node> = NodeCast::from_ref(self);
node.get_enabled_state()
}
fn has_class(&self, name: &str) -> bool {
(self as &AttributeHandlers).has_class(name)
fn has_class(self, name: &str) -> bool {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn has_class<T: AttributeHandlers>(this: T, name: &str) -> bool {
this.has_class(name)
}

has_class(self, name)
}
}
70 changes: 53 additions & 17 deletions components/script/dom/node.rs
View file
Open in desktop
Expand Up @@ -2035,37 +2035,73 @@ impl<'a> VirtualMethods for JSRef<'a, Node> {
}

impl<'a> style::TNode<'a, JSRef<'a, Element>> for JSRef<'a, Node> {
fn parent_node(&self) -> Option<JSRef<'a, Node>> {
(self as &NodeHelpers).parent_node().map(|node| *node.root())
fn parent_node(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn parent_node<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> Option<Temporary<Node>> {
this.parent_node()
}

parent_node(self).map(|node| *node.root())
}

fn tnode_first_child(&self) -> Option<JSRef<'a, Node>> {
(self as &NodeHelpers).first_child().map(|node| *node.root())
fn first_child(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn first_child<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> Option<Temporary<Node>> {
this.first_child()
}

first_child(self).map(|node| *node.root())
}

fn prev_sibling(&self) -> Option<JSRef<'a, Node>> {
(self as &NodeHelpers).prev_sibling().map(|node| *node.root())
fn prev_sibling(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn prev_sibling<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> Option<Temporary<Node>> {
this.prev_sibling()
}

prev_sibling(self).map(|node| *node.root())
}

fn next_sibling(&self) -> Option<JSRef<'a, Node>> {
(self as &NodeHelpers).next_sibling().map(|node| *node.root())
fn next_sibling(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn next_sibling<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> Option<Temporary<Node>> {
this.next_sibling()
}

next_sibling(self).map(|node| *node.root())
}

fn is_document(&self) -> bool {
(self as &NodeHelpers).is_document()
fn is_document(self) -> bool {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn is_document<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> bool {
this.is_document()
}

is_document(self)
}

fn is_element(&self) -> bool {
(self as &NodeHelpers).is_element()
fn is_element(self) -> bool {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn is_element<'a, 'b, T: NodeHelpers<'a, 'b>>(this: T) -> bool {
this.is_element()
}

is_element(self)
}

fn as_element(&self) -> JSRef<'a, Element> {
let elem: Option<JSRef<'a, Element>> = ElementCast::to_ref(*self);
fn as_element(self) -> JSRef<'a, Element> {
let elem: Option<JSRef<'a, Element>> = ElementCast::to_ref(self);
assert!(elem.is_some());
elem.unwrap()
}

fn match_attr(&self, attr: &style::AttrSelector, test: |&str| -> bool) -> bool {
fn match_attr(self, attr: &style::AttrSelector, test: |&str| -> bool) -> bool {
let name = {
if self.is_html_element_in_html_document() {
attr.lower_name.as_slice()
Expand All @@ -2083,8 +2119,8 @@ impl<'a> style::TNode<'a, JSRef<'a, Element>> for JSRef<'a, Node> {
}
}

fn is_html_element_in_html_document(&self) -> bool {
let elem: Option<JSRef<'a, Element>> = ElementCast::to_ref(*self);
fn is_html_element_in_html_document(self) -> bool {
let elem: Option<JSRef<'a, Element>> = ElementCast::to_ref(self);
assert!(elem.is_some());
elem.unwrap().html_element_in_html_document()
}
Expand Down