servo: Merge #19329 - Add RAII guard for cancelling fetch when the consumer no longer cares about it (from Manishearth:fetchcanceller); r=jdm

Source-Repo: https://github.com/servo/servo
Source-Revision: 976f9e3d13b6a7676dd863dd397dd6c60e868d58

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 9d7caa9c57c75423e10f6802d23835b2522d9f14
This commit is contained in:
Manish Goregaokar 2017-11-22 18:30:57 -06:00
parent e88ffe59d1
commit f1502e5e7d
12 changed files with 110 additions and 35 deletions

View File

@ -1110,9 +1110,9 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
FromScriptMsg::PipelineExited => {
self.handle_pipeline_exited(source_pipeline_id);
}
FromScriptMsg::InitiateNavigateRequest(req_init) => {
FromScriptMsg::InitiateNavigateRequest(req_init, cancel_chan) => {
debug!("constellation got initiate navigate request message");
self.handle_navigate_request(source_pipeline_id, req_init);
self.handle_navigate_request(source_pipeline_id, req_init, cancel_chan);
}
FromScriptMsg::ScriptLoadedURLInIFrame(load_info) => {
debug!("constellation got iframe URL load message {:?} {:?} {:?}",
@ -1689,14 +1689,15 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
fn handle_navigate_request(&self,
id: PipelineId,
req_init: RequestInit) {
req_init: RequestInit,
cancel_chan: IpcReceiver<()>) {
let listener = NetworkListener::new(
req_init,
id,
self.public_resource_threads.clone(),
self.network_listener_sender.clone());
listener.initiate_fetch();
listener.initiate_fetch(Some(cancel_chan));
}
// The script thread associated with pipeline_id has loaded a URL in an iframe via script. This

View File

@ -41,7 +41,7 @@ impl NetworkListener {
}
}
pub fn initiate_fetch(&self) {
pub fn initiate_fetch(&self, cancel_chan: Option<ipc::IpcReceiver<()>>) {
let (ipc_sender, ipc_receiver) = ipc::channel().expect("Failed to create IPC channel!");
let mut listener = NetworkListener {
@ -64,7 +64,7 @@ impl NetworkListener {
CoreResourceMsg::Fetch(
listener.req_init.clone(),
FetchChannels::ResponseMsg(ipc_sender, None))
FetchChannels::ResponseMsg(ipc_sender, cancel_chan))
}
};
@ -108,7 +108,11 @@ impl NetworkListener {
referrer: metadata.referrer.clone(),
});
self.initiate_fetch();
// XXXManishearth we don't have the cancel_chan anymore and
// can't use it here.
//
// Ideally the Fetch code would handle manual redirects on its own
self.initiate_fetch(None);
},
_ => {
// Response should be processed by script thread.

View File

@ -91,6 +91,7 @@ use dom::windowproxy::WindowProxy;
use dom_struct::dom_struct;
use encoding_rs::{Encoding, UTF_8};
use euclid::Point2D;
use fetch::FetchCanceller;
use html5ever::{LocalName, Namespace, QualName};
use hyper::header::{Header, SetCookie};
use hyper_serde::Serde;
@ -360,6 +361,8 @@ pub struct Document {
form_id_listener_map: DomRefCell<HashMap<Atom, HashSet<Dom<Element>>>>,
interactive_time: DomRefCell<InteractiveMetrics>,
tti_window: DomRefCell<InteractiveWindow>,
/// RAII canceller for Fetch
canceller: FetchCanceller,
}
#[derive(JSTraceable, MallocSizeOf)]
@ -2165,7 +2168,8 @@ impl Document {
source: DocumentSource,
doc_loader: DocumentLoader,
referrer: Option<String>,
referrer_policy: Option<ReferrerPolicy>)
referrer_policy: Option<ReferrerPolicy>,
canceller: FetchCanceller)
-> Document {
let url = url.unwrap_or_else(|| ServoUrl::parse("about:blank").unwrap());
@ -2270,6 +2274,7 @@ impl Document {
form_id_listener_map: Default::default(),
interactive_time: DomRefCell::new(interactive_time),
tti_window: DomRefCell::new(InteractiveWindow::new()),
canceller: canceller,
}
}
@ -2288,7 +2293,8 @@ impl Document {
DocumentSource::NotFromParser,
docloader,
None,
None))
None,
Default::default()))
}
pub fn new(window: &Window,
@ -2302,7 +2308,8 @@ impl Document {
source: DocumentSource,
doc_loader: DocumentLoader,
referrer: Option<String>,
referrer_policy: Option<ReferrerPolicy>)
referrer_policy: Option<ReferrerPolicy>,
canceller: FetchCanceller)
-> DomRoot<Document> {
let document = reflect_dom_object(
Box::new(Document::new_inherited(
@ -2317,7 +2324,8 @@ impl Document {
source,
doc_loader,
referrer,
referrer_policy
referrer_policy,
canceller
)),
window,
DocumentBinding::Wrap
@ -2474,7 +2482,8 @@ impl Document {
DocumentSource::NotFromParser,
DocumentLoader::new(&self.loader()),
None,
None);
None,
Default::default());
new_doc.appropriate_template_contents_owner_document.set(Some(&new_doc));
new_doc
})

