Skip to content

Commit

Permalink
Add a to_js method to the casting trait code in CodegenRust.py
Browse files Browse the repository at this point in the history 
Replace the manual checks and calls to transmute_copy in /layout/wrapper.rs with calls to to_js

Fixes #3616
  • Loading branch information
@kaseyc
kaseyc committed Oct 15, 2014
1 parent 3eb6b17 commit 6d91e92
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 61 deletions.
119 changes: 58 additions & 61 deletions components/layout/wrapper.rs
View file
Expand Up @@ -21,10 +21,7 @@
//!
//! Rules of the road for this file:
//!
//! * In general, you must not use the `Cast` functions; use explicit checks and `transmute_copy`
//! instead.
//!
//! * You must also not use `.get()`; instead, use `.unsafe_get()`.
//! * You must not use `.get()`; instead, use `.unsafe_get()`.
//!
//! * Do not call any methods on DOM nodes without checking to see whether they use borrow flags.
//!
Expand All @@ -38,17 +35,16 @@ use css::node_style::StyledNode;
use util::{LayoutDataAccess, LayoutDataWrapper, PrivateLayoutData, OpaqueNodeMethods};

use gfx::display_list::OpaqueNode;
use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementDerived};
use script::dom::bindings::codegen::InheritTypes::{HTMLImageElementDerived};
use script::dom::bindings::codegen::InheritTypes::{HTMLInputElementDerived, TextDerived};
use script::dom::bindings::codegen::InheritTypes::{ElementCast, HTMLIFrameElementCast, HTMLImageElementCast};
use script::dom::bindings::codegen::InheritTypes::{HTMLInputElementCast, TextCast};
use script::dom::bindings::js::JS;
use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementTypeId};
use script::dom::element::{HTMLLinkElementTypeId, LayoutElementHelpers, RawLayoutElementHelpers};
use script::dom::htmliframeelement::HTMLIFrameElement;
use script::dom::htmlimageelement::{HTMLImageElement, LayoutHTMLImageElementHelpers};
use script::dom::htmlinputelement::{HTMLInputElement, LayoutHTMLInputElementHelpers};
use script::dom::htmlimageelement::LayoutHTMLImageElementHelpers;
use script::dom::htmlinputelement::LayoutHTMLInputElementHelpers;
use script::dom::node::{DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId};
use script::dom::node::{LayoutNodeHelpers, RawLayoutNodeHelpers, SharedLayoutData, TextNodeTypeId};
use script::dom::node::{LayoutNodeHelpers, RawLayoutNodeHelpers, SharedLayoutData};
use script::dom::node::{HasChanged, IsDirty, HasDirtySiblings, HasDirtyDescendants};
use script::dom::text::Text;
use script::layout_interface::LayoutChan;
Expand Down Expand Up @@ -101,22 +97,22 @@ pub trait TLayoutNode {
/// FIXME(pcwalton): Don't copy URLs.
fn image_url(&self) -> Option<Url> {
unsafe {
if !self.get().is_htmlimageelement() {
fail!("not an image!")
match HTMLImageElementCast::to_js(self.get_jsmanaged()) {
Some(elem) => elem.image().as_ref().map(|url| (*url).clone()),
None => fail!("not an image!")
}
let image_element: JS<HTMLImageElement> = self.get_jsmanaged().transmute_copy();
image_element.image().as_ref().map(|url| (*url).clone())
}
}

/// If this node is an iframe element, returns its pipeline and subpage IDs. If this node is
/// not an iframe element, fails.
fn iframe_pipeline_and_subpage_ids(&self) -> (PipelineId, SubpageId) {
unsafe {
if !self.get().is_htmliframeelement() {
fail!("not an iframe element!")
}
let iframe_element: JS<HTMLIFrameElement> = self.get_jsmanaged().transmute_copy();
let iframe_element: JS<HTMLIFrameElement> =
match HTMLIFrameElementCast::to_js(self.get_jsmanaged()) {
Some(elem) => elem,
None => fail!("not an iframe element!")
};
let size = (*iframe_element.unsafe_get()).size().unwrap();
(*size.pipeline_id(), *size.subpage_id())
}
Expand Down Expand Up @@ -188,14 +184,13 @@ impl<'ln> TLayoutNode for LayoutNode<'ln> {

fn text(&self) -> String {
unsafe {
if self.get().is_text() {
let text: JS<Text> = self.get_jsmanaged().transmute_copy();
(*text.unsafe_get()).characterdata().data_for_layout().to_string()
} else if self.get().is_htmlinputelement() {
let input: JS<HTMLInputElement> = self.get_jsmanaged().transmute_copy();
input.get_value_for_layout()
} else {
fail!("not text!")
let text_opt: Option<JS<Text>> = TextCast::to_js(self.get_jsmanaged());
match text_opt {
Some(text) => (*text.unsafe_get()).characterdata().data_for_layout().to_string(),
None => match HTMLInputElementCast::to_js(self.get_jsmanaged()) {
Some(input) => input.get_value_for_layout(),
None => fail!("not text!")
}
}
}
}
Expand Down Expand Up @@ -303,9 +298,13 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
#[inline]
fn as_element(self) -> LayoutElement<'ln> {
unsafe {
assert!(self.node.is_element_for_layout());
let elem: JS<Element> = self.node.transmute_copy();
let elem: JS<Element> = match ElementCast::to_js(&self.node) {
Some(elem) => elem,
None => fail!("not an element")
};

let element = &*elem.unsafe_get();

LayoutElement {
element: mem::transmute(element),
}
Expand Down Expand Up @@ -341,9 +340,9 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {

fn is_html_element_in_html_document(self) -> bool {
unsafe {
self.is_element() && {
let element: JS<Element> = self.node.transmute_copy();
element.html_element_in_html_document_for_layout()
match ElementCast::to_js(&self.node) {
Some(elem) => elem.html_element_in_html_document_for_layout(),
None => false
}
}
}
Expand Down Expand Up @@ -714,9 +713,10 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
#[inline]
pub fn as_element(&self) -> ThreadSafeLayoutElement<'ln> {
unsafe {
assert!(self.get_jsmanaged().is_element_for_layout());
let elem: JS<Element> = self.get_jsmanaged().transmute_copy();
let element = elem.unsafe_get();
let element = match ElementCast::to_js(self.get_jsmanaged()) {
Some(e) => e.unsafe_get(),
None => fail!("not an element")
};
// FIXME(pcwalton): Workaround until Rust gets multiple lifetime parameters on
// implementations.
ThreadSafeLayoutElement {
Expand Down Expand Up @@ -806,47 +806,44 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
}

pub fn is_ignorable_whitespace(&self) -> bool {
match self.type_id() {
Some(TextNodeTypeId) => {
unsafe {
let text: JS<Text> = self.get_jsmanaged().transmute_copy();
if !is_whitespace((*text.unsafe_get()).characterdata().data_for_layout()) {
return false
}
unsafe {
let text: JS<Text> = match TextCast::to_js(self.get_jsmanaged()) {
Some(text) => text,
None => return false
};

// NB: See the rules for `white-space` here:
//
// http://www.w3.org/TR/CSS21/text.html#propdef-white-space
//
// If you implement other values for this property, you will almost certainly
// want to update this check.
match self.style().get_inheritedtext().white_space {
white_space::normal => true,
_ => false,
}
}
if !is_whitespace((*text.unsafe_get()).characterdata().data_for_layout()) {
return false
}

// NB: See the rules for `white-space` here:
//
// http://www.w3.org/TR/CSS21/text.html#propdef-white-space
//
// If you implement other values for this property, you will almost certainly
// want to update this check.
match self.style().get_inheritedtext().white_space {
white_space::normal => true,
_ => false,
}
_ => false
}
}

pub fn get_input_value(&self) -> String {
unsafe {
if !self.get().is_htmlinputelement() {
fail!("not an input element!")
match HTMLInputElementCast::to_js(self.get_jsmanaged()) {
Some(input) => input.get_value_for_layout(),
None => fail!("not an input element!")
}
let input: JS<HTMLInputElement> = self.get_jsmanaged().transmute_copy();
input.get_value_for_layout()
}
}

pub fn get_input_size(&self) -> u32 {
unsafe {
if !self.get().is_htmlinputelement() {
fail!("not an input element!")
match HTMLInputElementCast::to_js(self.get_jsmanaged()) {
Some(input) => input.get_size_for_layout(),
None => fail!("not an input element!")
}
let input: JS<HTMLInputElement> = self.get_jsmanaged().transmute_copy();
input.get_size_for_layout()
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions components/script/dom/bindings/codegen/CodegenRust.py
View file
Expand Up @@ -5480,6 +5480,17 @@ def InheritTypes(config):
}
}
#[inline(always)]
#[allow(unrooted_must_root)]
fn to_js<T: ${toBound}+Reflectable>(base: &JS<T>) -> Option<JS<Self>> {
unsafe {
match (*base.unsafe_get()).${checkFn}() {
true => Some(base.transmute_copy()),
false => None
}
}
}
#[inline(always)]
fn from_ref<'a, T: ${fromBound}+Reflectable>(derived: JSRef<'a, T>) -> JSRef<'a, Self> {
unsafe { derived.transmute() }
Expand Down

5 comments on commit 6d91e92

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from jdm
at https://github.com/kaseyc/servo/commit/6d91e92c90caa61107ac3a4a6f11b92b3a363d18

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging kaseyc/servo/add_to_js_method = 6d91e92 into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kaseyc/servo/add_to_js_method = 6d91e92 merged ok, testing candidate = f1c0505

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all tests pass:
success: http://build.servo.org/builders/linux/builds/290
success: http://build.servo.org/builders/mac/builds/295

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = f1c0505

Please sign in to comment.