Responding to review comments

This commit is contained in:
Alan Jeffrey 2019-09-11 11:40:50 -05:00
parent aeac382058
commit 49a5e84fb1
4 changed files with 127 additions and 105 deletions

View file

@ -25,6 +25,7 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use net_traits::request::{Destination, RequestBuilder}; use net_traits::request::{Destination, RequestBuilder};
use net_traits::response::{Response, ResponseInit}; use net_traits::response::{Response, ResponseInit};
use net_traits::storage_thread::StorageThreadMsg; use net_traits::storage_thread::StorageThreadMsg;
use net_traits::DiscardFetch;
use net_traits::FetchTaskTarget; use net_traits::FetchTaskTarget;
use net_traits::WebSocketNetworkEvent; use net_traits::WebSocketNetworkEvent;
use net_traits::{CookieSource, CoreResourceMsg, CoreResourceThread}; use net_traits::{CookieSource, CoreResourceMsg, CoreResourceThread};
@ -248,7 +249,7 @@ impl ResourceChannelManager {
), ),
FetchChannels::Prefetch => { FetchChannels::Prefetch => {
self.resource_manager self.resource_manager
.fetch(req_init, None, (), http_state, None) .fetch(req_init, None, DiscardFetch, http_state, None)
}, },
}, },
CoreResourceMsg::DeleteCookies(request) => { CoreResourceMsg::DeleteCookies(request) => {

View file

@ -237,7 +237,12 @@ impl FetchTaskTarget for IpcSender<FetchResponseMsg> {
} }
} }
impl FetchTaskTarget for () { /// A fetch task that discards all data it's sent,
/// useful when speculatively prefetching data that we don't need right
/// now, but might need in the future.
pub struct DiscardFetch;
impl FetchTaskTarget for DiscardFetch {
fn process_request_body(&mut self, _: &Request) {} fn process_request_body(&mut self, _: &Request) {}
fn process_request_eof(&mut self, _: &Request) {} fn process_request_eof(&mut self, _: &Request) {}

View file

@ -102,7 +102,9 @@ pub struct ServoParser {
aborted: Cell<bool>, aborted: Cell<bool>,
/// <https://html.spec.whatwg.org/multipage/#script-created-parser> /// <https://html.spec.whatwg.org/multipage/#script-created-parser>
script_created_parser: bool, script_created_parser: bool,
/// We do a quick-and-dirty parse of the input looking for resources to prefetch /// We do a quick-and-dirty parse of the input looking for resources to prefetch.
// TODO: if we had speculative parsing, we could do this when speculatively
// building the DOM. https://github.com/servo/servo/pull/19203
prefetch_tokenizer: DomRefCell<prefetch::Tokenizer>, prefetch_tokenizer: DomRefCell<prefetch::Tokenizer>,
#[ignore_malloc_size_of = "Defined in html5ever"] #[ignore_malloc_size_of = "Defined in html5ever"]
prefetch_input: DomRefCell<BufferQueue>, prefetch_input: DomRefCell<BufferQueue>,
@ -433,30 +435,31 @@ impl ServoParser {
} }
fn push_tendril_input_chunk(&self, chunk: StrTendril) { fn push_tendril_input_chunk(&self, chunk: StrTendril) {
if !chunk.is_empty() { if chunk.is_empty() {
// Per https://github.com/whatwg/html/issues/1495 return;
// stylesheets should not be loaded for documents
// without browsing contexts.
// https://github.com/whatwg/html/issues/1495#issuecomment-230334047
// suggests that no content should be preloaded in such a case.
// We're conservative, and only prefetch for documents
// with browsing contexts.
if self.document.browsing_context().is_some() {
// Push the chunk into the prefetch input stream,
// which is tokenized eagerly, to scan for resources
// to prefetch. If the user script uses `document.write()`
// to overwrite the network input, this prefetching may
// have been wasted, but in most cases it won't.
let mut prefetch_input = self.prefetch_input.borrow_mut();
prefetch_input.push_back(chunk.clone());
let _ = self.prefetch_tokenizer
.borrow_mut()
.feed(&mut *prefetch_input);
}
// Push the chunk into the network input stream,
// which is tokenized lazily.
self.network_input.borrow_mut().push_back(chunk);
} }
// Per https://github.com/whatwg/html/issues/1495
// stylesheets should not be loaded for documents
// without browsing contexts.
// https://github.com/whatwg/html/issues/1495#issuecomment-230334047
// suggests that no content should be preloaded in such a case.
// We're conservative, and only prefetch for documents
// with browsing contexts.
if self.document.browsing_context().is_some() {
// Push the chunk into the prefetch input stream,
// which is tokenized eagerly, to scan for resources
// to prefetch. If the user script uses `document.write()`
// to overwrite the network input, this prefetching may
// have been wasted, but in most cases it won't.
let mut prefetch_input = self.prefetch_input.borrow_mut();
prefetch_input.push_back(chunk.clone());
self.prefetch_tokenizer
.borrow_mut()
.feed(&mut *prefetch_input);
}
// Push the chunk into the network input stream,
// which is tokenized lazily.
self.network_input.borrow_mut().push_back(chunk);
} }
fn push_bytes_input_chunk(&self, chunk: Vec<u8>) { fn push_bytes_input_chunk(&self, chunk: Vec<u8>) {
@ -471,7 +474,6 @@ impl ServoParser {
} }
fn push_string_input_chunk(&self, chunk: String) { fn push_string_input_chunk(&self, chunk: String) {
// Convert the chunk to a tendril so cloning it isn't expensive.
// The input has already been decoded as a string, so doesn't need // The input has already been decoded as a string, so doesn't need
// to be decoded by the network decoder again. // to be decoded by the network decoder again.
let chunk = StrTendril::from(chunk); let chunk = StrTendril::from(chunk);

View file

@ -9,6 +9,7 @@ use crate::dom::htmlimageelement::image_fetch_request;
use crate::dom::htmlscriptelement::script_fetch_request; use crate::dom::htmlscriptelement::script_fetch_request;
use crate::stylesheet_loader::stylesheet_fetch_request; use crate::stylesheet_loader::stylesheet_fetch_request;
use html5ever::buffer_queue::BufferQueue; use html5ever::buffer_queue::BufferQueue;
use html5ever::tokenizer::states::RawKind;
use html5ever::tokenizer::Tag; use html5ever::tokenizer::Tag;
use html5ever::tokenizer::TagKind; use html5ever::tokenizer::TagKind;
use html5ever::tokenizer::Token; use html5ever::tokenizer::Token;
@ -63,8 +64,8 @@ impl Tokenizer {
Tokenizer { inner } Tokenizer { inner }
} }
pub fn feed(&mut self, input: &mut BufferQueue) -> TokenizerResult<()> { pub fn feed(&mut self, input: &mut BufferQueue) {
self.inner.feed(input) while let TokenizerResult::Script(PrefetchHandle) = self.inner.feed(input) {}
} }
} }
@ -79,86 +80,99 @@ struct PrefetchSink {
prefetching: bool, prefetching: bool,
} }
/// The prefetch tokenizer produces trivial results
struct PrefetchHandle;
impl TokenSink for PrefetchSink { impl TokenSink for PrefetchSink {
type Handle = (); type Handle = PrefetchHandle;
fn process_token(&mut self, token: Token, _line_number: u64) -> TokenSinkResult<()> { fn process_token(
if let Token::TagToken(ref tag) = token { &mut self,
match (tag.kind, &tag.name) { token: Token,
(TagKind::StartTag, local_name!("script")) if self.prefetching => { _line_number: u64,
if let Some(url) = self.get_url(tag, local_name!("src")) { ) -> TokenSinkResult<PrefetchHandle> {
debug!("Prefetch script {}", url); let tag = match token {
let cors_setting = self.get_cors_settings(tag, local_name!("crossorigin")); Token::TagToken(ref tag) => tag,
let integrity_metadata = self _ => return TokenSinkResult::Continue,
.get_attr(tag, local_name!("integrity")) };
.map(|attr| String::from(&attr.value)) match (tag.kind, &tag.name) {
.unwrap_or_default(); (TagKind::StartTag, local_name!("script")) if self.prefetching => {
let request = script_fetch_request( if let Some(url) = self.get_url(tag, local_name!("src")) {
url, debug!("Prefetch script {}", url);
cors_setting, let cors_setting = self.get_cors_settings(tag, local_name!("crossorigin"));
self.origin.clone(), let integrity_metadata = self
self.pipeline_id, .get_attr(tag, local_name!("integrity"))
self.referrer.clone(), .map(|attr| String::from(&attr.value))
self.referrer_policy, .unwrap_or_default();
integrity_metadata, let request = script_fetch_request(
); url,
let _ = self cors_setting,
.resource_threads self.origin.clone(),
.send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch)); self.pipeline_id,
} self.referrer.clone(),
// Don't prefetch inside script self.referrer_policy,
self.prefetching = false; integrity_metadata,
}, );
(TagKind::StartTag, local_name!("img")) if self.prefetching => { let _ = self
if let Some(url) = self.get_url(tag, local_name!("src")) { .resource_threads
debug!("Prefetch {} {}", tag.name, url); .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
let request = }
image_fetch_request(url, self.origin.clone(), self.pipeline_id); TokenSinkResult::RawData(RawKind::ScriptData)
let _ = self },
.resource_threads (TagKind::StartTag, local_name!("img")) if self.prefetching => {
.send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch)); if let Some(url) = self.get_url(tag, local_name!("src")) {
} debug!("Prefetch {} {}", tag.name, url);
}, let request = image_fetch_request(url, self.origin.clone(), self.pipeline_id);
(TagKind::StartTag, local_name!("link")) if self.prefetching => { let _ = self
if let Some(rel) = self.get_attr(tag, local_name!("rel")) { .resource_threads
if rel.value.eq_ignore_ascii_case("stylesheet") { .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
if let Some(url) = self.get_url(tag, local_name!("href")) { }
debug!("Prefetch {} {}", tag.name, url); TokenSinkResult::Continue
let cors_setting = },
self.get_cors_settings(tag, local_name!("crossorigin")); (TagKind::StartTag, local_name!("link")) if self.prefetching => {
let integrity_metadata = self if let Some(rel) = self.get_attr(tag, local_name!("rel")) {
.get_attr(tag, local_name!("integrity")) if rel.value.eq_ignore_ascii_case("stylesheet") {
.map(|attr| String::from(&attr.value)) if let Some(url) = self.get_url(tag, local_name!("href")) {
.unwrap_or_default(); debug!("Prefetch {} {}", tag.name, url);
let request = stylesheet_fetch_request( let cors_setting =
url, self.get_cors_settings(tag, local_name!("crossorigin"));
cors_setting, let integrity_metadata = self
self.origin.clone(), .get_attr(tag, local_name!("integrity"))
self.pipeline_id, .map(|attr| String::from(&attr.value))
self.referrer.clone(), .unwrap_or_default();
self.referrer_policy, let request = stylesheet_fetch_request(
integrity_metadata, url,
); cors_setting,
let _ = self self.origin.clone(),
.resource_threads self.pipeline_id,
.send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch)); self.referrer.clone(),
} self.referrer_policy,
integrity_metadata,
);
let _ = self
.resource_threads
.send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
} }
} }
}, }
(TagKind::EndTag, local_name!("script")) => { TokenSinkResult::Continue
// After the first script tag, the main parser is blocked, so it's worth prefetching. },
self.prefetching = true; (TagKind::StartTag, local_name!("script")) => {
}, TokenSinkResult::RawData(RawKind::ScriptData)
(TagKind::StartTag, local_name!("base")) => { },
if let Some(url) = self.get_url(tag, local_name!("href")) { (TagKind::EndTag, local_name!("script")) => {
debug!("Setting base {}", url); // After the first script tag, the main parser is blocked, so it's worth prefetching.
self.base = url; self.prefetching = true;
} TokenSinkResult::Script(PrefetchHandle)
}, },
_ => {}, (TagKind::StartTag, local_name!("base")) => {
} if let Some(url) = self.get_url(tag, local_name!("href")) {
debug!("Setting base {}", url);
self.base = url;
}
TokenSinkResult::Continue
},
_ => TokenSinkResult::Continue,
} }
TokenSinkResult::Continue
} }
} }