View File

@ -137,7 +137,8 @@ impl DOMImplementationMethods for DOMImplementation {
DocumentSource::NotFromParser,
loader,
None,
None);
None,
Default::default());
{
// Step 3.

View File

@ -70,7 +70,8 @@ impl DOMParserMethods for DOMParser {
DocumentSource::FromParser,
loader,
None,
None);
None,
Default::default());
ServoParser::parse_html_document(&document, s, url);
document.set_ready_state(DocumentReadyState::Complete);
Ok(document)
@ -88,7 +89,8 @@ impl DOMParserMethods for DOMParser {
DocumentSource::NotFromParser,
loader,
None,
None);
None,
Default::default());
ServoParser::parse_xml_document(&document, s, url);
Ok(document)
}

View File

@ -1814,7 +1814,7 @@ impl Node {
is_html_doc, None,
None, DocumentActivity::Inactive,
DocumentSource::NotFromParser, loader,
None, None);
None, None, Default::default());
DomRoot::upcast::<Node>(document)
},
NodeTypeId::Element(..) => {

View File

@ -138,7 +138,8 @@ impl ServoParser {
DocumentSource::FromParser,
loader,
None,
None);
None,
Default::default());
// Step 2.
document.set_quirks_mode(context_document.quirks_mode());

View File

@ -48,7 +48,8 @@ impl XMLDocument {
source,
doc_loader,
None,
None),
None,
Default::default()),
}
}

View File

