Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make document url mutable and implement location.replace() #13418

Merged
merged 5 commits into from Nov 20, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Remove redundant url clones

They are now redundant since now document.url() returns a struct rather
than a reference.
  • Loading branch information
stshine committed Nov 18, 2016
commit 91f3d4f4749a1dd53d665f741be85559b820cbe7
@@ -615,5 +615,5 @@ fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>, referre
debug!("following hyperlink to {}", url);

let window = document.window();
window.load_url(url, false, referrer_policy);
window.load_url(url, false, false, referrer_policy);
}
@@ -341,7 +341,7 @@ impl HTMLFormElement {
let _target = submitter.target();
// TODO: Handle browsing contexts, partially loaded documents (step 16-17)

let mut load_data = LoadData::new(action_components, doc.get_referrer_policy(), Some(doc.url().clone()));
let mut load_data = LoadData::new(action_components, doc.get_referrer_policy(), Some(doc.url()));

// Step 18
match (&*scheme, method) {
@@ -157,7 +157,7 @@ impl HTMLIFrameElement {

let document = document_from_node(self);
self.navigate_or_reload_child_browsing_context(
Some(LoadData::new(url, document.get_referrer_policy(), Some(document.url().clone()))), false);
Some(LoadData::new(url, document.get_referrer_policy(), Some(document.url()))), false);
}

#[allow(unsafe_code)]
@@ -280,9 +280,9 @@ impl HTMLLinkElement {
destination: Destination::Style,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: document.url().clone(),
origin: document.url(),
pipeline_id: Some(self.global().pipeline_id()),
referrer_url: Some(document.url().clone()),
referrer_url: Some(document.url()),
referrer_policy: referrer_policy,
.. RequestInit::default()
};
@@ -548,9 +548,9 @@ impl HTMLMediaElement {
destination: Destination::Media,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: document.url().clone(),
origin: document.url(),
pipeline_id: Some(self.global().pipeline_id()),
referrer_url: Some(document.url().clone()),
referrer_url: Some(document.url()),
referrer_policy: document.get_referrer_policy(),
.. RequestInit::default()
};
@@ -241,9 +241,9 @@ fn fetch_a_classic_script(script: &HTMLScriptElement,
Some(CorsSettings::Anonymous) => CredentialsMode::CredentialsSameOrigin,
_ => CredentialsMode::Include,
},
origin: doc.url().clone(),
origin: doc.url(),
pipeline_id: Some(script.global().pipeline_id()),
referrer_url: Some(doc.url().clone()),
referrer_url: Some(doc.url()),
referrer_policy: doc.get_referrer_policy(),
.. RequestInit::default()
};
@@ -135,7 +135,7 @@ macro_rules! make_string_or_document_url_getter(

if val.is_empty() {
let doc = document_from_node(self);
DOMString::from(doc.url().clone().into_string())
DOMString::from(doc.url().into_string())
} else {
val
}
@@ -1751,7 +1751,7 @@ impl Node {
let window = document.window();
let loader = DocumentLoader::new(&*document.loader());
let document = Document::new(window, None,
Some((*document.url()).clone()),
Some(document.url()),
is_html_doc, None,
None, DocumentSource::NotFromParser, loader,
None, None);
@@ -1366,7 +1366,7 @@ impl Window {
}

pub fn get_url(&self) -> ServoUrl {
(*self.Document().url()).clone()
self.Document().url()
}

pub fn layout_chan(&self) -> &Sender<Msg> {
@@ -159,7 +159,7 @@ impl XMLHttpRequest {
//TODO - update this when referrer policy implemented for workers
let (referrer_url, referrer_policy) = if let Some(window) = global.downcast::<Window>() {
let document = window.Document();
(Some(document.url().clone()), document.get_referrer_policy())
(Some(document.url()), document.get_referrer_policy())
} else {
(None, None)
};
@@ -1239,7 +1239,7 @@ impl ScriptThread {
let mut reports = vec![];

for (_, document) in self.documents.borrow().iter() {
let current_url = document.url().as_str();
let current_url = document.url();

for child in document.upcast::<Node>().traverse_preorder() {
dom_tree_size += heap_size_of_self_and_children(&*child);
@@ -1249,10 +1249,10 @@ impl ScriptThread {
if reports.len() > 0 {
path_seg.push_str(", ");
}
path_seg.push_str(current_url);
path_seg.push_str(current_url.as_str());

reports.push(Report {
path: path![format!("url({})", current_url), "dom-tree"],
path: path![format!("url({})", current_url.as_str()), "dom-tree"],

This comment has been minimized.

@KiChjang

KiChjang Nov 18, 2016

Member

I don't see why this is an improvement?

This comment has been minimized.

@stshine

stshine Nov 19, 2016

Author Contributor

Oh this is because now document.url() return a value while as_str() returns a reference, so we have to keep the value on the stack.

This comment has been minimized.

@KiChjang

KiChjang Nov 19, 2016

Member

Why does the existing code not work?

This comment has been minimized.

@stshine

stshine Nov 19, 2016

Author Contributor

In the existing code document.url() return a reference so document.url().as_str() borrows ownership from document, but now document.url() returns a value as_str() borrow ownership from it, so we have to keep it in the url variable.

kind: ReportKind::ExplicitJemallocHeapSize,
size: dom_tree_size,
});
@@ -195,7 +195,7 @@ pub fn handle_get_cookies(documents: &Documents,
let url = document.url();
let (sender, receiver) = ipc::channel().unwrap();
let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
GetCookiesDataForUrl(url.clone(), sender, NonHTTP)
GetCookiesDataForUrl(url, sender, NonHTTP)
);
receiver.recv().unwrap()
},
@@ -215,7 +215,7 @@ pub fn handle_get_cookie(documents: &Documents,
let url = document.url();
let (sender, receiver) = ipc::channel().unwrap();
let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
GetCookiesDataForUrl(url.clone(), sender, NonHTTP)
GetCookiesDataForUrl(url, sender, NonHTTP)
);
receiver.recv().unwrap()
},
@@ -243,13 +243,13 @@ pub fn handle_add_cookie(documents: &Documents,
(true, _) => Err(WebDriverCookieError::InvalidDomain),
(false, Some(ref domain)) if url.host_str().map(|x| { x == &**domain }).unwrap_or(false) => {
let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
SetCookiesForUrlWithData(url.clone(), cookie, method)
SetCookiesForUrlWithData(url, cookie, method)
);
Ok(())
},
(false, None) => {
let _ = document.window().upcast::<GlobalScope>().resource_threads().send(
SetCookiesForUrlWithData(url.clone(), cookie, method)
SetCookiesForUrlWithData(url, cookie, method)
);
Ok(())
},
@@ -364,7 +364,7 @@ pub fn handle_get_url(documents: &Documents,
reply: IpcSender<ServoUrl>) {
// TODO: Return an error if the pipeline doesn't exist.
let url = documents.find_document(pipeline)
.map(|document| document.url().clone())
.map(|document| document.url())
.unwrap_or_else(|| ServoUrl::parse("about:blank").expect("infallible"));
reply.send(url).unwrap();
}
@@ -208,7 +208,7 @@ impl AttrValue {
AttrValue::Atom(value)
}

pub fn from_url(base: &ServoUrl, url: String) -> AttrValue {
pub fn from_url(base: ServoUrl, url: String) -> AttrValue {
let joined = base.join(&url).ok();
AttrValue::Url(url, joined)
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.