Auto merge of #23368 - gterzian:clean_up_navigation, r=asajeffrey

Clean-up navigation

<!-- Please describe your changes on the following line: -->

1. Navigation as a result of following a hyperlink should be done in a task: https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks:dom-manipulation-task-source
2. The javascript url navigation should also be done in a task: https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigating-across-documents:dom-manipulation-task-source
3. In `window.load_url`, it seems there is no need to send a message to the script-thread(the entirety of `load_url` should instead be done in a task when appropriate), so we can just do that last part "sync" by calling a method on the script, which will send a message to the constellation(for the parallel navigation steps), or queue task(for the JS navigation), as appropriate.
4. Separate the "normal" navigation flow from the handling of "navigate an iframe" message from constellation, since doing everything in one method as was previously done with `handle_navigate`, is confusing.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23368)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-07-18 14:22:03 -04:00 committed by GitHub
commit 2dfbbd0ca2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 402 additions and 220 deletions

View file

@ -11,6 +11,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLAnchorElementBinding::HTMLAncho
use crate::dom::bindings::codegen::Bindings::MouseEventBinding::MouseEventMethods;
use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
use crate::dom::bindings::str::{DOMString, USVString};
use crate::dom::document::determine_policy_for_token;
@ -19,16 +20,19 @@ use crate::dom::domtokenlist::DOMTokenList;
use crate::dom::element::Element;
use crate::dom::event::Event;
use crate::dom::eventtarget::EventTarget;
use crate::dom::globalscope::GlobalScope;
use crate::dom::htmlelement::HTMLElement;
use crate::dom::htmlimageelement::HTMLImageElement;
use crate::dom::mouseevent::MouseEvent;
use crate::dom::node::{document_from_node, Node};
use crate::dom::urlhelper::UrlHelper;
use crate::dom::virtualmethods::VirtualMethods;
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use html5ever::{LocalName, Prefix};
use net_traits::request::Referrer;
use num_traits::ToPrimitive;
use script_traits::{HistoryEntryReplacement, LoadData, LoadOrigin};
use servo_url::ServoUrl;
use std::default::Default;
use style::attr::AttrValue;
@ -624,8 +628,19 @@ pub fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>) {
// Step 7.
let (maybe_chosen, replace) = match target_attribute_value {
Some(name) => source.choose_browsing_context(name.Value(), noopener),
None => (Some(window.window_proxy()), false),
Some(name) => {
let (maybe_chosen, new) = source.choose_browsing_context(name.Value(), noopener);
let replace = if new {
HistoryEntryReplacement::Enabled
} else {
HistoryEntryReplacement::Disabled
};
(maybe_chosen, replace)
},
None => (
Some(window.window_proxy()),
HistoryEntryReplacement::Disabled,
),
};
// Step 8.
@ -667,7 +682,23 @@ pub fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>) {
};
// Step 14
debug!("following hyperlink to {}", url);
target_window.load_url(url, replace, false, referrer, referrer_policy);
let pipeline_id = target_window.upcast::<GlobalScope>().pipeline_id();
let load_data = LoadData::new(
LoadOrigin::Script(document.origin().immutable().clone()),
url,
Some(pipeline_id),
Some(referrer),
referrer_policy,
);
let target = Trusted::new(target_window);
let task = task!(navigate_follow_hyperlink: move || {
debug!("following hyperlink to {}", load_data.url);
target.root().load_url(replace, false, load_data);
});
target_window
.task_manager()
.dom_manipulation_task_source()
.queue(task, target_window.upcast())
.unwrap();
};
}

View file