@ -38,6 +38,7 @@ use dom::xmlhttprequestupload::XMLHttpRequestUpload;
use dom_struct::dom_struct;
use encoding_rs::{Encoding, UTF_8};
use euclid::Length;
use fetch::FetchCanceller;
use html5ever::serialize;
use html5ever::serialize::SerializeOpts;
use hyper::header::{ContentLength, ContentType, ContentEncoding};
@ -154,8 +155,7 @@ pub struct XMLHttpRequest {
response_status: Cell<Result<(), ()>>,
referrer_url: Option<ServoUrl>,
referrer_policy: Option<ReferrerPolicy>,
#[ignore_malloc_size_of = "channels are hard"]
cancellation_chan: DomRefCell<Option<ipc::IpcSender<()>>>,
canceller: DomRefCell<FetchCanceller>,
}
impl XMLHttpRequest {
@ -200,7 +200,7 @@ impl XMLHttpRequest {
response_status: Cell::new(Ok(())),
referrer_url: referrer_url,
referrer_policy: referrer_policy,
cancellation_chan: DomRefCell::new(None),
canceller: DomRefCell::new(Default::default()),
}
}
pub fn new(global: &GlobalScope) -> DomRoot<XMLHttpRequest> {
@ -978,6 +978,7 @@ impl XMLHttpRequest {
self.sync.get());
self.cancel_timeout();
self.canceller.borrow_mut().ignore();
// Part of step 11, send() (processing response end of file)
// XXXManishearth handle errors, if any (substep 2)
@ -994,6 +995,7 @@ impl XMLHttpRequest {
},
XHRProgress::Errored(_, e) => {
self.cancel_timeout();
self.canceller.borrow_mut().ignore();
self.discard_subsequent_responses();
self.send_flag.set(false);
@ -1023,12 +1025,7 @@ impl XMLHttpRequest {
}
fn terminate_ongoing_fetch(&self) {
if let Some(ref cancel_chan) = *self.cancellation_chan.borrow() {
// The receiver will be destroyed if the request has already completed;
// so we throw away the error. Cancellation is a courtesy call,
// we don't actually care if the other side heard.
let _ = cancel_chan.send(());
}
self.canceller.borrow_mut().cancel();
let GenerationId(prev_id) = self.generation_id.get();
self.generation_id.set(GenerationId(prev_id + 1));
self.response_status.set(Ok(()));
@ -1264,7 +1261,8 @@ impl XMLHttpRequest {
DocumentSource::FromParser,
docloader,
None,
None)
None,
Default::default())
}
fn filter_response_headers(&self) -> Headers {
@ -1322,8 +1320,7 @@ impl XMLHttpRequest {
(global.networking_task_source(), None)
};
let (cancel_sender, cancel_receiver) = ipc::channel().unwrap();
*self.cancellation_chan.borrow_mut() = Some(cancel_sender);
let cancel_receiver = self.canceller.borrow_mut().initialize();
XMLHttpRequest::initiate_async_xhr(context.clone(), task_source,
global, init, cancel_receiver);

View File

@ -38,6 +38,58 @@ struct FetchContext {
body: Vec<u8>,
}
/// RAII fetch canceller object. By default initialized to not having a canceller
/// in it, however you can ask it for a cancellation receiver to send to Fetch
/// in which case it will store the sender. You can manually cancel it
/// or let it cancel on Drop in that case.
#[derive(Default, JSTraceable, MallocSizeOf)]
pub struct FetchCanceller {
#[ignore_malloc_size_of = "channels are hard"]
cancel_chan: Option<ipc::IpcSender<()>>
}
impl FetchCanceller {
/// Create an empty FetchCanceller
pub fn new() -> Self {
Default::default()
}
/// Obtain an IpcReceiver to send over to Fetch, and initialize
/// the internal sender
pub fn initialize(&mut self) -> ipc::IpcReceiver<()> {
// cancel previous fetch
self.cancel();
let (rx, tx) = ipc::channel().unwrap();
self.cancel_chan = Some(rx);
tx
}
/// Cancel a fetch if it is ongoing
pub fn cancel(&mut self) {
if let Some(chan) = self.cancel_chan.take() {
// stop trying to make fetch happen
// it's not going to happen
// The receiver will be destroyed if the request has already completed;
// so we throw away the error. Cancellation is a courtesy call,
// we don't actually care if the other side heard.
let _ = chan.send(());
}
}
/// Use this if you don't want it to send a cancellation request
/// on drop (e.g. if the fetch completes)
pub fn ignore(&mut self) {
let _ = self.cancel_chan.take();
}
}
impl Drop for FetchCanceller {
fn drop(&mut self) {
self.cancel()
}
}
fn from_referrer_to_referrer_url(request: &NetTraitsRequest) -> Option<ServoUrl> {
request.referrer.to_url().map(|url| url.clone())
}

View File

@ -62,6 +62,7 @@ use dom::worker::TrustedWorkerAddress;
use dom::worklet::WorkletThreadPool;
use dom::workletglobalscope::WorkletGlobalScopeInit;
use euclid::{Point2D, Vector2D, Rect};
use fetch::FetchCanceller;
use hyper::header::{ContentType, HttpDate, Headers, LastModified};
use hyper::header::ReferrerPolicy as ReferrerPolicyHeader;
use hyper::mime::{Mime, SubLevel, TopLevel};
@ -170,6 +171,8 @@ struct InProgressLoad {
navigation_start: u64,
/// High res timestamp reporting the time when the browser started this load.
navigation_start_precise: u64,
/// For cancelling the fetch
canceller: FetchCanceller,
}
impl InProgressLoad {
@ -198,6 +201,7 @@ impl InProgressLoad {
origin: origin,
navigation_start: (current_time.sec * 1000 + current_time.nsec as i64 / 1000000) as u64,
navigation_start_precise: navigation_start_precise,
canceller: Default::default(),
}
}
}
@ -2215,7 +2219,8 @@ impl ScriptThread {
DocumentSource::FromParser,
loader,
referrer,
referrer_policy);
referrer_policy,
incomplete.canceller);
document.set_ready_state(DocumentReadyState::Loading);
self.documents.borrow_mut().insert(incomplete.pipeline_id, &*document);
@ -2536,7 +2541,7 @@ impl ScriptThread {
/// Instructs the constellation to fetch the document that will be loaded. Stores the InProgressLoad
/// argument until a notification is received that the fetch is complete.
fn pre_page_load(&self, incomplete: InProgressLoad, load_data: LoadData) {
fn pre_page_load(&self, mut incomplete: InProgressLoad, load_data: LoadData) {
let id = incomplete.pipeline_id.clone();
let req_init = RequestInit {
url: load_data.url.clone(),
@ -2557,7 +2562,9 @@ impl ScriptThread {
let context = ParserContext::new(id, load_data.url);
self.incomplete_parser_contexts.borrow_mut().push((id, context));
self.script_sender.send((id, ScriptMsg::InitiateNavigateRequest(req_init))).unwrap();
let cancel_chan = incomplete.canceller.initialize();
self.script_sender.send((id, ScriptMsg::InitiateNavigateRequest(req_init, cancel_chan))).unwrap();
self.incomplete_loads.borrow_mut().push(incomplete);
}

View File

@ -16,7 +16,7 @@ use canvas_traits::canvas::CanvasMsg;
use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId};
use euclid::{Point2D, Size2D, TypedSize2D};
use gfx_traits::Epoch;
use ipc_channel::ipc::IpcSender;
use ipc_channel::ipc::{IpcReceiver, IpcSender};
use msg::constellation_msg::{BrowsingContextId, FrameType, PipelineId, TraversalDirection};
use msg::constellation_msg::{Key, KeyModifiers, KeyState};
use net_traits::CoreResourceMsg;
@ -71,7 +71,7 @@ pub enum LogEntry {
pub enum ScriptMsg {
/// Requests are sent to constellation and fetches are checked manually
/// for cross-origin loads
InitiateNavigateRequest(RequestInit),
InitiateNavigateRequest(RequestInit, /* cancellation_chan */ IpcReceiver<()>),
/// Broadcast a storage event to every same-origin pipeline.
/// The strings are key, old value and new value.
BroadcastStorageEvent(StorageType, ServoUrl, Option<String>, Option<String>, Option<String>),