Use Rc instead of cloning the DOMString

Specifically, I changed the `text` field of `ScriptOrigin` from a
`DOMString` to an `Rc<DOMString>`. Then I updated all the related code
to work with an `Rc`.

This is just a first pass to get the code to compile. There are probably
more things I can do that will improve the code and further reduce
cloning.
This commit is contained in:
Camelid 2020-07-14 20:32:37 -07:00
parent 12d4c0d5eb
commit 69881e8b06
2 changed files with 21 additions and 19 deletions

View file

@ -49,6 +49,7 @@ use std::fs::{create_dir_all, read_to_string, File};
use std::io::{Read, Seek, Write}; use std::io::{Read, Seek, Write};
use std::path::PathBuf; use std::path::PathBuf;
use std::process::Command; use std::process::Command;
use std::rc::Rc;
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use style::str::{StaticStringVec, HTML_SPACE_CHARACTERS}; use style::str::{StaticStringVec, HTML_SPACE_CHARACTERS};
use uuid::Uuid; use uuid::Uuid;
@ -150,14 +151,15 @@ pub enum ScriptType {
#[derive(JSTraceable, MallocSizeOf)] #[derive(JSTraceable, MallocSizeOf)]
pub struct ScriptOrigin { pub struct ScriptOrigin {
text: DOMString, #[ignore_malloc_size_of = "Rc is hard"]
text: Rc<DOMString>,
url: ServoUrl, url: ServoUrl,
external: bool, external: bool,
type_: ScriptType, type_: ScriptType,
} }
impl ScriptOrigin { impl ScriptOrigin {
pub fn internal(text: DOMString, url: ServoUrl, type_: ScriptType) -> ScriptOrigin { pub fn internal(text: Rc<DOMString>, url: ServoUrl, type_: ScriptType) -> ScriptOrigin {
ScriptOrigin { ScriptOrigin {
text: text, text: text,
url: url, url: url,
@ -166,7 +168,7 @@ impl ScriptOrigin {
} }
} }
pub fn external(text: DOMString, url: ServoUrl, type_: ScriptType) -> ScriptOrigin { pub fn external(text: Rc<DOMString>, url: ServoUrl, type_: ScriptType) -> ScriptOrigin {
ScriptOrigin { ScriptOrigin {
text: text, text: text,
url: url, url: url,
@ -175,8 +177,8 @@ impl ScriptOrigin {
} }
} }
pub fn text(&self) -> DOMString { pub fn text(&self) -> Rc<DOMString> {
self.text.clone() Rc::clone(&self.text)
} }
} }
@ -257,7 +259,7 @@ impl FetchResponseListener for ClassicContext {
// Step 7. // Step 7.
let (source_text, _, _) = encoding.decode(&self.data); let (source_text, _, _) = encoding.decode(&self.data);
ScriptOrigin::external( ScriptOrigin::external(
DOMString::from(source_text), Rc::new(DOMString::from(source_text)),
metadata.final_url, metadata.final_url,
ScriptType::Classic, ScriptType::Classic,
) )
@ -613,7 +615,7 @@ impl HTMLScriptElement {
// Step 25-1. & 25-2. // Step 25-1. & 25-2.
let result = Ok(ScriptOrigin::internal( let result = Ok(ScriptOrigin::internal(
text.clone(), Rc::new(text.clone()),
base_url.clone(), base_url.clone(),
script_type.clone(), script_type.clone(),
)); ));
@ -647,7 +649,7 @@ impl HTMLScriptElement {
fetch_inline_module_script( fetch_inline_module_script(
ModuleOwner::Window(Trusted::new(self)), ModuleOwner::Window(Trusted::new(self)),
text.clone(), Rc::new(text.clone()),
base_url.clone(), base_url.clone(),
self.id.clone(), self.id.clone(),
credentials_mode.unwrap(), credentials_mode.unwrap(),
@ -679,7 +681,7 @@ impl HTMLScriptElement {
let mut script_content = String::new(); let mut script_content = String::new();
output.seek(std::io::SeekFrom::Start(0)).unwrap(); output.seek(std::io::SeekFrom::Start(0)).unwrap();
output.read_to_string(&mut script_content).unwrap(); output.read_to_string(&mut script_content).unwrap();
script.text = DOMString::from(script_content); script.text = Rc::new(DOMString::from(script_content));
}, },
_ => { _ => {
warn!("Failed to execute js-beautify. Will store unmodified script"); warn!("Failed to execute js-beautify. Will store unmodified script");
@ -757,7 +759,7 @@ impl HTMLScriptElement {
match read_to_string(path.clone()) { match read_to_string(path.clone()) {
Ok(local_script) => { Ok(local_script) => {
debug!("Found script stored at: {:?}", path); debug!("Found script stored at: {:?}", path);
script.text = DOMString::from(local_script); script.text = Rc::new(DOMString::from(local_script));
}, },
Err(why) => warn!("Could not restore script from file {:?}", why), Err(why) => warn!("Could not restore script from file {:?}", why),
} }

View file

@ -143,7 +143,7 @@ impl ModuleIdentity {
#[derive(JSTraceable)] #[derive(JSTraceable)]
pub struct ModuleTree { pub struct ModuleTree {
url: ServoUrl, url: ServoUrl,
text: DomRefCell<DOMString>, text: DomRefCell<Rc<DOMString>>,
record: DomRefCell<Option<ModuleObject>>, record: DomRefCell<Option<ModuleObject>>,
status: DomRefCell<ModuleStatus>, status: DomRefCell<ModuleStatus>,
// The spec maintains load order for descendants, so we use an indexset for descendants and // The spec maintains load order for descendants, so we use an indexset for descendants and
@ -171,7 +171,7 @@ impl ModuleTree {
pub fn new(url: ServoUrl, external: bool, visited_urls: HashSet<ServoUrl>) -> Self { pub fn new(url: ServoUrl, external: bool, visited_urls: HashSet<ServoUrl>) -> Self {
ModuleTree { ModuleTree {
url, url,
text: DomRefCell::new(DOMString::new()), text: DomRefCell::new(Rc::new(DOMString::new())),
record: DomRefCell::new(None), record: DomRefCell::new(None),
status: DomRefCell::new(ModuleStatus::Initial), status: DomRefCell::new(ModuleStatus::Initial),
parent_identities: DomRefCell::new(IndexSet::new()), parent_identities: DomRefCell::new(IndexSet::new()),
@ -217,11 +217,11 @@ impl ModuleTree {
*self.network_error.borrow_mut() = Some(network_error); *self.network_error.borrow_mut() = Some(network_error);
} }
pub fn get_text(&self) -> &DomRefCell<DOMString> { pub fn get_text(&self) -> &DomRefCell<Rc<DOMString>> {
&self.text &self.text
} }
pub fn set_text(&self, module_text: DOMString) { pub fn set_text(&self, module_text: Rc<DOMString>) {
*self.text.borrow_mut() = module_text; *self.text.borrow_mut() = module_text;
} }
@ -351,7 +351,7 @@ impl ModuleTree {
fn compile_module_script( fn compile_module_script(
&self, &self,
global: &GlobalScope, global: &GlobalScope,
module_script_text: DOMString, module_script_text: Rc<DOMString>,
url: ServoUrl, url: ServoUrl,
) -> Result<ModuleObject, RethrowError> { ) -> Result<ModuleObject, RethrowError> {
let module: Vec<u16> = module_script_text.encode_utf16().collect(); let module: Vec<u16> = module_script_text.encode_utf16().collect();
@ -852,12 +852,12 @@ impl ModuleOwner {
Some(network_error) => Err(network_error.clone()), Some(network_error) => Err(network_error.clone()),
None => match module_identity { None => match module_identity {
ModuleIdentity::ModuleUrl(script_src) => Ok(ScriptOrigin::external( ModuleIdentity::ModuleUrl(script_src) => Ok(ScriptOrigin::external(
module_tree.get_text().borrow().clone(), Rc::clone(&module_tree.get_text().borrow()),
script_src.clone(), script_src.clone(),
ScriptType::Module, ScriptType::Module,
)), )),
ModuleIdentity::ScriptId(_) => Ok(ScriptOrigin::internal( ModuleIdentity::ScriptId(_) => Ok(ScriptOrigin::internal(
module_tree.get_text().borrow().clone(), Rc::clone(&module_tree.get_text().borrow()),
document.base_url().clone(), document.base_url().clone(),
ScriptType::Module, ScriptType::Module,
)), )),
@ -980,7 +980,7 @@ impl FetchResponseListener for ModuleContext {
// Step 10. // Step 10.
let (source_text, _, _) = UTF_8.decode(&self.data); let (source_text, _, _) = UTF_8.decode(&self.data);
Ok(ScriptOrigin::external( Ok(ScriptOrigin::external(
DOMString::from(source_text), Rc::new(DOMString::from(source_text)),
meta.final_url, meta.final_url,
ScriptType::Module, ScriptType::Module,
)) ))
@ -1302,7 +1302,7 @@ pub fn fetch_single_module_script(
/// https://html.spec.whatwg.org/multipage/#fetch-an-inline-module-script-graph /// https://html.spec.whatwg.org/multipage/#fetch-an-inline-module-script-graph
pub fn fetch_inline_module_script( pub fn fetch_inline_module_script(
owner: ModuleOwner, owner: ModuleOwner,
module_script_text: DOMString, module_script_text: Rc<DOMString>,
url: ServoUrl, url: ServoUrl,
script_id: ScriptId, script_id: ScriptId,
credentials_mode: CredentialsMode, credentials_mode: CredentialsMode,