Auto merge of #5611 - nox:cleanup-characterdata, r=Ms2ger

components/layout/wrapper.rs

@@ -41,11 +41,12 @@ use opaque_node::OpaqueNodeMethods;
 
 use cssparser::RGBA;
 use gfx::display_list::OpaqueNode;
-use script::dom::bindings::codegen::InheritTypes::{ElementCast, HTMLIFrameElementCast};
+use script::dom::bindings::codegen::InheritTypes::{CharacterDataCast, ElementCast};
-use script::dom::bindings::codegen::InheritTypes::{HTMLCanvasElementCast, HTMLImageElementCast};
+use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementCast, HTMLCanvasElementCast};
-use script::dom::bindings::codegen::InheritTypes::{HTMLInputElementCast, HTMLTextAreaElementCast};
+use script::dom::bindings::codegen::InheritTypes::{HTMLImageElementCast, HTMLInputElementCast};
-use script::dom::bindings::codegen::InheritTypes::{NodeCast, TextCast};
+use script::dom::bindings::codegen::InheritTypes::{HTMLTextAreaElementCast, NodeCast, TextCast};
 use script::dom::bindings::js::LayoutJS;
+use script::dom::characterdata::LayoutCharacterDataHelpers;
 use script::dom::element::{Element, ElementTypeId};
 use script::dom::element::{LayoutElementHelpers, RawLayoutElementHelpers};
 use script::dom::htmlelement::HTMLElementTypeId;
@@ -222,9 +223,8 @@ impl<'ln> TLayoutNode for LayoutNode<'ln> {
             let text: Option<LayoutJS<Text>> = TextCast::to_layout_js(self.get_jsmanaged());
             if let Some(text) = text {
                 return vec![
-                    ContentItem::String((*text.unsafe_get()).characterdata()
+                    ContentItem::String(
-                                                            .data_for_layout()
+                        CharacterDataCast::from_layout_js(&text).data_for_layout().to_owned())
-                                                            .to_owned())
                 ];
             }
             let input: Option<LayoutJS<HTMLInputElement>> =
@@ -961,7 +961,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
                 None => return false
             };
 