@ -3,6 +3,7 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use crate::dom::bindings::cell::DomRefCell;
use crate::dom::bindings::codegen::Bindings::AttrBinding::AttrBinding::AttrMethods;
use crate::dom::bindings::codegen::Bindings::BlobBinding::BlobMethods;
use crate::dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
use crate::dom::bindings::codegen::Bindings::EventBinding::EventMethods;
@ -12,6 +13,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLFormElementBinding;
use crate::dom::bindings::codegen::Bindings::HTMLFormElementBinding::HTMLFormElementMethods;
use crate::dom::bindings::codegen::Bindings::HTMLInputElementBinding::HTMLInputElementMethods;
use crate::dom::bindings::codegen::Bindings::HTMLTextAreaElementBinding::HTMLTextAreaElementMethods;
use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
use crate::dom::bindings::refcounted::Trusted;
use crate::dom::bindings::reflector::DomObject;
@ -46,7 +48,6 @@ use crate::dom::node::{UnbindContext, VecPreOrderInsertionHelper};
use crate::dom::validitystate::ValidationFlags;
use crate::dom::virtualmethods::VirtualMethods;
use crate::dom::window::Window;
use crate::script_thread::MainThreadScriptMsg;
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use encoding_rs::{Encoding, UTF_8};
@ -56,7 +57,7 @@ use hyper::Method;
use mime::{self, Mime};
use net_traits::http_percent_encode;
use net_traits::request::Referrer;
use script_traits::LoadData;
use script_traits::{HistoryEntryReplacement, LoadData, LoadOrigin};
use servo_rand::random;
use std::borrow::ToOwned;
use std::cell::Cell;
@ -399,6 +400,7 @@ impl HTMLFormElement {
};
let target_window = target_document.window();
let mut load_data = LoadData::new(
LoadOrigin::Script(doc.origin().immutable().clone()),
action_components,
None,
Some(Referrer::ReferrerUrl(target_document.url())),
@ -529,7 +531,7 @@ impl HTMLFormElement {
}
/// [Planned navigation](https://html.spec.whatwg.org/multipage/#planned-navigation)
fn plan_to_navigate(&self, load_data: LoadData, target: &Window) {
fn plan_to_navigate(&self, mut load_data: LoadData, target: &Window) {
// Step 1
// Each planned navigation task is tagged with a generation ID, and
// before the task is handled, it first checks whether the HTMLFormElement's
@ -537,19 +539,35 @@ impl HTMLFormElement {
let generation_id = GenerationId(self.generation_id.get().0 + 1);
self.generation_id.set(generation_id);
// Step 2.
// Step 2
let elem = self.upcast::<Element>();
let referrer = match elem.get_attribute(&ns!(), &local_name!("rel")) {
Some(ref link_types) if link_types.Value().contains("noreferrer") => {
Referrer::NoReferrer
},
_ => Referrer::Client,
};
let referrer_policy = target.Document().get_referrer_policy();
let pipeline_id = target.upcast::<GlobalScope>().pipeline_id();
let script_chan = target.main_thread_script_chan().clone();
load_data.creator_pipeline_id = Some(pipeline_id);
load_data.referrer = Some(referrer);
load_data.referrer_policy = referrer_policy;
// Step 4.
let this = Trusted::new(self);
let window = Trusted::new(target);
let task = task!(navigate_to_form_planned_navigation: move || {
if generation_id != this.root().generation_id.get() {
return;
}
script_chan.send(MainThreadScriptMsg::Navigate(
pipeline_id,
load_data,
false,
)).unwrap();
window
.root()
.load_url(
HistoryEntryReplacement::Disabled,
false,
load_data,
);
});
// Step 3.

View file

@ -37,8 +37,8 @@ use profile_traits::ipc as ProfiledIpc;
use script_layout_interface::message::ReflowGoal;
use script_traits::IFrameSandboxState::{IFrameSandboxed, IFrameUnsandboxed};
use script_traits::{
IFrameLoadInfo, IFrameLoadInfoWithData, JsEvalResult, LoadData, UpdatePipelineIdReason,
WindowSizeData,
HistoryEntryReplacement, IFrameLoadInfo, IFrameLoadInfoWithData, JsEvalResult, LoadData,
LoadOrigin, UpdatePipelineIdReason, WindowSizeData,
};
use script_traits::{NewLayoutInfo, ScriptMsg};
use servo_url::ServoUrl;
@ -109,9 +109,9 @@ impl HTMLIFrameElement {
pub fn navigate_or_reload_child_browsing_context(
&self,
mut load_data: Option<LoadData>,
mut load_data: LoadData,
nav_type: NavigationType,
replace: bool,
replace: HistoryEntryReplacement,
) {
let sandboxed = if self.is_sandboxed() {
IFrameSandboxed
@ -136,30 +136,27 @@ impl HTMLIFrameElement {
// document; the new navigation will continue blocking it.
LoadBlocker::terminate(&mut load_blocker);
if let Some(ref mut load_data) = load_data {
let is_javascript = load_data.url.scheme() == "javascript";
if is_javascript {
let window_proxy = self.GetContentWindow();
if let Some(window_proxy) = window_proxy {
ScriptThread::eval_js_url(&window_proxy.global(), load_data);
if load_data.url.scheme() == "javascript" {
let window_proxy = self.GetContentWindow();
if let Some(window_proxy) = window_proxy {
// Important re security. See https://github.com/servo/servo/issues/23373
// TODO: check according to https://w3c.github.io/webappsec-csp/#should-block-navigation-request
if ScriptThread::check_load_origin(&load_data.load_origin, &document.url().origin())
{
ScriptThread::eval_js_url(&window_proxy.global(), &mut load_data);
}
}
}
//TODO(#9592): Deal with the case where an iframe is being reloaded so url is None.
// The iframe should always have access to the nested context's active
// document URL through the browsing context.
if let Some(ref load_data) = load_data {
match load_data.js_eval_result {
Some(JsEvalResult::NoContent) => (),
_ => {
*load_blocker = Some(LoadBlocker::new(
&*document,
LoadType::Subframe(load_data.url.clone()),
));
},
};
}
match load_data.js_eval_result {
Some(JsEvalResult::NoContent) => (),
_ => {
*load_blocker = Some(LoadBlocker::new(
&*document,
LoadType::Subframe(load_data.url.clone()),
));
},
};
let window = window_from_node(self);
let old_pipeline_id = self.pipeline_id();
@ -182,6 +179,12 @@ impl HTMLIFrameElement {
self.about_blank_pipeline_id.set(Some(new_pipeline_id));
let load_info = IFrameLoadInfoWithData {
info: load_info,
load_data: load_data.clone(),
old_pipeline_id: old_pipeline_id,
sandbox: sandboxed,
};
global_scope
.script_to_constellation_chan()
.send(ScriptMsg::ScriptNewIFrame(load_info, pipeline_sender))
@ -193,7 +196,7 @@ impl HTMLIFrameElement {
browsing_context_id: browsing_context_id,
top_level_browsing_context_id: top_level_browsing_context_id,
opener: None,
load_data: load_data.unwrap(),
load_data: load_data,
pipeline_port: pipeline_receiver,
content_process_shutdown_chan: None,
window_size: WindowSizeData {
@ -270,6 +273,7 @@ impl HTMLIFrameElement {
let document = document_from_node(self);
let load_data = LoadData::new(
LoadOrigin::Script(document.origin().immutable().clone()),
url,
creator_pipeline_id,
Some(Referrer::ReferrerUrl(document.url())),
@ -281,12 +285,12 @@ impl HTMLIFrameElement {
// see https://html.spec.whatwg.org/multipage/#the-iframe-element:about:blank-3
let is_about_blank =
pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get();
let replace = is_about_blank;
self.navigate_or_reload_child_browsing_context(
Some(load_data),
NavigationType::Regular,
replace,
);
let replace = if is_about_blank {
HistoryEntryReplacement::Enabled
} else {
HistoryEntryReplacement::Disabled
};
self.navigate_or_reload_child_browsing_context(load_data, NavigationType::Regular, replace);
}
fn create_nested_browsing_context(&self) {
@ -296,6 +300,7 @@ impl HTMLIFrameElement {
let window = window_from_node(self);
let pipeline_id = Some(window.upcast::<GlobalScope>().pipeline_id());
let load_data = LoadData::new(
LoadOrigin::Script(document.origin().immutable().clone()),
url,
pipeline_id,
Some(Referrer::ReferrerUrl(document.url().clone())),
@ -309,9 +314,9 @@ impl HTMLIFrameElement {
.set(Some(top_level_browsing_context_id));
self.browsing_context_id.set(Some(browsing_context_id));
self.navigate_or_reload_child_browsing_context(
Some(load_data),
load_data,
NavigationType::InitialAboutBlank,
false,
HistoryEntryReplacement::Disabled,
);
}

View file

@ -6,6 +6,7 @@ use crate::dom::bindings::codegen::Bindings::LocationBinding;
use crate::dom::bindings::codegen::Bindings::LocationBinding::LocationMethods;
use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
use crate::dom::bindings::error::{Error, ErrorResult, Fallible};
use crate::dom::bindings::inheritance::Castable;
use crate::dom::bindings::reflector::{reflect_dom_object, Reflector};
use crate::dom::bindings::root::{Dom, DomRoot};
use crate::dom::bindings::str::{DOMString, USVString};
@ -14,6 +15,7 @@ use crate::dom::urlhelper::UrlHelper;
use crate::dom::window::Window;
use dom_struct::dom_struct;
use net_traits::request::Referrer;
use script_traits::{HistoryEntryReplacement, LoadData, LoadOrigin};
use servo_url::{MutableOrigin, ServoUrl};
#[dom_struct]
@ -38,6 +40,29 @@ impl Location {
)
}
/// https://html.spec.whatwg.org/multipage/#location-object-navigate
fn navigate(
&self,
url: ServoUrl,
referrer: Referrer,
replacement_flag: HistoryEntryReplacement,
reload_triggered: bool,
) {
let document = self.window.Document();
let referrer_policy = document.get_referrer_policy();
let pipeline_id = self.window.upcast::<GlobalScope>().pipeline_id();
let load_data = LoadData::new(
LoadOrigin::Script(document.origin().immutable().clone()),
url,
Some(pipeline_id),
Some(referrer),
referrer_policy,
);
// TODO: rethrow exceptions, set exceptions enabled flag.
self.window
.load_url(replacement_flag, reload_triggered, load_data);
}
fn get_url(&self) -> ServoUrl {
self.window.get_url()
}
@ -46,7 +71,7 @@ impl Location {
let mut url = self.window.get_url();
let referrer = Referrer::ReferrerUrl(url.clone());
setter(&mut url, value);
self.window.load_url(url, false, false, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Disabled, false);
}
fn check_same_origin_domain(&self) -> ErrorResult {
@ -66,7 +91,7 @@ impl Location {
pub fn reload_without_origin_check(&self) {
let url = self.get_url();
let referrer = Referrer::ReferrerUrl(url.clone());
self.window.load_url(url, true, true, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Enabled, true);
}
#[allow(dead_code)]
@ -84,7 +109,7 @@ impl LocationMethods for Location {
let base_url = self.window.get_url();
if let Ok(url) = base_url.join(&url.0) {
let referrer = Referrer::ReferrerUrl(base_url.clone());
self.window.load_url(url, false, false, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Disabled, false);
Ok(())
} else {
Err(Error::Syntax)
@ -96,7 +121,7 @@ impl LocationMethods for Location {
self.check_same_origin_domain()?;
let url = self.get_url();
let referrer = Referrer::ReferrerUrl(url.clone());
self.window.load_url(url, true, true, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Enabled, true);
Ok(())
}
@ -108,7 +133,7 @@ impl LocationMethods for Location {
let base_url = self.window.get_url();
if let Ok(url) = base_url.join(&url.0) {
let referrer = Referrer::ReferrerUrl(base_url.clone());
self.window.load_url(url, true, false, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Enabled, false);
Ok(())
} else {
Err(Error::Syntax)
@ -178,7 +203,7 @@ impl LocationMethods for Location {
Err(e) => return Err(Error::Type(format!("Couldn't parse URL: {}", e))),
};
let referrer = Referrer::ReferrerUrl(current_url.clone());
self.window.load_url(url, false, false, referrer, None);
self.navigate(url, referrer, HistoryEntryReplacement::Disabled, false);
Ok(())
}

View file

@ -89,9 +89,8 @@ use media::WindowGLContext;
use msg::constellation_msg::PipelineId;
use net_traits::image_cache::{ImageCache, ImageResponder, ImageResponse};
use net_traits::image_cache::{PendingImageId, PendingImageResponse};
use net_traits::request::Referrer;
use net_traits::storage_thread::StorageType;
use net_traits::{ReferrerPolicy, ResourceThreads};
use net_traits::ResourceThreads;
use num_traits::ToPrimitive;
use profile_traits::ipc as ProfiledIpc;
use profile_traits::mem::ProfilerChan as MemProfilerChan;
@ -103,7 +102,7 @@ use script_layout_interface::rpc::{
};
use script_layout_interface::{PendingImageState, TrustedNodeAddress};
use script_traits::webdriver_msg::{WebDriverJSError, WebDriverJSResult};
use script_traits::{ConstellationControlMsg, DocumentState, LoadData};
use script_traits::{ConstellationControlMsg, DocumentState, HistoryEntryReplacement, LoadData};
use script_traits::{ScriptMsg, ScriptToConstellationChan, ScrollState, TimerEvent, TimerEventId};
use script_traits::{TimerSchedulerMsg, WindowSizeData, WindowSizeType};
use selectors::attr::CaseSensitivity;
@ -1770,27 +1769,31 @@ impl Window {
}
/// Commence a new URL load which will either replace this window or scroll to a fragment.
///
/// https://html.spec.whatwg.org/multipage/#navigating-across-documents
pub fn load_url(
&self,
url: ServoUrl,
replace: bool,
replace: HistoryEntryReplacement,
force_reload: bool,
referrer: Referrer,
referrer_policy: Option<ReferrerPolicy>,
load_data: LoadData,
) {
let doc = self.Document();
let referrer_policy = referrer_policy.or(doc.get_referrer_policy());
// https://html.spec.whatwg.org/multipage/#navigating-across-documents
// TODO: Important re security. See https://github.com/servo/servo/issues/23373
// Step 3: check that the source browsing-context is "allowed to navigate" this window.
if !force_reload &&
url.as_url()[..Position::AfterQuery] == doc.url().as_url()[..Position::AfterQuery]
load_data.url.as_url()[..Position::AfterQuery] ==
doc.url().as_url()[..Position::AfterQuery]
{
// Step 6
if let Some(fragment) = url.fragment() {
self.send_to_constellation(ScriptMsg::NavigatedToFragment(url.clone(), replace));
if let Some(fragment) = load_data.url.fragment() {
self.send_to_constellation(ScriptMsg::NavigatedToFragment(
load_data.url.clone(),
replace,
));
doc.check_and_scroll_fragment(fragment);
let this = Trusted::new(self);
let old_url = doc.url().into_string();
let new_url = url.clone().into_string();
let new_url = load_data.url.clone().into_string();
let task = task!(hashchange_event: move || {
let this = this.root();
let event = HashChangeEvent::new(
@ -1813,7 +1816,7 @@ impl Window {
self.pipeline_id(),
TaskSourceName::DOMManipulation,
));
doc.set_url(url.clone());
doc.set_url(load_data.url.clone());
return;
}
}
@ -1838,13 +1841,9 @@ impl Window {
// then put it in the delaying load events mode.
self.window_proxy().start_delaying_load_events_mode();
}
self.main_thread_script_chan()
.send(MainThreadScriptMsg::Navigate(
pipeline_id,
LoadData::new(url, Some(pipeline_id), Some(referrer), referrer_policy),
replace,
))
.unwrap();
// TODO: step 11, navigationType.
// Step 12, 13
ScriptThread::navigate(pipeline_id, load_data, replace);
};
}

View file

@ -47,7 +47,10 @@ use msg::constellation_msg::BrowsingContextId;
use msg::constellation_msg::PipelineId;
use msg::constellation_msg::TopLevelBrowsingContextId;
use net_traits::request::Referrer;
use script_traits::{AuxiliaryBrowsingContextLoadInfo, LoadData, NewLayoutInfo, ScriptMsg};
use script_traits::{
AuxiliaryBrowsingContextLoadInfo, HistoryEntryReplacement, LoadData, LoadOrigin,
};
use script_traits::{NewLayoutInfo, ScriptMsg};
use servo_url::ServoUrl;
use std::cell::Cell;
use std::ptr;
@ -268,24 +271,28 @@ impl WindowProxy {
let new_browsing_context_id =
BrowsingContextId::from(new_top_level_browsing_context_id);
let new_pipeline_id = PipelineId::new();
let load_info = AuxiliaryBrowsingContextLoadInfo {
opener_pipeline_id: self.currently_active.get().unwrap(),
new_browsing_context_id: new_browsing_context_id,
new_top_level_browsing_context_id: new_top_level_browsing_context_id,
new_pipeline_id: new_pipeline_id,
};
let document = self
.currently_active
.get()
.and_then(|id| ScriptThread::find_document(id))
.unwrap();
.expect("A WindowProxy creating an auxiliary to have an active document");
let blank_url = ServoUrl::parse("about:blank").ok().unwrap();
let load_data = LoadData::new(
LoadOrigin::Script(document.origin().immutable().clone()),
blank_url,
None,
Some(Referrer::ReferrerUrl(document.url().clone())),
document.get_referrer_policy(),
);
let load_info = AuxiliaryBrowsingContextLoadInfo {
load_data: load_data.clone(),
opener_pipeline_id: self.currently_active.get().unwrap(),
new_browsing_context_id: new_browsing_context_id,
new_top_level_browsing_context_id: new_top_level_browsing_context_id,
new_pipeline_id: new_pipeline_id,
};
let (pipeline_sender, pipeline_receiver) = ipc::channel().unwrap();
let new_layout_info = NewLayoutInfo {
parent_info: None,
@ -437,13 +444,21 @@ impl WindowProxy {
Referrer::Client
};
// Step 14.5
target_window.load_url(
let referrer_policy = target_document.get_referrer_policy();
let pipeline_id = target_window.upcast::<GlobalScope>().pipeline_id();
let load_data = LoadData::new(
LoadOrigin::Script(existing_document.origin().immutable().clone()),
url,
new,
false,
referrer,
target_document.get_referrer_policy(),
Some(pipeline_id),
Some(referrer),
referrer_policy,
);
let replacement_flag = if new {
HistoryEntryReplacement::Enabled
} else {
HistoryEntryReplacement::Disabled
};
target_window.load_url(replacement_flag, false, load_data);
}
if noopener {
// Step 15 (Dis-owning has been done in create_auxiliary_browsing_context).