Auto merge of #6597 - pcwalton:image-cache-ipc, r=jdm

script: Make the `ImageCacheTask` use IPC.

This necessitated getting rid of the boxed trait object that was being
be passed between the script task and the image cache task.

r? @jdm

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6597)
<!-- Reviewable:end -->

components/layout/context.rs

@@ -15,7 +15,7 @@ use euclid::{Rect, Size2D};
 use gfx::display_list::OpaqueNode;
 use gfx::font_cache_task::FontCacheTask;
 use gfx::font_context::FontContext;
-use ipc_channel::ipc::IpcSender;
+use ipc_channel::ipc::{self, IpcSender};
 use msg::constellation_msg::ConstellationChan;
 use net_traits::image::base::Image;
 use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageResponse, ImageState};
@@ -192,7 +192,7 @@ impl<'a> LayoutContext<'a> {
                     (ImageState::LoadError, _) => None,
                     // Not loaded, test mode - load the image synchronously
                     (_, true) => {
-                        let (sync_tx, sync_rx) = channel();
+                        let (sync_tx, sync_rx) = ipc::channel().unwrap();
                         self.shared.image_cache_task.request_image(url,
                                                                    ImageCacheChan(sync_tx),
                                                                    None);

components/layout/layout_task.rs

@@ -68,7 +68,6 @@ use std::mem::transmute;
 use std::ops::{Deref, DerefMut};
 use std::sync::mpsc::{channel, Sender, Receiver, Select};
 use std::sync::{Arc, Mutex, MutexGuard};
-use std::thread;
 use style::computed_values::{filter, mix_blend_mode};
 use style::media_queries::{MediaType, MediaQueryList, Device};
 use style::selector_matching::Stylist;
@@ -327,16 +326,15 @@ impl LayoutTask {
 
         // Create the channel on which new animations can be sent.
         let (new_animations_sender, new_animations_receiver) = channel();
-        let (image_cache_sender, image_cache_receiver) = channel();
         let (canvas_layers_sender, canvas_layers_receiver) = channel();
 
-        // Start a thread to proxy IPC messages from the layout thread to us.
+        // Proxy IPC messages from the pipeline to the layout thread.
-        let (pipeline_sender, pipeline_receiver) = channel();
+        let pipeline_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(pipeline_port);
-        thread::spawn(move || {
+
-            while let Ok(message) = pipeline_port.recv() {
+        // Ask the router to proxy IPC messages from the image cache task to the layout thread.
-                pipeline_sender.send(message).unwrap()
+        let (ipc_image_cache_sender, ipc_image_cache_receiver) = ipc::channel().unwrap();
-            }
+        let image_cache_receiver =
-        });
+            ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_image_cache_receiver);
 
         LayoutTask {
             id: id,
@@ -354,7 +352,7 @@ impl LayoutTask {
             font_cache_task: font_cache_task,
             first_reflow: Cell::new(true),
             image_cache_receiver: image_cache_receiver,
-            image_cache_sender: ImageCacheChan(image_cache_sender),
+            image_cache_sender: ImageCacheChan(ipc_image_cache_sender),
             canvas_layers_receiver: canvas_layers_receiver,
             canvas_layers_sender: canvas_layers_sender,
             rw_data: Arc::new(Mutex::new(

components/net/image_cache_task.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use ipc_channel::ipc::{self, IpcSender};
+use ipc_channel::router::ROUTER;
 use net_traits::image::base::{Image, load_from_memory};
 use net_traits::image_cache_task::{ImageState, ImageCacheTask, ImageCacheChan, ImageCacheCommand};
 use net_traits::image_cache_task::{ImageCacheResult, ImageResponse, UsePlaceholder};
@@ -69,11 +71,11 @@ impl CompletedLoad {
 /// of an image changes.
 struct ImageListener {
     sender: ImageCacheChan,
-    responder: Option<Box<ImageResponder>>,
+    responder: Option<ImageResponder>,
 }
 
 impl ImageListener {
-    fn new(sender: ImageCacheChan, responder: Option<Box<ImageResponder>>) -> ImageListener {
+    fn new(sender: ImageCacheChan, responder: Option<ImageResponder>) -> ImageListener {
         ImageListener {
             sender: sender,
             responder: responder,
@@ -153,7 +155,7 @@ enum SelectResult {
 
 impl ImageCache {
     fn run(&mut self) {
-        let mut exit_sender: Option<Sender<()>> = None;
+        let mut exit_sender: Option<IpcSender<()>> = None;
 
         loop {
             let result = {
@@ -203,7 +205,7 @@ impl ImageCache {
     }
 
     // Handle a request from a client
-    fn handle_cmd(&mut self, cmd: ImageCacheCommand) -> Option<Sender<()>> {
+    fn handle_cmd(&mut self, cmd: ImageCacheCommand) -> Option<IpcSender<()>> {
         match cmd {
             ImageCacheCommand::Exit(sender) => {
                 return Some(sender);
@@ -303,7 +305,7 @@ impl ImageCache {
     fn request_image(&mut self,
                      url: Url,
                      result_chan: ImageCacheChan,
-                     responder: Option<Box<ImageResponder>>) {
+                     responder: Option<ImageResponder>) {
         let image_listener = ImageListener::new(result_chan, responder);
 
         // Check if already completed
@@ -343,7 +345,7 @@ impl ImageCache {
 
 /// Create a new image cache.
 pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
-    let (cmd_sender, cmd_receiver) = channel();
+    let (ipc_command_sender, ipc_command_receiver) = ipc::channel().unwrap();
     let (progress_sender, progress_receiver) = channel();
     let (decoder_sender, decoder_receiver) = channel();
 
@@ -370,6 +372,9 @@ pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
             }
         };
 
+        // Ask the router to proxy messages received over IPC to us.
+        let cmd_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_command_receiver);
+
         let mut cache = ImageCache {
             cmd_receiver: cmd_receiver,
             progress_sender: progress_sender,
@@ -386,6 +391,6 @@ pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
         cache.run();
     });
 
-    ImageCacheTask::new(cmd_sender)
+    ImageCacheTask::new(ipc_command_sender)
 }
 

components/net_traits/Cargo.toml

@@ -24,12 +24,15 @@ git = "https://github.com/servo/rust-stb-image"
 version = "0.6"
 features = [ "serde-serialization" ]
 
+[dependencies.url]
+version = "0.2.36"
+features = [ "serde_serialization" ]
+
 [dependencies.ipc-channel]
 git = "https://github.com/pcwalton/ipc-channel"
 
 [dependencies]
 log = "0.3"
-url = "0.2.36"
 euclid = "0.1"
 serde = "0.4"
 serde_macros = "0.4"

components/net_traits/image_cache_task.rs

@@ -3,20 +3,33 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use image::base::Image;
+use ipc_channel::ipc::{self, IpcSender};
 use url::Url;
 use std::sync::Arc;
-use std::sync::mpsc::{channel, Sender};
 
 /// This is optionally passed to the image cache when requesting
 /// and image, and returned to the specified event loop when the
 /// image load completes. It is typically used to trigger a reflow
 /// and/or repaint.
-pub trait ImageResponder : Send {
+#[derive(Deserialize, Serialize)]
-    fn respond(&self, ImageResponse);
+pub struct ImageResponder {
+    sender: IpcSender<ImageResponse>,
+}
+
+impl ImageResponder {
+    pub fn new(sender: IpcSender<ImageResponse>) -> ImageResponder {
+        ImageResponder {
+            sender: sender,
+        }
+    }
+
+    pub fn respond(&self, response: ImageResponse) {
+        self.sender.send(response).unwrap()
+    }
 }
 
 /// The current state of an image in the cache.
-#[derive(PartialEq, Copy, Clone)]
+#[derive(PartialEq, Copy, Clone, Deserialize, Serialize)]
 pub enum ImageState {
     Pending,
     LoadError,
@@ -24,7 +37,7 @@ pub enum ImageState {
 }
 
 /// The returned image.
-#[derive(Clone)]
+#[derive(Clone, Deserialize, Serialize)]
 pub enum ImageResponse {
     /// The requested image was loaded.
     Loaded(Arc<Image>),
@@ -35,34 +48,36 @@ pub enum ImageResponse {
 }
 
 /// Channel for sending commands to the image cache.
-#[derive(Clone)]
+#[derive(Clone, Deserialize, Serialize)]
-pub struct ImageCacheChan(pub Sender<ImageCacheResult>);
+pub struct ImageCacheChan(pub IpcSender<ImageCacheResult>);
 
 /// The result of an image cache command that is returned to the
 /// caller.
+#[derive(Deserialize, Serialize)]
 pub struct ImageCacheResult {
-    pub responder: Option<Box<ImageResponder>>,
+    pub responder: Option<ImageResponder>,
     pub image_response: ImageResponse,
 }
 
 /// Commands that the image cache understands.
+#[derive(Deserialize, Serialize)]
 pub enum ImageCacheCommand {
     /// Request an image asynchronously from the cache. Supply a channel
     /// to receive the result, and optionally an image responder
     /// that is passed to the result channel.
-    RequestImage(Url, ImageCacheChan, Option<Box<ImageResponder>>),
+    RequestImage(Url, ImageCacheChan, Option<ImageResponder>),
 
     /// Synchronously check the state of an image in the cache.
     /// TODO(gw): Profile this on some real world sites and see
     /// if it's worth caching the results of this locally in each
     /// layout / paint task.
-    GetImageIfAvailable(Url, UsePlaceholder, Sender<Result<Arc<Image>, ImageState>>),
+    GetImageIfAvailable(Url, UsePlaceholder, IpcSender<Result<Arc<Image>, ImageState>>),
 
     /// Clients must wait for a response before shutting down the ResourceTask
-    Exit(Sender<()>),
+    Exit(IpcSender<()>),
 }
 
-#[derive(Copy, Clone, PartialEq)]
+#[derive(Copy, Clone, PartialEq, Deserialize, Serialize)]
 pub enum UsePlaceholder {
     No,
     Yes,
@@ -70,16 +85,16 @@ pub enum UsePlaceholder {
 
 /// The client side of the image cache task. This can be safely cloned
 /// and passed to different tasks.
-#[derive(Clone)]
+#[derive(Clone, Deserialize, Serialize)]
 pub struct ImageCacheTask {
-    chan: Sender<ImageCacheCommand>,
+    chan: IpcSender<ImageCacheCommand>,
 }
 
 /// The public API for the image cache task.
 impl ImageCacheTask {
 
     /// Construct a new image cache
-    pub fn new(chan: Sender<ImageCacheCommand>) -> ImageCacheTask {
+    pub fn new(chan: IpcSender<ImageCacheCommand>) -> ImageCacheTask {
         ImageCacheTask {
             chan: chan,
         }
@@ -89,7 +104,7 @@ impl ImageCacheTask {
     pub fn request_image(&self,
                          url: Url,
                          result_chan: ImageCacheChan,
-                         responder: Option<Box<ImageResponder>>) {
+                         responder: Option<ImageResponder>) {
         let msg = ImageCacheCommand::RequestImage(url, result_chan, responder);
         self.chan.send(msg).unwrap();
     }
@@ -97,7 +112,7 @@ impl ImageCacheTask {
     /// Get the current state of an image. See ImageCacheCommand::GetImageIfAvailable.
     pub fn get_image_if_available(&self, url: Url, use_placeholder: UsePlaceholder)
                                   -> Result<Arc<Image>, ImageState> {
-        let (sender, receiver) = channel();
+        let (sender, receiver) = ipc::channel().unwrap();
         let msg = ImageCacheCommand::GetImageIfAvailable(url, use_placeholder, sender);
         self.chan.send(msg).unwrap();
         receiver.recv().unwrap()
@@ -105,7 +120,7 @@ impl ImageCacheTask {
 
     /// Shutdown the image cache task.
     pub fn exit(&self) {
-        let (response_chan, response_port) = channel();
+        let (response_chan, response_port) = ipc::channel().unwrap();
         self.chan.send(ImageCacheCommand::Exit(response_chan)).unwrap();
         response_port.recv().unwrap();
     }

components/net_traits/lib.rs

@@ -10,6 +10,8 @@
 #![feature(vec_push_all)]
 #![plugin(serde_macros)]
 
+#![plugin(serde_macros)]
+
 extern crate euclid;
 extern crate hyper;
 extern crate ipc_channel;

components/script/dom/canvasrenderingcontext2d.rs

@@ -28,7 +28,6 @@ use euclid::matrix2d::Matrix2D;
 use euclid::point::Point2D;
 use euclid::rect::Rect;
 use euclid::size::Size2D;
-use ipc_channel::ipc;
 
 use canvas_traits::{CanvasMsg, Canvas2dMsg, CanvasCommonMsg};
 use canvas_traits::{FillOrStrokeStyle, LinearGradientStyle, RadialGradientStyle, RepetitionStyle};
@@ -38,7 +37,7 @@ use msg::constellation_msg::Msg as ConstellationMsg;
 use net_traits::image_cache_task::{ImageCacheChan, ImageResponse};
 use net_traits::image::base::PixelFormat;
 
-use ipc_channel::ipc::IpcSender;
+use ipc_channel::ipc::{self, IpcSender};
 use num::{Float, ToPrimitive};
 use std::borrow::ToOwned;
 use std::cell::RefCell;
@@ -341,7 +340,7 @@ impl CanvasRenderingContext2D {
         let window = window_from_node(canvas.r());
         let window = window.r();
         let image_cache = window.image_cache_task();
-        let (response_chan, response_port) = channel();
+        let (response_chan, response_port) = ipc::channel().unwrap();
         image_cache.request_image(url, ImageCacheChan(response_chan), None);
         let result = response_port.recv().unwrap();
         result.image_response

components/script/dom/htmlimageelement.rs

@@ -23,9 +23,12 @@ use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
 use dom::node::{document_from_node, Node, NodeTypeId, NodeHelpers, NodeDamage, window_from_node};
 use dom::virtualmethods::VirtualMethods;
 use dom::window::WindowHelpers;
+use script_task::{Runnable, ScriptChan, ScriptMsg};
 use util::str::DOMString;
 use string_cache::Atom;
 
+use ipc_channel::ipc;
+use ipc_channel::router::ROUTER;
 use net_traits::image::base::Image;
 use net_traits::image_cache_task::{ImageResponder, ImageResponse};
 use url::{Url, UrlParser};
@@ -62,27 +65,27 @@ trait PrivateHTMLImageElementHelpers {
     fn update_image(self, value: Option<(DOMString, &Url)>);
 }
 
-/// This is passed to the image cache when the src attribute
+struct ImageResponseHandlerRunnable {
-/// changes. It is returned via a message to the script task,
-/// which marks the element as dirty and triggers a reflow.
-struct Responder {
     element: Trusted<HTMLImageElement>,
+    image: ImageResponse,
 }
 
-impl Responder {
+impl ImageResponseHandlerRunnable {
-    fn new(element: Trusted<HTMLImageElement>) -> Responder {
+    fn new(element: Trusted<HTMLImageElement>, image: ImageResponse)
-        Responder {
+           -> ImageResponseHandlerRunnable {
-            element: element
+        ImageResponseHandlerRunnable {
+            element: element,
+            image: image,
         }
     }
 }
 
-impl ImageResponder for Responder {
+impl Runnable for ImageResponseHandlerRunnable {
-    fn respond(&self, image: ImageResponse) {
+    fn handler(self: Box<Self>) {
         // Update the image field
         let element = self.element.root();
         let element_ref = element.r();
-        *element_ref.image.borrow_mut() = match image {
+        *element_ref.image.borrow_mut() = match self.image {
             ImageResponse::Loaded(image) | ImageResponse::PlaceholderLoaded(image) => {
                 Some(image)
             }
@@ -130,8 +133,20 @@ impl<'a> PrivateHTMLImageElementHelpers for &'a HTMLImageElement {
                 *self.url.borrow_mut() = Some(img_url.clone());
 
                 let trusted_node = Trusted::new(window.get_cx(), self, window.script_chan());
-                let responder = box Responder::new(trusted_node);
+                let (responder_sender, responder_receiver) = ipc::channel().unwrap();
-                image_cache.request_image(img_url, window.image_cache_chan(), Some(responder));
+                let script_chan = window.script_chan();
+                ROUTER.add_route(responder_receiver.to_opaque(), box move |message| {
+                    // Return the image via a message to the script task, which marks the element
+                    // as dirty and triggers a reflow.
+                    let image_response = message.to().unwrap();
+                    script_chan.send(ScriptMsg::RunnableMsg(box ImageResponseHandlerRunnable::new(
+                                trusted_node.clone(),
+                                image_response))).unwrap();
+                });
+
+                image_cache.request_image(img_url,
+                                          window.image_cache_chan(),
+                                          Some(ImageResponder::new(responder_sender)));
             }
         }
     }

components/script/script_task.rs

@@ -53,12 +53,12 @@ use webdriver_handlers;
 use devtools_traits::{DevtoolsControlChan, DevtoolsControlPort, DevtoolsPageInfo};
 use devtools_traits::{DevtoolsControlMsg, DevtoolScriptControlMsg};
 use devtools_traits::{TimelineMarker, TimelineMarkerType, TracingMetadata};
-use script_traits::{CompositorEvent, MouseButton};
-use script_traits::CompositorEvent::{ResizeEvent, ClickEvent};
 use script_traits::CompositorEvent::{MouseDownEvent, MouseUpEvent};
 use script_traits::CompositorEvent::{MouseMoveEvent, KeyEvent};
-use script_traits::{NewLayoutInfo, OpaqueScriptLayoutChannel};
+use script_traits::CompositorEvent::{ResizeEvent, ClickEvent};
+use script_traits::{CompositorEvent, MouseButton};
 use script_traits::{ConstellationControlMsg, ScriptControlChan};
+use script_traits::{NewLayoutInfo, OpaqueScriptLayoutChannel};
 use script_traits::{ScriptState, ScriptTaskFactory};
 use msg::compositor_msg::{LayerId, ScriptListener};
 use msg::constellation_msg::{ConstellationChan, FocusType};
@@ -517,14 +517,18 @@ impl ScriptTask {
         }
 
         let (devtools_sender, devtools_receiver) = channel();
-        let (image_cache_channel, image_cache_port) = channel();
+
+        // Ask the router to proxy IPC messages from the image cache task to us.
+        let (ipc_image_cache_channel, ipc_image_cache_port) = ipc::channel().unwrap();
+        let image_cache_port =
+            ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_image_cache_port);
 
         ScriptTask {
             page: DOMRefCell::new(None),
             incomplete_loads: DOMRefCell::new(vec!()),
 
             image_cache_task: image_cache_task,
-            image_cache_channel: ImageCacheChan(image_cache_channel),
+            image_cache_channel: ImageCacheChan(ipc_image_cache_channel),
             image_cache_port: image_cache_port,
 
             resource_task: resource_task,

ports/cef/Cargo.lock

@@ -942,7 +942,7 @@ name = "openssl"
 version = "0.6.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "bitflags 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "bitflags 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "openssl-sys 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",

ports/gonk/Cargo.lock

@@ -859,7 +859,7 @@ name = "openssl"
 version = "0.6.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "bitflags 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "bitflags 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 0.1.12 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "openssl-sys 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)",