From 6d91e92c90caa61107ac3a4a6f11b92b3a363d18 Mon Sep 17 00:00:00 2001 From: Kasey Carrothers Date: Sat, 11 Oct 2014 16:02:59 -0700 Subject: [PATCH] Add a to_js method to the casting trait code in CodegenRust.py Replace the manual checks and calls to transmute_copy in /layout/wrapper.rs with calls to to_js Fixes #3616 --- components/layout/wrapper.rs | 119 +++++++++--------- .../dom/bindings/codegen/CodegenRust.py | 11 ++ 2 files changed, 69 insertions(+), 61 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 6f1d01dcbf2..3a9425c99c6 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -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. //! @@ -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; @@ -101,11 +97,10 @@ pub trait TLayoutNode { /// FIXME(pcwalton): Don't copy URLs. fn image_url(&self) -> Option { 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 = self.get_jsmanaged().transmute_copy(); - image_element.image().as_ref().map(|url| (*url).clone()) } } @@ -113,10 +108,11 @@ pub trait TLayoutNode { /// 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 = self.get_jsmanaged().transmute_copy(); + let iframe_element: JS = + 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()) } @@ -188,14 +184,13 @@ impl<'ln> TLayoutNode for LayoutNode<'ln> { fn text(&self) -> String { unsafe { - if self.get().is_text() { - let text: JS = self.get_jsmanaged().transmute_copy(); - (*text.unsafe_get()).characterdata().data_for_layout().to_string() - } else if self.get().is_htmlinputelement() { - let input: JS = self.get_jsmanaged().transmute_copy(); - input.get_value_for_layout() - } else { - fail!("not text!") + let text_opt: Option> = 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!") + } } } } @@ -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 = self.node.transmute_copy(); + let elem: JS = match ElementCast::to_js(&self.node) { + Some(elem) => elem, + None => fail!("not an element") + }; + let element = &*elem.unsafe_get(); + LayoutElement { element: mem::transmute(element), } @@ -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 = 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 } } } @@ -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 = 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 { @@ -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 = self.get_jsmanaged().transmute_copy(); - if !is_whitespace((*text.unsafe_get()).characterdata().data_for_layout()) { - return false - } + unsafe { + let text: JS = 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 = 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 = self.get_jsmanaged().transmute_copy(); - input.get_size_for_layout() } } } diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 2981cbf341a..bca334de6ea 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5480,6 +5480,17 @@ class GlobalGenRoots(): } } + #[inline(always)] + #[allow(unrooted_must_root)] + fn to_js(base: &JS) -> Option> { + 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() }