From 4c16729a01999978326e21f9b2e69de764938792 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 13:54:52 +0200 Subject: [PATCH 1/9] Make AttrHelpersForLayout methods be safe The unsafety isn't there, it's in the creation of the LayoutDom values. --- components/script/dom/attr.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index cea5df2abf5..b8633619bbb 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -234,26 +234,26 @@ impl Attr { #[allow(unsafe_code)] pub trait AttrHelpersForLayout<'dom> { - unsafe fn value(self) -> &'dom AttrValue; - unsafe fn value_ref_forever(self) -> &'dom str; - unsafe fn value_tokens(self) -> Option<&'dom [Atom]>; - unsafe fn local_name_atom(self) -> LocalName; + fn value(self) -> &'dom AttrValue; + fn value_ref_forever(self) -> &'dom str; + fn value_tokens(self) -> Option<&'dom [Atom]>; + fn local_name_atom(self) -> LocalName; } #[allow(unsafe_code)] impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { #[inline] - unsafe fn value(self) -> &'dom AttrValue { - (*self.unsafe_get()).value.borrow_for_layout() + fn value(self) -> &'dom AttrValue { + unsafe { self.unsafe_get().value.borrow_for_layout() } } #[inline] - unsafe fn value_ref_forever(self) -> &'dom str { + fn value_ref_forever(self) -> &'dom str { &**self.value() } #[inline] - unsafe fn value_tokens(self) -> Option<&'dom [Atom]> { + fn value_tokens(self) -> Option<&'dom [Atom]> { // This transmute is used to cheat the lifetime restriction. match *self.value() { AttrValue::TokenList(_, ref tokens) => Some(tokens), @@ -262,7 +262,7 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { } #[inline] - unsafe fn local_name_atom(self) -> LocalName { - (*self.unsafe_get()).identifier.local_name.clone() + fn local_name_atom(self) -> LocalName { + unsafe { self.unsafe_get().identifier.local_name.clone() } } } From 4db84bede8ec9330568c01fcf243a0ace0cef18b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 13:56:19 +0200 Subject: [PATCH 2/9] Rename AttrHelpersForLayout methods --- components/script/dom/attr.rs | 12 ++++++------ components/script/dom/element.rs | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index b8633619bbb..b641fa5d615 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -235,9 +235,9 @@ impl Attr { #[allow(unsafe_code)] pub trait AttrHelpersForLayout<'dom> { fn value(self) -> &'dom AttrValue; - fn value_ref_forever(self) -> &'dom str; - fn value_tokens(self) -> Option<&'dom [Atom]>; - fn local_name_atom(self) -> LocalName; + fn as_str(self) -> &'dom str; + fn as_tokens(self) -> Option<&'dom [Atom]>; + fn local_name(self) -> LocalName; } #[allow(unsafe_code)] @@ -248,12 +248,12 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { } #[inline] - fn value_ref_forever(self) -> &'dom str { + fn as_str(self) -> &'dom str { &**self.value() } #[inline] - fn value_tokens(self) -> Option<&'dom [Atom]> { + fn as_tokens(self) -> Option<&'dom [Atom]> { // This transmute is used to cheat the lifetime restriction. match *self.value() { AttrValue::TokenList(_, ref tokens) => Some(tokens), @@ -262,7 +262,7 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { } #[inline] - fn local_name_atom(self) -> LocalName { + fn local_name(self) -> LocalName { unsafe { self.unsafe_get().identifier.local_name.clone() } } } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 602cb148d7a..35f952a676c 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -578,7 +578,7 @@ pub unsafe fn get_attr_for_layout<'dom>( .iter() .find(|attr| { let attr = attr.to_layout(); - *name == attr.local_name_atom() && (*attr.unsafe_get()).namespace() == namespace + *name == attr.local_name() && (*attr.unsafe_get()).namespace() == namespace }) .map(|attr| attr.to_layout()) } @@ -600,7 +600,7 @@ impl RawLayoutElementHelpers for Element { namespace: &Namespace, name: &LocalName, ) -> Option<&'a str> { - get_attr_for_layout(self, namespace, name).map(|attr| attr.value_ref_forever()) + get_attr_for_layout(self, namespace, name).map(|attr| attr.as_str()) } #[inline] @@ -610,7 +610,7 @@ impl RawLayoutElementHelpers for Element { .iter() .filter_map(|attr| { let attr = attr.to_layout(); - if *name == attr.local_name_atom() { + if *name == attr.local_name() { Some(attr.value()) } else { None @@ -656,7 +656,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class")).map_or( false, |attr| { - attr.value_tokens() + attr.as_tokens() .unwrap() .iter() .any(|atom| case_sensitivity.eq_atom(atom, name)) @@ -668,7 +668,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> { #[inline] unsafe fn get_classes_for_layout(self) -> Option<&'dom [Atom]> { get_attr_for_layout(&*self.unsafe_get(), &ns!(), &local_name!("class")) - .map(|attr| attr.value_tokens().unwrap()) + .map(|attr| attr.as_tokens().unwrap()) } #[allow(unsafe_code)] From 47c9f1912150b41695810f6c987f0ca208547bd4 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:02:14 +0200 Subject: [PATCH 3/9] Don't clone the result of AttrHelpersForLayout::local_name There is no need to do that. Embrace returning borrows from the DOM, it good. --- components/script/dom/attr.rs | 6 +++--- components/script/dom/element.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index b641fa5d615..a9f832b97bb 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -237,7 +237,7 @@ pub trait AttrHelpersForLayout<'dom> { fn value(self) -> &'dom AttrValue; fn as_str(self) -> &'dom str; fn as_tokens(self) -> Option<&'dom [Atom]>; - fn local_name(self) -> LocalName; + fn local_name(self) -> &'dom LocalName; } #[allow(unsafe_code)] @@ -262,7 +262,7 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { } #[inline] - fn local_name(self) -> LocalName { - unsafe { self.unsafe_get().identifier.local_name.clone() } + fn local_name(self) -> &'dom LocalName { + unsafe { &self.unsafe_get().identifier.local_name } } } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 35f952a676c..6875d302e67 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -578,7 +578,7 @@ pub unsafe fn get_attr_for_layout<'dom>( .iter() .find(|attr| { let attr = attr.to_layout(); - *name == attr.local_name() && (*attr.unsafe_get()).namespace() == namespace + name == attr.local_name() && (*attr.unsafe_get()).namespace() == namespace }) .map(|attr| attr.to_layout()) } @@ -610,7 +610,7 @@ impl RawLayoutElementHelpers for Element { .iter() .filter_map(|attr| { let attr = attr.to_layout(); - if *name == attr.local_name() { + if name == attr.local_name() { Some(attr.value()) } else { None From 3e875ce3eb5270d31a13fbe0bd9796687465fcff Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:06:22 +0200 Subject: [PATCH 4/9] Introduce AttrHelpersForLayout::namespace --- components/script/dom/attr.rs | 6 ++++++ components/script/dom/element.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index a9f832b97bb..cd1a98ad799 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -238,6 +238,7 @@ pub trait AttrHelpersForLayout<'dom> { fn as_str(self) -> &'dom str; fn as_tokens(self) -> Option<&'dom [Atom]>; fn local_name(self) -> &'dom LocalName; + fn namespace(self) -> &'dom Namespace; } #[allow(unsafe_code)] @@ -265,4 +266,9 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { fn local_name(self) -> &'dom LocalName { unsafe { &self.unsafe_get().identifier.local_name } } + + #[inline] + fn namespace(self) -> &'dom Namespace { + unsafe { &self.unsafe_get().identifier.namespace } + } } diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 6875d302e67..7d167bf2574 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -578,7 +578,7 @@ pub unsafe fn get_attr_for_layout<'dom>( .iter() .find(|attr| { let attr = attr.to_layout(); - name == attr.local_name() && (*attr.unsafe_get()).namespace() == namespace + name == attr.local_name() && namespace == attr.namespace() }) .map(|attr| attr.to_layout()) } From e1e913d33c13a50ceed5a7ea19439b1b3d4d8ced Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:30:18 +0200 Subject: [PATCH 5/9] Make LayoutHTMLInputElementHelpers::value_for_layout return a cow --- components/script/dom/htmlinputelement.rs | 55 ++++++++++++----------- components/script/dom/node.rs | 2 +- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/components/script/dom/htmlinputelement.rs b/components/script/dom/htmlinputelement.rs index 05eca7b35eb..5d7438ecc4a 100755 --- a/components/script/dom/htmlinputelement.rs +++ b/components/script/dom/htmlinputelement.rs @@ -67,7 +67,7 @@ use profile_traits::ipc; use script_layout_interface::rpc::TextIndexResponse; use script_traits::ScriptToConstellationChan; use servo_atoms::Atom; -use std::borrow::ToOwned; +use std::borrow::Cow; use std::cell::Cell; use std::ops::Range; use std::ptr::NonNull; @@ -705,9 +705,8 @@ impl HTMLInputElement { } } -pub trait LayoutHTMLInputElementHelpers { - #[allow(unsafe_code)] - unsafe fn value_for_layout(self) -> String; +pub trait LayoutHTMLInputElementHelpers<'dom> { + fn value_for_layout(self) -> Cow<'dom, str>; #[allow(unsafe_code)] unsafe fn size_for_layout(self) -> u32; #[allow(unsafe_code)] @@ -726,41 +725,47 @@ unsafe fn get_raw_textinput_value(input: LayoutDom) -> DOMStri .get_content() } -impl LayoutHTMLInputElementHelpers for LayoutDom<'_, HTMLInputElement> { +impl<'dom> LayoutHTMLInputElementHelpers<'dom> for LayoutDom<'dom, HTMLInputElement> { #[allow(unsafe_code)] - unsafe fn value_for_layout(self) -> String { - #[allow(unsafe_code)] - unsafe fn get_raw_attr_value( - input: LayoutDom<'_, HTMLInputElement>, - default: &str, - ) -> String { - let elem = input.upcast::(); - let value = (*elem.unsafe_get()) - .get_attr_val_for_layout(&ns!(), &local_name!("value")) - .unwrap_or(default); - String::from(value) + fn value_for_layout(self) -> Cow<'dom, str> { + fn get_raw_attr_value<'dom>( + input: LayoutDom<'dom, HTMLInputElement>, + default: &'static str, + ) -> Cow<'dom, str> { + unsafe { + input + .upcast::() + .unsafe_get() + .get_attr_val_for_layout(&ns!(), &local_name!("value")) + .unwrap_or(default) + .into() + } } - match (*self.unsafe_get()).input_type() { - InputType::Checkbox | InputType::Radio => String::new(), - InputType::File | InputType::Image => String::new(), + let placeholder = unsafe { &**self.unsafe_get().placeholder.borrow_for_layout() }; + match unsafe { self.unsafe_get().input_type() } { + InputType::Checkbox | InputType::Radio => "".into(), + InputType::File | InputType::Image => "".into(), InputType::Button => get_raw_attr_value(self, ""), InputType::Submit => get_raw_attr_value(self, DEFAULT_SUBMIT_VALUE), InputType::Reset => get_raw_attr_value(self, DEFAULT_RESET_VALUE), InputType::Password => { - let text = get_raw_textinput_value(self); + let text = unsafe { get_raw_textinput_value(self) }; if !text.is_empty() { - text.chars().map(|_| PASSWORD_REPLACEMENT_CHAR).collect() + text.chars() + .map(|_| PASSWORD_REPLACEMENT_CHAR) + .collect::() + .into() } else { - String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) + placeholder.into() } }, _ => { - let text = get_raw_textinput_value(self); + let text = unsafe { get_raw_textinput_value(self) }; if !text.is_empty() { - String::from(text) + text.into() } else { - String::from((*self.unsafe_get()).placeholder.borrow_for_layout().clone()) + placeholder.into() } }, } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index d92e3b6a364..b72dc6b695e 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1463,7 +1463,7 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { } if let Some(input) = self.downcast::() { - return unsafe { input.value_for_layout() }; + return input.value_for_layout().into_owned(); } if let Some(area) = self.downcast::() { From 00c5ec202ccde20eace5c867578262efa5b69844 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:35:37 +0200 Subject: [PATCH 6/9] Make LayoutHTMLTextAreaElementHelpers::value_for_layout safe --- components/script/dom/htmltextareaelement.rs | 25 ++++++++++---------- components/script/dom/node.rs | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/components/script/dom/htmltextareaelement.rs b/components/script/dom/htmltextareaelement.rs index 8ee42bc2c86..e3721a78213 100755 --- a/components/script/dom/htmltextareaelement.rs +++ b/components/script/dom/htmltextareaelement.rs @@ -56,8 +56,7 @@ pub struct HTMLTextAreaElement { } pub trait LayoutHTMLTextAreaElementHelpers { - #[allow(unsafe_code)] - unsafe fn value_for_layout(self) -> String; + fn value_for_layout(self) -> String; #[allow(unsafe_code)] unsafe fn selection_for_layout(self) -> Option>; #[allow(unsafe_code)] @@ -67,19 +66,19 @@ pub trait LayoutHTMLTextAreaElementHelpers { } impl LayoutHTMLTextAreaElementHelpers for LayoutDom<'_, HTMLTextAreaElement> { - #[allow(unrooted_must_root)] #[allow(unsafe_code)] - unsafe fn value_for_layout(self) -> String { - let text = (*self.unsafe_get()) - .textinput - .borrow_for_layout() - .get_content(); - if text.is_empty() { - (*self.unsafe_get()) - .placeholder + fn value_for_layout(self) -> String { + let text = unsafe { + self.unsafe_get() + .textinput .borrow_for_layout() - .replace("\r\n", "\n") - .replace("\r", "\n") + .get_content() + }; + if text.is_empty() { + let placeholder = unsafe { self.unsafe_get().placeholder.borrow_for_layout() }; + // FIXME(nox): Would be cool to not allocate a new string if the + // placeholder is single line, but that's an unimportant detail. + placeholder.replace("\r\n", "\n").replace("\r", "\n").into() } else { text.into() } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index b72dc6b695e..425e29d45f3 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1467,7 +1467,7 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { } if let Some(area) = self.downcast::() { - return unsafe { area.value_for_layout() }; + return area.value_for_layout(); } panic!("not text!") From 409bd3d989c1877f8e0a5da92bd758dd20ac724b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:40:47 +0200 Subject: [PATCH 7/9] Make LayoutCharacterDataHelpers::data_for_layout be safe --- components/layout_thread/dom_wrapper.rs | 2 +- components/layout_thread_2020/dom_wrapper.rs | 2 +- components/script/dom/characterdata.rs | 9 ++++----- components/script/dom/node.rs | 3 +-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index f2ba0a6fafa..a0eb66a7fd8 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -828,7 +828,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { .dom_children() .all(|node| match node.script_type_id() { NodeTypeId::Element(..) => false, - NodeTypeId::CharacterData(CharacterDataTypeId::Text(TextTypeId::Text)) => unsafe { + NodeTypeId::CharacterData(CharacterDataTypeId::Text(TextTypeId::Text)) => { node.node.downcast().unwrap().data_for_layout().is_empty() }, _ => true, diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index d8aa7b05360..c79aa891214 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -835,7 +835,7 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { .dom_children() .all(|node| match node.script_type_id() { NodeTypeId::Element(..) => false, - NodeTypeId::CharacterData(CharacterDataTypeId::Text(TextTypeId::Text)) => unsafe { + NodeTypeId::CharacterData(CharacterDataTypeId::Text(TextTypeId::Text)) => { node.node.downcast().unwrap().data_for_layout().is_empty() }, _ => true, diff --git a/components/script/dom/characterdata.rs b/components/script/dom/characterdata.rs index 4e0d32d4456..f8c5acb5a8f 100644 --- a/components/script/dom/characterdata.rs +++ b/components/script/dom/characterdata.rs @@ -280,16 +280,15 @@ impl CharacterDataMethods for CharacterData { } } -#[allow(unsafe_code)] pub trait LayoutCharacterDataHelpers<'dom> { - unsafe fn data_for_layout(self) -> &'dom str; + fn data_for_layout(self) -> &'dom str; } -#[allow(unsafe_code)] impl<'dom> LayoutCharacterDataHelpers<'dom> for LayoutDom<'dom, CharacterData> { + #[allow(unsafe_code)] #[inline] - unsafe fn data_for_layout(self) -> &'dom str { - &(*self.unsafe_get()).data.borrow_for_layout() + fn data_for_layout(self) -> &'dom str { + unsafe { self.unsafe_get().data.borrow_for_layout() } } } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 425e29d45f3..07b7d60cbe6 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -1456,10 +1456,9 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { val } - #[allow(unsafe_code)] fn text_content(self) -> String { if let Some(text) = self.downcast::() { - return unsafe { text.upcast().data_for_layout().to_owned() }; + return text.upcast().data_for_layout().to_owned(); } if let Some(input) = self.downcast::() { From 6fe294fa5bab1b0ccd80d6dfb53be85adfaec4ad Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 14:58:56 +0200 Subject: [PATCH 8/9] Make LayoutNodeHelpers::text_content return a cow --- components/layout/wrapper.rs | 2 +- components/layout_2020/dom_traversal.rs | 14 ++++++++++---- components/layout_2020/flow/construct.rs | 8 +++++++- components/layout_thread/dom_wrapper.rs | 6 +++--- components/layout_thread_2020/dom_wrapper.rs | 6 +++--- components/script/dom/node.rs | 12 ++++++------ .../script_layout_interface/wrapper_traits.rs | 3 ++- 7 files changed, 32 insertions(+), 19 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index af8d6b6a0c7..60eace1f16a 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -130,7 +130,7 @@ where }); } - TextContent::Text(self.node_text_content().into_boxed_str()) + TextContent::Text(self.node_text_content().into_owned().into_boxed_str()) } fn restyle_damage(self) -> RestyleDamage { diff --git a/components/layout_2020/dom_traversal.rs b/components/layout_2020/dom_traversal.rs index 020282f50ff..ef4570a85bb 100644 --- a/components/layout_2020/dom_traversal.rs +++ b/components/layout_2020/dom_traversal.rs @@ -17,6 +17,7 @@ use script_layout_interface::wrapper_traits::{ }; use script_layout_interface::HTMLCanvasDataSource; use servo_arc::Arc as ServoArc; +use std::borrow::Cow; use std::marker::PhantomData as marker; use std::sync::{Arc, Mutex}; use style::dom::{OpaqueNode, TNode}; @@ -59,7 +60,12 @@ pub(super) trait TraversalHandler<'dom, Node> where Node: 'dom, { - fn handle_text(&mut self, node: Node, text: String, parent_style: &ServoArc); + fn handle_text( + &mut self, + node: Node, + text: Cow<'dom, str>, + parent_style: &ServoArc, + ); /// Or pseudo-element fn handle_element( @@ -166,7 +172,7 @@ fn traverse_pseudo_element_contents<'dom, Node>( for item in items { match item { PseudoElementContentItem::Text(text) => { - handler.handle_text(node, text, pseudo_element_style) + handler.handle_text(node, text.into(), pseudo_element_style) }, PseudoElementContentItem::Replaced(contents) => { let item_style = anonymous_style.get_or_insert_with(|| { @@ -351,7 +357,7 @@ impl Drop for BoxSlot<'_> { pub(crate) trait NodeExt<'dom>: 'dom + Copy + LayoutNode<'dom> + Send + Sync { fn is_element(self) -> bool; - fn as_text(self) -> Option; + fn as_text(self) -> Option>; /// Returns the image if it’s loaded, and its size in image pixels /// adjusted for `image_density`. @@ -378,7 +384,7 @@ where self.to_threadsafe().as_element().is_some() } - fn as_text(self) -> Option { + fn as_text(self) -> Option> { if self.is_text_node() { Some(self.to_threadsafe().node_text_content()) } else { diff --git a/components/layout_2020/flow/construct.rs b/components/layout_2020/flow/construct.rs index fe9465f775e..186a4b4d4ff 100644 --- a/components/layout_2020/flow/construct.rs +++ b/components/layout_2020/flow/construct.rs @@ -16,6 +16,7 @@ use crate::style_ext::{ComputedValuesExt, DisplayGeneratingBox, DisplayInside, D use rayon::iter::{IntoParallelIterator, ParallelIterator}; use rayon_croissant::ParallelIteratorExt; use servo_arc::Arc; +use std::borrow::Cow; use std::convert::{TryFrom, TryInto}; use style::properties::ComputedValues; use style::selector_parser::PseudoElement; @@ -286,7 +287,12 @@ where } } - fn handle_text(&mut self, node: Node, input: String, parent_style: &Arc) { + fn handle_text( + &mut self, + node: Node, + input: Cow<'dom, str>, + parent_style: &Arc, + ) { let (leading_whitespace, mut input) = self.handle_leading_whitespace(&input); if leading_whitespace || !input.is_empty() { // This text node should be pushed either to the next ongoing diff --git a/components/layout_thread/dom_wrapper.rs b/components/layout_thread/dom_wrapper.rs index a0eb66a7fd8..e3197c8e78a 100644 --- a/components/layout_thread/dom_wrapper.rs +++ b/components/layout_thread/dom_wrapper.rs @@ -67,6 +67,7 @@ use selectors::sink::Push; use servo_arc::{Arc, ArcBorrow}; use servo_atoms::Atom; use servo_url::ServoUrl; +use std::borrow::Cow; use std::fmt; use std::fmt::Debug; use std::hash::{Hash, Hasher}; @@ -1110,9 +1111,8 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { self.node } - fn node_text_content(&self) -> String { - let this = unsafe { self.get_jsmanaged() }; - return this.text_content(); + fn node_text_content(self) -> Cow<'ln, str> { + unsafe { self.get_jsmanaged().text_content() } } fn selection(&self) -> Option> { diff --git a/components/layout_thread_2020/dom_wrapper.rs b/components/layout_thread_2020/dom_wrapper.rs index c79aa891214..7d57d7f511e 100644 --- a/components/layout_thread_2020/dom_wrapper.rs +++ b/components/layout_thread_2020/dom_wrapper.rs @@ -67,6 +67,7 @@ use selectors::sink::Push; use servo_arc::{Arc, ArcBorrow}; use servo_atoms::Atom; use servo_url::ServoUrl; +use std::borrow::Cow; use std::fmt; use std::fmt::Debug; use std::hash::{Hash, Hasher}; @@ -1117,9 +1118,8 @@ impl<'ln> ThreadSafeLayoutNode<'ln> for ServoThreadSafeLayoutNode<'ln> { self.node } - fn node_text_content(&self) -> String { - let this = unsafe { self.get_jsmanaged() }; - return this.text_content(); + fn node_text_content(self) -> Cow<'ln, str> { + unsafe { self.get_jsmanaged().text_content() } } fn selection(&self) -> Option> { diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index 07b7d60cbe6..251f6fcab18 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -87,7 +87,7 @@ use servo_arc::Arc; use servo_atoms::Atom; use servo_url::ServoUrl; use smallvec::SmallVec; -use std::borrow::ToOwned; +use std::borrow::Cow; use std::cell::{Cell, UnsafeCell}; use std::cmp; use std::default::Default; @@ -1326,7 +1326,7 @@ pub trait LayoutNodeHelpers<'dom> { unsafe fn init_style_and_layout_data(self, _: OpaqueStyleAndLayoutData); unsafe fn take_style_and_layout_data(self) -> OpaqueStyleAndLayoutData; - fn text_content(self) -> String; + fn text_content(self) -> Cow<'dom, str>; fn selection(self) -> Option>; fn image_url(self) -> Option; fn image_density(self) -> Option; @@ -1456,17 +1456,17 @@ impl<'dom> LayoutNodeHelpers<'dom> for LayoutDom<'dom, Node> { val } - fn text_content(self) -> String { + fn text_content(self) -> Cow<'dom, str> { if let Some(text) = self.downcast::() { - return text.upcast().data_for_layout().to_owned(); + return text.upcast().data_for_layout().into(); } if let Some(input) = self.downcast::() { - return input.value_for_layout().into_owned(); + return input.value_for_layout(); } if let Some(area) = self.downcast::() { - return area.value_for_layout(); + return area.value_for_layout().into(); } panic!("not text!") diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index 67b107a86d7..462d11f2c7e 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -17,6 +17,7 @@ use net_traits::image::base::{Image, ImageMetadata}; use range::Range; use servo_arc::Arc; use servo_url::ServoUrl; +use std::borrow::Cow; use std::fmt::Debug; use std::sync::Arc as StdArc; use style::attr::AttrValue; @@ -262,7 +263,7 @@ pub trait ThreadSafeLayoutNode<'dom>: /// data flags, and we have this annoying trait separation between script and layout :-( unsafe fn unsafe_get(self) -> Self::ConcreteNode; - fn node_text_content(&self) -> String; + fn node_text_content(self) -> Cow<'dom, str>; /// If the insertion point is within this node, returns it. Otherwise, returns `None`. fn selection(&self) -> Option>; From fb1ff3f097c0c8a994c9e7c4bb72c3ba5b64492d Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Tue, 31 Mar 2020 15:23:06 +0200 Subject: [PATCH 9/9] Remove an obsolete comment --- components/script/dom/attr.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/script/dom/attr.rs b/components/script/dom/attr.rs index cd1a98ad799..e2e8b79f9af 100644 --- a/components/script/dom/attr.rs +++ b/components/script/dom/attr.rs @@ -255,7 +255,6 @@ impl<'dom> AttrHelpersForLayout<'dom> for LayoutDom<'dom, Attr> { #[inline] fn as_tokens(self) -> Option<&'dom [Atom]> { - // This transmute is used to cheat the lifetime restriction. match *self.value() { AttrValue::TokenList(_, ref tokens) => Some(tokens), _ => None,