-            if !is_whitespace((*text.unsafe_get()).characterdata().data_for_layout()) {
+            if !is_whitespace(CharacterDataCast::from_layout_js(&text).data_for_layout()) {
                 return false
             }
 

components/script/dom/bindings/codegen/CodegenRust.py

@@ -5427,6 +5427,12 @@ impl ${name}Cast {
         unsafe { derived.transmute_borrowed() }
     }
 
+    #[inline(always)]
+    #[allow(unrooted_must_root)]
+    pub fn from_layout_js<T: ${fromBound}+Reflectable>(derived: &LayoutJS<T>) -> LayoutJS<${name}> {
+        unsafe { derived.transmute_copy() }
+    }
+
     #[inline(always)]
     pub fn from_temporary<T: ${fromBound}+Reflectable>(derived: Temporary<T>) -> Temporary<${name}> {
         unsafe { derived.transmute() }

components/script/dom/characterdata.rs

@@ -10,7 +10,7 @@ use dom::bindings::codegen::InheritTypes::{CharacterDataDerived, ElementCast};
 use dom::bindings::codegen::InheritTypes::NodeCast;
 use dom::bindings::error::{Fallible, ErrorResult};
 use dom::bindings::error::Error::IndexSize;
-use dom::bindings::js::{JSRef, Temporary};
+use dom::bindings::js::{JSRef, LayoutJS, Temporary};
 use dom::document::Document;
 use dom::element::Element;
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
@@ -20,6 +20,7 @@ use util::str::DOMString;
 
 use std::borrow::ToOwned;
 use std::cell::Ref;
+use std::cmp;
 
 #[dom_struct]
 pub struct CharacterData {
@@ -45,67 +46,59 @@ impl CharacterData {
             data: DOMRefCell::new(data),
         }
     }
-
-    #[inline]
-    pub fn node<'a>(&'a self) -> &'a Node {
-        &self.node
-    }
-
-    #[inline]
-    pub fn data(&self) -> Ref<DOMString> {
-        self.data.borrow()
-    }
-
-    #[inline]
-    pub fn set_data(&self, data: DOMString) {
-        *self.data.borrow_mut() = data;
-    }
-
-    #[inline]
-    #[allow(unsafe_code)]
-    pub unsafe fn data_for_layout<'a>(&'a self) -> &'a str {
-        self.data.borrow_for_layout().as_slice()
-    }
-
 }
 
 impl<'a> CharacterDataMethods for JSRef<'a, CharacterData> {
+    // https://dom.spec.whatwg.org/#dom-characterdata-data
     fn Data(self) -> DOMString {
         // FIXME(https://github.com/rust-lang/rust/issues/23338)
         let data = self.data.borrow();
         data.clone()
     }
 
-    fn SetData(self, arg: DOMString) -> ErrorResult {
+    // https://dom.spec.whatwg.org/#dom-characterdata-data
-        *self.data.borrow_mut() = arg;
+    fn SetData(self, data: DOMString) {
-        Ok(())
+        *self.data.borrow_mut() = data;
     }
 
+    // https://dom.spec.whatwg.org/#dom-characterdata-length
     fn Length(self) -> u32 {
         // FIXME(https://github.com/rust-lang/rust/issues/23338)
         let data = self.data.borrow();
         data.chars().count() as u32
     }
 
+    // https://dom.spec.whatwg.org/#dom-characterdata-substringdata
     fn SubstringData(self, offset: u32, count: u32) -> Fallible<DOMString> {
-        // FIXME(https://github.com/rust-lang/rust/issues/23338)
         let data = self.data.borrow();
-        Ok(data.slice_chars(offset as usize, (offset + count) as usize).to_owned())
+        // Step 1.
+        let len = data.chars().count();
+        if len > offset as usize {
+            // Step 2.
+            return Err(IndexSize);
+        }
+        // Step 3.
+        let end = cmp::min((offset + count) as usize, len);
+        // Step 4.
+        Ok(data.slice_chars(offset as usize, end).to_owned())
     }
 
-    fn AppendData(self, arg: DOMString) -> ErrorResult {
+    // https://dom.spec.whatwg.org/#dom-characterdata-appenddata
-        self.data.borrow_mut().push_str(arg.as_slice());
+    fn AppendData(self, data: DOMString) {
-        Ok(())
+        self.data.borrow_mut().push_str(&data);
     }
 
+    // https://dom.spec.whatwg.org/#dom-characterdata-insertdata
     fn InsertData(self, offset: u32, arg: DOMString) -> ErrorResult {
         self.ReplaceData(offset, 0, arg)
     }
 
+    // https://dom.spec.whatwg.org/#dom-characterdata-deletedata
     fn DeleteData(self, offset: u32, count: u32) -> ErrorResult {
         self.ReplaceData(offset, count, "".to_owned())
     }
 
+    // https://dom.spec.whatwg.org/#dom-characterdata-replacedata
     fn ReplaceData(self, offset: u32, count: u32, arg: DOMString) -> ErrorResult {
         let length = self.data.borrow().chars().count() as u32;
         if offset > length {
@@ -143,3 +136,26 @@ impl<'a> CharacterDataMethods for JSRef<'a, CharacterData> {
     }
 }
 
+pub trait CharacterDataHelpers<'a> {
+    fn data(self) -> Ref<'a, DOMString>;
+}
+
+impl<'a> CharacterDataHelpers<'a> for JSRef<'a, CharacterData> {
+    #[inline]
+    fn data(self) -> Ref<'a, DOMString> {
+        self.extended_deref().data.borrow()
+    }
+}
+
+#[allow(unsafe_code)]
+pub trait LayoutCharacterDataHelpers {
+    unsafe fn data_for_layout<'a>(&'a self) -> &'a str;
+}
+
+#[allow(unsafe_code)]
+impl LayoutCharacterDataHelpers for LayoutJS<CharacterData> {
+    #[inline]
+    unsafe fn data_for_layout<'a>(&'a self) -> &'a str {
+        &(*self.unsafe_get()).data.borrow_for_layout()
+    }
+}

components/script/dom/comment.rs

