Auto merge of #15771 - jdm:img-panic, r=nox

Improve behaviour of image elements that perform multiple requests

This addresses cases where image elements end up making multiple requests, as well as makes the element respond to additional relevant mutations that trigger updating the image data.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #15709 (github issue number if applicable).
- [X] There are tests for these changes

<!-- 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/15771)
<!-- Reviewable:end -->

components/script/dom/htmlimageelement.rs

@@ -47,6 +47,8 @@ use network_listener::{NetworkListener, PreInvoke};
 use num_traits::ToPrimitive;
 use script_thread::Runnable;
 use servo_url::ServoUrl;
+use std::cell::Cell;
+use std::default::Default;
 use std::i32;
 use std::sync::{Arc, Mutex};
 use style::attr::{AttrValue, LengthOrPercentageOrAuto};
@@ -76,6 +78,7 @@ pub struct HTMLImageElement {
     htmlelement: HTMLElement,
     current_request: DOMRefCell<ImageRequest>,
     pending_request: DOMRefCell<ImageRequest>,
+    generation: Cell<u32>,
 }
 
 impl HTMLImageElement {
@@ -87,14 +90,16 @@ impl HTMLImageElement {
 struct ImageResponseHandlerRunnable {
     element: Trusted<HTMLImageElement>,
     image: ImageResponse,
+    generation: u32,
 }
 
 impl ImageResponseHandlerRunnable {
-    fn new(element: Trusted<HTMLImageElement>, image: ImageResponse)
+    fn new(element: Trusted<HTMLImageElement>, image: ImageResponse, generation: u32)
            -> ImageResponseHandlerRunnable {
         ImageResponseHandlerRunnable {
             element: element,
             image: image,
+            generation: generation,
         }
     }
 }
@@ -104,35 +109,29 @@ impl Runnable for ImageResponseHandlerRunnable {
 
     fn handler(self: Box<Self>) {
         let element = self.element.root();
-        element.process_image_response(self.image);
+        // Ignore any image response for a previous request that has been discarded.
+        if element.generation.get() == self.generation {
+            element.process_image_response(self.image);
+        }
     }
 }
 
 /// The context required for asynchronously loading an external image.
 struct ImageContext {
-    /// The element that initiated the request.
-    elem: Trusted<HTMLImageElement>,
-    /// The initial URL requested.
-    url: ServoUrl,
+    /// A handle with which to communicate with the image cache.
+    image_cache: ImageCacheThread,
     /// Indicates whether the request failed, and why
     status: Result<(), NetworkError>,
     /// The cache ID for this request.
     id: PendingImageId,
 }
 
-impl ImageContext {
-    fn image_cache(&self) -> ImageCacheThread {
-        let elem = self.elem.root();
-        window_from_node(&*elem).image_cache_thread().clone()
-    }
-}
-
 impl FetchResponseListener for ImageContext {
     fn process_request_body(&mut self) {}
     fn process_request_eof(&mut self) {}
 
     fn process_response(&mut self, metadata: Result<FetchMetadata, NetworkError>) {
-        self.image_cache().notify_pending_response(
+        self.image_cache.notify_pending_response(
             self.id,
             FetchResponseMsg::ProcessResponse(metadata.clone()));
 
@@ -156,19 +155,16 @@ impl FetchResponseListener for ImageContext {
 
     fn process_response_chunk(&mut self, payload: Vec<u8>) {
         if self.status.is_ok() {
-            self.image_cache().notify_pending_response(
+            self.image_cache.notify_pending_response(
                 self.id,
                 FetchResponseMsg::ProcessResponseChunk(payload));
         }
     }
 
     fn process_response_eof(&mut self, response: Result<(), NetworkError>) {
-        let elem = self.elem.root();
-        let document = document_from_node(&*elem);
-        let image_cache = self.image_cache();
-        image_cache.notify_pending_response(self.id,
-                                            FetchResponseMsg::ProcessResponseEOF(response));
-        document.finish_load(LoadType::Image(self.url.clone()));
+        self.image_cache.notify_pending_response(
+            self.id,
+            FetchResponseMsg::ProcessResponseEOF(response));
     }
 }
 
@@ -197,11 +193,12 @@ impl HTMLImageElement {
             let window = window_from_node(elem);
             let task_source = window.networking_task_source();
             let wrapper = window.get_runnable_wrapper();
+            let generation = elem.generation.get();
             ROUTER.add_route(responder_receiver.to_opaque(), box move |message| {
                 // Return the image via a message to the script thread, which marks the element
                 // as dirty and triggers a reflow.
                 let runnable = ImageResponseHandlerRunnable::new(
-                    trusted_node.clone(), message.to().unwrap());
+                    trusted_node.clone(), message.to().unwrap(), generation);
                 let _ = task_source.queue_with_wrapper(box runnable, &wrapper);
             });
 
@@ -243,8 +240,7 @@ impl HTMLImageElement {
         let window = window_from_node(self);
 
         let context = Arc::new(Mutex::new(ImageContext {
-            elem: Trusted::new(self),
-            url: img_url.clone(),
+            image_cache: window.image_cache_thread().clone(),
             status: Ok(()),
             id: id,
         }));
@@ -267,7 +263,9 @@ impl HTMLImageElement {
             .. RequestInit::default()
         };
 
-        document.fetch_async(LoadType::Image(img_url), request, action_sender);
+        // This is a background load because the load blocker already fulfills the
+        // purpose of delaying the document's load event.
+        document.loader().fetch_async_background(request, action_sender);
     }
 
     fn process_image_response(&self, image: ImageResponse) {
@@ -299,7 +297,9 @@ impl HTMLImageElement {
             self.upcast::<EventTarget>().fire_event(atom!("error"));
         }
 
-        LoadBlocker::terminate(&mut self.current_request.borrow_mut().blocker);
+        if trigger_image_load || trigger_image_error {
+            LoadBlocker::terminate(&mut self.current_request.borrow_mut().blocker);
+        }
 
         // Trigger reflow
         let window = window_from_node(self);
@@ -309,6 +309,9 @@ impl HTMLImageElement {
     /// Makes the local `image` member match the status of the `src` attribute and starts
     /// prefetching the image. This method must be called after `src` is changed.
     fn update_image(&self, value: Option<(DOMString, ServoUrl)>) {
+        // Force any in-progress request to be ignored.
+        self.generation.set(self.generation.get() + 1);
+
         let document = document_from_node(self);
         let window = document.window();
         match value {
@@ -381,6 +384,7 @@ impl HTMLImageElement {
                 metadata: None,
                 blocker: None,
             }),
+            generation: Default::default(),
         }
     }
 
@@ -622,6 +626,15 @@ impl VirtualMethods for HTMLImageElement {
         Some(self.upcast::<HTMLElement>() as &VirtualMethods)
     }
 
+    fn adopting_steps(&self, old_doc: &Document) {
+        self.super_type().unwrap().adopting_steps(old_doc);
+
+        let elem = self.upcast::<Element>();
+        let document = document_from_node(self);
+        self.update_image(Some((elem.get_string_attribute(&local_name!("src")),
+                                document.base_url())));
+    }
+
     fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
         self.super_type().unwrap().attribute_mutated(attr, mutation);
         match attr.local_name() {