Responding to review comments

4 files changed, 127 insertions, 105 deletions

diff --git a/components/net/resource_thread.rs b/components/net/resource_thread.rs
index 2986686b221..6b7ac04b891 100644
--- a/components/net/resource_thread.rs
+++ b/components/net/resource_thread.rs
@@ -25,6 +25,7 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use net_traits::request::{Destination, RequestBuilder};
 use net_traits::response::{Response, ResponseInit};
 use net_traits::storage_thread::StorageThreadMsg;
+use net_traits::DiscardFetch;
 use net_traits::FetchTaskTarget;
 use net_traits::WebSocketNetworkEvent;
 use net_traits::{CookieSource, CoreResourceMsg, CoreResourceThread};
@@ -248,7 +249,7 @@ impl ResourceChannelManager {
                 ),
                 FetchChannels::Prefetch => {
                     self.resource_manager
-                        .fetch(req_init, None, (), http_state, None)
+                        .fetch(req_init, None, DiscardFetch, http_state, None)
                 },
             },
             CoreResourceMsg::DeleteCookies(request) => {
diff --git a/components/net_traits/lib.rs b/components/net_traits/lib.rs
index df06cb74eb3..369c1efedaa 100644
--- a/components/net_traits/lib.rs
+++ b/components/net_traits/lib.rs
@@ -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_eof(&mut self, _: &Request) {}
diff --git a/components/script/dom/servoparser/mod.rs b/components/script/dom/servoparser/mod.rs
index a90905b105a..a05dea2a66d 100644
--- a/components/script/dom/servoparser/mod.rs
+++ b/components/script/dom/servoparser/mod.rs
@@ -102,7 +102,9 @@ pub struct ServoParser {
     aborted: Cell<bool>,
     /// <https://html.spec.whatwg.org/multipage/#script-created-parser>
     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>,
     #[ignore_malloc_size_of = "Defined in html5ever"]
     prefetch_input: DomRefCell<BufferQueue>,
@@ -433,30 +435,31 @@ impl ServoParser {
     }
 
     fn push_tendril_input_chunk(&self, chunk: StrTendril) {
-        if !chunk.is_empty() {
-            // 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());
-                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);
+        if chunk.is_empty() {
+            return;
+        }
+        // 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>) {
@@ -471,7 +474,6 @@ impl ServoParser {
     }
 
     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
         // to be decoded by the network decoder again.
         let chunk = StrTendril::from(chunk);
diff --git a/components/script/dom/servoparser/prefetch.rs b/components/script/dom/servoparser/prefetch.rs
index 314cb3b97a1..b249c695ab2 100644
--- a/components/script/dom/servoparser/prefetch.rs
+++ b/components/script/dom/servoparser/prefetch.rs
@@ -9,6 +9,7 @@ use crate::dom::htmlimageelement::image_fetch_request;
 use crate::dom::htmlscriptelement::script_fetch_request;
 use crate::stylesheet_loader::stylesheet_fetch_request;
 use html5ever::buffer_queue::BufferQueue;
+use html5ever::tokenizer::states::RawKind;
 use html5ever::tokenizer::Tag;
 use html5ever::tokenizer::TagKind;
 use html5ever::tokenizer::Token;
@@ -63,8 +64,8 @@ impl Tokenizer {
         Tokenizer { inner }
     }
 
-    pub fn feed(&mut self, input: &mut BufferQueue) -> TokenizerResult<()> {
-        self.inner.feed(input)
+    pub fn feed(&mut self, input: &mut BufferQueue) {
+        while let TokenizerResult::Script(PrefetchHandle) = self.inner.feed(input) {}
     }
 }
 
@@ -79,86 +80,99 @@ struct PrefetchSink {
     prefetching: bool,
 }
 
+/// The prefetch tokenizer produces trivial results
+struct PrefetchHandle;
+
 impl TokenSink for PrefetchSink {
-    type Handle = ();
-    fn process_token(&mut self, token: Token, _line_number: u64) -> TokenSinkResult<()> {
-        if let Token::TagToken(ref tag) = token {
-            match (tag.kind, &tag.name) {
-                (TagKind::StartTag, local_name!("script")) if self.prefetching => {
-                    if let Some(url) = self.get_url(tag, local_name!("src")) {
-                        debug!("Prefetch script {}", url);
-                        let cors_setting = self.get_cors_settings(tag, local_name!("crossorigin"));
-                        let integrity_metadata = self
-                            .get_attr(tag, local_name!("integrity"))
-                            .map(|attr| String::from(&attr.value))
-                            .unwrap_or_default();
-                        let request = script_fetch_request(
-                            url,
-                            cors_setting,
-                            self.origin.clone(),
-                            self.pipeline_id,
-                            self.referrer.clone(),
-                            self.referrer_policy,
-                            integrity_metadata,
-                        );
-                        let _ = self
-                            .resource_threads
-                            .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
-                    }
-                    // Don't prefetch inside script
-                    self.prefetching = false;
-                },
-                (TagKind::StartTag, local_name!("img")) if self.prefetching => {
-                    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);
-                        let _ = self
-                            .resource_threads
-                            .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
-                    }
-                },
-                (TagKind::StartTag, local_name!("link")) if self.prefetching => {
-                    if let Some(rel) = self.get_attr(tag, local_name!("rel")) {
-                        if rel.value.eq_ignore_ascii_case("stylesheet") {
-                            if let Some(url) = self.get_url(tag, local_name!("href")) {
-                                debug!("Prefetch {} {}", tag.name, url);
-                                let cors_setting =
-                                    self.get_cors_settings(tag, local_name!("crossorigin"));
-                                let integrity_metadata = self
-                                    .get_attr(tag, local_name!("integrity"))
-                                    .map(|attr| String::from(&attr.value))
-                                    .unwrap_or_default();
-                                let request = stylesheet_fetch_request(
-                                    url,
-                                    cors_setting,
-                                    self.origin.clone(),
-                                    self.pipeline_id,
-                                    self.referrer.clone(),
-                                    self.referrer_policy,
-                                    integrity_metadata,
-                                );
-                                let _ = self
-                                    .resource_threads
-                                    .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
-                            }
+    type Handle = PrefetchHandle;
+    fn process_token(
+        &mut self,
+        token: Token,
+        _line_number: u64,
+    ) -> TokenSinkResult<PrefetchHandle> {
+        let tag = match token {
+            Token::TagToken(ref tag) => tag,
+            _ => return TokenSinkResult::Continue,
+        };
+        match (tag.kind, &tag.name) {
+            (TagKind::StartTag, local_name!("script")) if self.prefetching => {
+                if let Some(url) = self.get_url(tag, local_name!("src")) {
+                    debug!("Prefetch script {}", url);
+                    let cors_setting = self.get_cors_settings(tag, local_name!("crossorigin"));
+                    let integrity_metadata = self
+                        .get_attr(tag, local_name!("integrity"))
+                        .map(|attr| String::from(&attr.value))
+                        .unwrap_or_default();
+                    let request = script_fetch_request(
+                        url,
+                        cors_setting,
+                        self.origin.clone(),
+                        self.pipeline_id,
+                        self.referrer.clone(),
+                        self.referrer_policy,
+                        integrity_metadata,
+                    );
+                    let _ = self
+                        .resource_threads
+                        .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
+                }
+                TokenSinkResult::RawData(RawKind::ScriptData)
+            },
+            (TagKind::StartTag, local_name!("img")) if self.prefetching => {
+                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);
+                    let _ = self
+                        .resource_threads
+                        .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
+                }
+                TokenSinkResult::Continue
+            },
+            (TagKind::StartTag, local_name!("link")) if self.prefetching => {
+                if let Some(rel) = self.get_attr(tag, local_name!("rel")) {
+                    if rel.value.eq_ignore_ascii_case("stylesheet") {
+                        if let Some(url) = self.get_url(tag, local_name!("href")) {
+                            debug!("Prefetch {} {}", tag.name, url);
+                            let cors_setting =
+                                self.get_cors_settings(tag, local_name!("crossorigin"));
+                            let integrity_metadata = self
+                                .get_attr(tag, local_name!("integrity"))
+                                .map(|attr| String::from(&attr.value))
+                                .unwrap_or_default();
+                            let request = stylesheet_fetch_request(
+                                url,
+                                cors_setting,
+                                self.origin.clone(),
+                                self.pipeline_id,
+                                self.referrer.clone(),
+                                self.referrer_policy,
+                                integrity_metadata,
+                            );
+                            let _ = self
+                                .resource_threads
+                                .send(CoreResourceMsg::Fetch(request, FetchChannels::Prefetch));
                         }
                     }
-                },
-                (TagKind::EndTag, local_name!("script")) => {
-                    // After the first script tag, the main parser is blocked, so it's worth prefetching.
-                    self.prefetching = true;
-                },
-                (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
+            },
+            (TagKind::StartTag, local_name!("script")) => {
+                TokenSinkResult::RawData(RawKind::ScriptData)
+            },
+            (TagKind::EndTag, local_name!("script")) => {
+                // After the first script tag, the main parser is blocked, so it's worth prefetching.
+                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
     }
 }