@@ -42,10 +42,5 @@ impl Comment {
         let document = global.as_window().Document().root();
         Ok(Comment::new(data, document.r()))
     }
-
-    #[inline]
-    pub fn characterdata<'a>(&'a self) -> &'a CharacterData {
-        &self.characterdata
-    }
 }
 

components/script/dom/document.rs

@@ -18,7 +18,7 @@ use dom::bindings::codegen::InheritTypes::{EventTargetCast, HTMLAnchorElementCas
 use dom::bindings::codegen::InheritTypes::{HTMLAnchorElementDerived, HTMLAppletElementDerived};
 use dom::bindings::codegen::InheritTypes::{HTMLAreaElementDerived, HTMLEmbedElementDerived};
 use dom::bindings::codegen::InheritTypes::{HTMLFormElementDerived, HTMLImageElementDerived};
-use dom::bindings::codegen::InheritTypes::{HTMLScriptElementDerived};
+use dom::bindings::codegen::InheritTypes::{HTMLScriptElementDerived, CharacterDataCast};
 use dom::bindings::error::{ErrorResult, Fallible};
 use dom::bindings::error::Error::{NotSupported, InvalidCharacter, Security};
 use dom::bindings::error::Error::HierarchyRequest;
@@ -29,6 +29,7 @@ use dom::bindings::refcounted::Trusted;
 use dom::bindings::utils::reflect_dom_object;
 use dom::bindings::utils::{xml_name_type, validate_and_extract};
 use dom::bindings::utils::XMLName::InvalidXMLName;
+use dom::characterdata::CharacterDataHelpers;
 use dom::comment::Comment;
 use dom::customevent::CustomEvent;
 use dom::documentfragment::DocumentFragment;
@@ -1125,7 +1126,7 @@ impl<'a> DocumentMethods for JSRef<'a, Document> {
             for child in title_element.children() {
                 let child = child.root();
                 if let Some(text) = TextCast::to_ref(child.r()) {
-                    title.push_str(&text.characterdata().data());
+                    title.push_str(&CharacterDataCast::from_ref(text).data());
                 }
             }
         }

components/script/dom/htmltitleelement.rs

@@ -6,8 +6,9 @@ use dom::bindings::codegen::Bindings::HTMLTitleElementBinding;
 use dom::bindings::codegen::Bindings::HTMLTitleElementBinding::HTMLTitleElementMethods;
 use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::codegen::InheritTypes::{HTMLElementCast, HTMLTitleElementDerived, NodeCast};
-use dom::bindings::codegen::InheritTypes::{TextCast};
+use dom::bindings::codegen::InheritTypes::{CharacterDataCast, TextCast};
 use dom::bindings::js::{JSRef, Temporary};
+use dom::characterdata::CharacterDataHelpers;
 use dom::document::{Document, DocumentHelpers};
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::element::ElementTypeId;
@@ -51,7 +52,7 @@ impl<'a> HTMLTitleElementMethods for JSRef<'a, HTMLTitleElement> {
             let child = child.root();
             let text: Option<JSRef<Text>> = TextCast::to_ref(child.r());
             match text {
-                Some(text) => content.push_str(text.characterdata().data().as_slice()),
+                Some(text) => content.push_str(&CharacterDataCast::from_ref(text).data()),
                 None => (),
             }
         }

components/script/dom/node.rs

@@ -13,12 +13,11 @@ use dom::bindings::codegen::Bindings::NamedNodeMapBinding::NamedNodeMapMethods;
 use dom::bindings::codegen::Bindings::NodeBinding::{NodeConstants, NodeMethods};
 use dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods;
 use dom::bindings::codegen::Bindings::ProcessingInstructionBinding::ProcessingInstructionMethods;
-use dom::bindings::codegen::InheritTypes::{CommentCast, DocumentCast, DocumentTypeCast};
+use dom::bindings::codegen::InheritTypes::{CharacterDataCast, DocumentCast, DocumentTypeCast};
-use dom::bindings::codegen::InheritTypes::{ElementCast, TextCast, NodeCast, ElementDerived};
+use dom::bindings::codegen::InheritTypes::{ElementCast, NodeCast, ElementDerived, EventTargetCast};
-use dom::bindings::codegen::InheritTypes::{CharacterDataCast, NodeBase, NodeDerived};
-use dom::bindings::codegen::InheritTypes::{ProcessingInstructionCast, EventTargetCast};
 use dom::bindings::codegen::InheritTypes::{HTMLLegendElementDerived, HTMLFieldSetElementDerived};
-use dom::bindings::codegen::InheritTypes::HTMLOptGroupElementDerived;
+use dom::bindings::codegen::InheritTypes::{HTMLOptGroupElementDerived, NodeBase, NodeDerived};
+use dom::bindings::codegen::InheritTypes::{ProcessingInstructionCast, TextCast};
 use dom::bindings::conversions;
 use dom::bindings::error::Fallible;
 use dom::bindings::error::Error::{NotFound, HierarchyRequest, Syntax};
@@ -29,7 +28,7 @@ use dom::bindings::js::{ResultRootable, OptionalRootable, MutNullableJS};
 use dom::bindings::trace::JSTraceable;
 use dom::bindings::trace::RootedVec;
 use dom::bindings::utils::{Reflectable, reflect_dom_object};
-use dom::characterdata::CharacterData;
+use dom::characterdata::{CharacterData, CharacterDataHelpers};
 use dom::comment::Comment;
 use dom::document::{Document, DocumentHelpers, IsHTMLDocument, DocumentSource};
 use dom::documentfragment::DocumentFragment;
@@ -1601,8 +1600,8 @@ impl Node {
                 NodeCast::from_temporary(doc_fragment)
             },
             NodeTypeId::Comment => {
-                let comment: JSRef<Comment> = CommentCast::to_ref(node).unwrap();
+                let cdata = CharacterDataCast::to_ref(node).unwrap();
-                let comment = Comment::new(comment.characterdata().data().clone(), document.r());
+                let comment = Comment::new(cdata.Data(), document.r());
                 NodeCast::from_temporary(comment)
             },
             NodeTypeId::Document => {
@@ -1629,14 +1628,14 @@ impl Node {
                 NodeCast::from_temporary(element)
             },
             NodeTypeId::Text => {
-                let text: JSRef<Text> = TextCast::to_ref(node).unwrap();
+                let cdata = CharacterDataCast::to_ref(node).unwrap();
-                let text = Text::new(text.characterdata().data().clone(), document.r());
+                let text = Text::new(cdata.Data(), document.r());
                 NodeCast::from_temporary(text)
             },
             NodeTypeId::ProcessingInstruction => {
                 let pi: JSRef<ProcessingInstruction> = ProcessingInstructionCast::to_ref(node).unwrap();
                 let pi = ProcessingInstruction::new(pi.target().clone(),
-                                                    pi.characterdata().data().clone(), document.r());
+                                                    CharacterDataCast::from_ref(pi).Data(), document.r());
                 NodeCast::from_temporary(pi)
             },
         }.root();
@@ -1696,7 +1695,7 @@ impl Node {
             let node = node.root();
             let text = TextCast::to_ref(node.r());
             match text {
-                Some(text) => content.push_str(text.characterdata().data().as_slice()),
+                Some(text) => content.push_str(&CharacterDataCast::from_ref(text).Data()),
                 None => (),
             }
         }
@@ -1880,7 +1879,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
             NodeTypeId::Text |
             NodeTypeId::ProcessingInstruction => {
                 let characterdata: JSRef<CharacterData> = CharacterDataCast::to_ref(self).unwrap();
-                characterdata.set_data(value);
+                characterdata.SetData(value);
 
                 // Notify the document that the content of this node is different
                 let document = self.owner_doc().root();
@@ -2123,7 +2122,7 @@ impl<'a> NodeMethods for JSRef<'a, Node> {
             let pi: JSRef<ProcessingInstruction> = ProcessingInstructionCast::to_ref(node).unwrap();
             let other_pi: JSRef<ProcessingInstruction> = ProcessingInstructionCast::to_ref(other).unwrap();
             (*pi.target() == *other_pi.target()) &&
-            (*pi.characterdata().data() == *other_pi.characterdata().data())
+            (*CharacterDataCast::from_ref(pi).data() == *CharacterDataCast::from_ref(other_pi).data())
         }
         fn is_equal_characterdata(node: JSRef<Node>, other: JSRef<Node>) -> bool {
             let characterdata: JSRef<CharacterData> = CharacterDataCast::to_ref(node).unwrap();

components/script/dom/processinginstruction.rs

@@ -38,10 +38,6 @@ impl ProcessingInstruction {
                            document, ProcessingInstructionBinding::Wrap)
     }
 
-    pub fn characterdata<'a>(&'a self) -> &'a CharacterData {
-        &self.characterdata
-    }
-
     pub fn target<'a>(&'a self) -> &'a DOMString {
         &self.target
     }

components/script/dom/text.rs

@@ -42,10 +42,5 @@ impl Text {
         let document = global.as_window().Document().root();
         Ok(Text::new(text, document.r()))
     }
-
-    #[inline]
-    pub fn characterdata<'a>(&'a self) -> &'a CharacterData {
-        &self.characterdata
-    }
 }
 

components/script/dom/webidls/CharacterData.webidl

@@ -11,11 +11,10 @@
  */
 
 interface CharacterData : Node {
-  [TreatNullAs=EmptyString,SetterThrows] attribute DOMString data;
+  [TreatNullAs=EmptyString] attribute DOMString data;
   readonly attribute unsigned long length;
   [Throws]
   DOMString substringData(unsigned long offset, unsigned long count);
-  [Throws]
   void appendData(DOMString data);
   [Throws]
   void insertData(unsigned long offset, DOMString data);

components/script/parse/html.rs

@@ -7,13 +7,14 @@
 use dom::attr::AttrHelpers;
 use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
-use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast, HTMLScriptElementCast};
+use dom::bindings::codegen::InheritTypes::{CharacterDataCast, DocumentTypeCast};
-use dom::bindings::codegen::InheritTypes::{DocumentTypeCast, TextCast, CommentCast};
+use dom::bindings::codegen::InheritTypes::{ElementCast, HTMLScriptElementCast};
+use dom::bindings::codegen::InheritTypes::{HTMLFormElementDerived, NodeCast};
 use dom::bindings::codegen::InheritTypes::ProcessingInstructionCast;
-use dom::bindings::codegen::InheritTypes::HTMLFormElementDerived;
 use dom::bindings::js::{JS, JSRef, Temporary, OptionalRootable, Root};
 use dom::bindings::js::RootedReference;
 use dom::bindings::trace::RootedVec;
+use dom::characterdata::CharacterDataHelpers;
 use dom::comment::Comment;
 use dom::document::{Document, DocumentHelpers};
 use dom::document::{DocumentSource, IsHTMLDocument};
@@ -234,22 +235,19 @@ impl<'a> Serializable for JSRef<'a, Node> {
             },
 
             (IncludeNode, NodeTypeId::Text) => {
-                let text: JSRef<Text> = TextCast::to_ref(node).unwrap();
+                let cdata = CharacterDataCast::to_ref(node).unwrap();
-                let data = text.characterdata().data();
+                serializer.write_text(&cdata.data())
-                serializer.write_text(data.as_slice())
             },
 
             (IncludeNode, NodeTypeId::Comment) => {
-                let comment: JSRef<Comment> = CommentCast::to_ref(node).unwrap();
+                let cdata = CharacterDataCast::to_ref(node).unwrap();
-                let data = comment.characterdata().data();
+                serializer.write_comment(&cdata.data())
-                serializer.write_comment(data.as_slice())
             },
 
             (IncludeNode, NodeTypeId::ProcessingInstruction) => {
                 let pi: JSRef<ProcessingInstruction> = ProcessingInstructionCast::to_ref(node).unwrap();
-                let data = pi.characterdata().data();
+                let data = CharacterDataCast::from_ref(pi).data();
-                serializer.write_processing_instruction(pi.target().as_slice(),
+                serializer.write_processing_instruction(&pi.target(), &data)
-                                                        data.as_slice())
             },
 
             (IncludeNode, NodeTypeId::DocumentFragment) => Ok(()),