Auto merge of #11373 - servo:threadfactory, r=larsbergstrom

Use associated types to improve LayoutThreadFactory and ScriptThreadFactory.

Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data:
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

Either:
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because refactoring

Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11373)
<!-- Reviewable:end -->

components/constellation/constellation.rs

@@ -90,7 +90,7 @@ enum ReadyToSave {
 /// `ScriptThreadFactory` (which in practice are implemented by
 /// `LayoutThread` in the `layout` crate, and `ScriptThread` in
 /// the `script` crate).
-pub struct Constellation<LTF, STF> {
+pub struct Constellation<Message, LTF, STF> {
     /// A channel through which script messages can be sent to this object.
     script_sender: IpcSender<FromScriptMsg>,
 
@@ -167,7 +167,7 @@ pub struct Constellation<LTF, STF> {
     /// A channel through which messages can be sent to the memory profiler.
     mem_profiler_chan: mem::ProfilerChan,
 
-    phantom: PhantomData<(LTF, STF)>,
+    phantom: PhantomData<(Message, LTF, STF)>,
 
     window_size: WindowSizeData,
 
@@ -319,7 +319,10 @@ enum ChildProcess {
     Unsandboxed(process::Child),
 }
 
-impl<LTF: LayoutThreadFactory, STF: ScriptThreadFactory> Constellation<LTF, STF> {
+impl<Message, LTF, STF> Constellation<Message, LTF, STF>
+    where LTF: LayoutThreadFactory<Message=Message>,
+          STF: ScriptThreadFactory<Message=Message>
+{
     pub fn start(state: InitialConstellationState) -> Sender<FromCompositorMsg> {
         let (ipc_script_sender, ipc_script_receiver) = ipc::channel().expect("ipc channel failure");
         let script_receiver = ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_script_receiver);
@@ -334,7 +337,7 @@ impl<LTF: LayoutThreadFactory, STF: ScriptThreadFactory> Constellation<LTF, STF>
         let compositor_sender_clone = compositor_sender.clone();
 
         spawn_named("Constellation".to_owned(), move || {
-            let mut constellation: Constellation<LTF, STF> = Constellation {
+            let mut constellation: Constellation<Message, LTF, STF> = Constellation {
                 script_sender: ipc_script_sender,
                 compositor_sender: compositor_sender_clone,
                 layout_sender: ipc_layout_sender,
@@ -456,7 +459,7 @@ impl<LTF: LayoutThreadFactory, STF: ScriptThreadFactory> Constellation<LTF, STF>
             if opts::multiprocess() {
                 self.spawn_multiprocess(pipeline_id, unprivileged_pipeline_content);
             } else {
-                unprivileged_pipeline_content.start_all::<LTF, STF>(false);
+                unprivileged_pipeline_content.start_all::<Message, LTF, STF>(false);
             }
         }
 

components/constellation/pipeline.rs

@@ -407,13 +407,11 @@ pub struct UnprivilegedPipelineContent {
 }
 
 impl UnprivilegedPipelineContent {
-    pub fn start_all<LTF, STF>(mut self, wait_for_completion: bool)
+    pub fn start_all<Message, LTF, STF>(mut self, wait_for_completion: bool)
-        where LTF: LayoutThreadFactory,
+        where LTF: LayoutThreadFactory<Message=Message>,
-              STF: ScriptThreadFactory
+              STF: ScriptThreadFactory<Message=Message>
     {
-        let layout_pair = STF::create_layout_channel();
+        let layout_pair = STF::create(InitialScriptState {
-
-        STF::create(InitialScriptState {
             id: self.id,
             parent_info: self.parent_info,
             compositor: self.script_to_compositor_chan,
@@ -432,7 +430,7 @@ impl UnprivilegedPipelineContent {
             window_size: self.window_size,
             pipeline_namespace_id: self.pipeline_namespace_id,
             content_process_shutdown_chan: self.script_content_process_shutdown_chan.clone(),
-        }, &layout_pair, self.load_data.clone());
+        }, self.load_data.clone());
 
         LTF::create(self.id,
                     self.load_data.url.clone(),

components/layout/layout_thread.rs

@@ -50,11 +50,10 @@ use query::{process_node_overflow_request, process_resolved_style_request, proce
 use query::{process_offset_parent_query};
 use script::dom::node::OpaqueStyleAndLayoutData;
 use script::layout_interface::{LayoutRPC, OffsetParentResponse, NodeOverflowResponse, MarginStyleResponse};
-use script::layout_interface::{Msg, NewLayoutThreadInfo, Reflow, ReflowQueryType};
+use script::layout_interface::{Msg, NewLayoutThreadInfo, Reflow, ReflowQueryType, ScriptReflow};
-use script::layout_interface::{ScriptLayoutChan, ScriptReflow};
 use script::reporter::CSSErrorReporter;
 use script_traits::ConstellationControlMsg;
-use script_traits::{LayoutControlMsg, LayoutMsg as ConstellationMsg, OpaqueScriptLayoutChannel};
+use script_traits::{LayoutControlMsg, LayoutMsg as ConstellationMsg};
 use sequential;
 use serde_json;
 use std::borrow::ToOwned;
@@ -246,11 +245,13 @@ pub struct LayoutThread {
 }
 
 impl LayoutThreadFactory for LayoutThread {
+    type Message = Msg;
+
     /// Spawns a new layout thread.
     fn create(id: PipelineId,
               url: Url,
               is_iframe: bool,
-              chan: OpaqueScriptLayoutChannel,
+              chan: (Sender<Msg>, Receiver<Msg>),
               pipeline_port: IpcReceiver<LayoutControlMsg>,
               constellation_chan: IpcSender<ConstellationMsg>,
               panic_chan: IpcSender<PanicMsg>,
@@ -267,11 +268,11 @@ impl LayoutThreadFactory for LayoutThread {
                                                thread_state::LAYOUT,
                                                move || {
             { // Ensures layout thread is destroyed before we send shutdown message
-                let sender = chan.sender();
+                let sender = chan.0;
                 let layout = LayoutThread::new(id,
                                              url,
                                              is_iframe,
-                                             chan.receiver(),
+                                             chan.1,
                                              pipeline_port,
                                              constellation_chan,
                                              script_chan,

components/layout_traits/lib.rs

@@ -30,7 +30,8 @@ use msg::constellation_msg::{PanicMsg, PipelineId, PipelineNamespaceId, Pipeline
 use net_traits::image_cache_thread::ImageCacheThread;
 use profile_traits::{mem, time};
 use script_traits::LayoutMsg as ConstellationMsg;
-use script_traits::{LayoutControlMsg, ConstellationControlMsg, OpaqueScriptLayoutChannel};
+use script_traits::{LayoutControlMsg, ConstellationControlMsg};
+use std::sync::mpsc::{Sender, Receiver};
 use url::Url;
 use util::ipc::OptionalIpcSender;
 
@@ -41,10 +42,11 @@ pub struct LayoutControlChan(pub IpcSender<LayoutControlMsg>);
 // A static method creating a layout thread
 // Here to remove the compositor -> layout dependency
 pub trait LayoutThreadFactory {
+    type Message;
     fn create(id: PipelineId,
               url: Url,
               is_iframe: bool,
-              chan: OpaqueScriptLayoutChannel,
+              chan: (Sender<Self::Message>, Receiver<Self::Message>),
               pipeline_port: IpcReceiver<LayoutControlMsg>,
               constellation_chan: IpcSender<ConstellationMsg>,
               panic_chan: IpcSender<PanicMsg>,

components/script/layout_interface.rs

@@ -16,8 +16,7 @@ use msg::constellation_msg::{PanicMsg, PipelineId, WindowSizeData};
 use net_traits::image_cache_thread::ImageCacheThread;
 use profile_traits::mem::ReportsChan;
 use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg};
-use script_traits::{OpaqueScriptLayoutChannel, UntrustedNodeAddress};
+use script_traits::{UntrustedNodeAddress};
-use std::any::Any;
 use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender, channel};
 use string_cache::Atom;
@@ -230,36 +229,11 @@ impl LayoutChan {
     }
 }
 
-/// A trait to manage opaque references to script<->layout channels without needing
-/// to expose the message type to crates that don't need to know about them.
-pub trait ScriptLayoutChan {
-    fn new(sender: Sender<Msg>, receiver: Receiver<Msg>) -> Self;
-    fn sender(&self) -> Sender<Msg>;
-    fn receiver(self) -> Receiver<Msg>;
-}
-
-impl ScriptLayoutChan for OpaqueScriptLayoutChannel {
-    fn new(sender: Sender<Msg>, receiver: Receiver<Msg>) -> OpaqueScriptLayoutChannel {
-        let inner = (box sender as Box<Any + Send>, box receiver as Box<Any + Send>);
-        OpaqueScriptLayoutChannel(inner)
-    }
-
-    fn sender(&self) -> Sender<Msg> {
-        let &OpaqueScriptLayoutChannel((ref sender, _)) = self;
-        (*sender.downcast_ref::<Sender<Msg>>().unwrap()).clone()
-    }
-
-    fn receiver(self) -> Receiver<Msg> {
-        let OpaqueScriptLayoutChannel((_, receiver)) = self;
-        *receiver.downcast::<Receiver<Msg>>().unwrap()
-    }
-}
-
 pub struct NewLayoutThreadInfo {
     pub id: PipelineId,
     pub url: Url,
     pub is_parent: bool,
-    pub layout_pair: OpaqueScriptLayoutChannel,
+    pub layout_pair: (Sender<Msg>, Receiver<Msg>),
     pub pipeline_port: IpcReceiver<LayoutControlMsg>,
     pub constellation_chan: IpcSender<ConstellationMsg>,
     pub panic_chan: IpcSender<PanicMsg>,

components/script/script_thread.rs

@@ -58,7 +58,7 @@ use js::jsapi::{JSContext, JS_SetWrapObjectCallbacks, JSTracer, SetWindowProxyCl
 use js::jsval::UndefinedValue;
 use js::rust::Runtime;
 use layout_interface::{ReflowQueryType};
-use layout_interface::{self, LayoutChan, NewLayoutThreadInfo, ScriptLayoutChan};
+use layout_interface::{self, LayoutChan, NewLayoutThreadInfo};
 use mem::heap_size_of_self_and_children;
 use msg::constellation_msg::{LoadData, PanicMsg, PipelineId, PipelineNamespace};
 use msg::constellation_msg::{SubpageId, WindowSizeData, WindowSizeType};
@@ -80,10 +80,9 @@ use script_traits::CompositorEvent::{KeyEvent, MouseButtonEvent, MouseMoveEvent,
 use script_traits::CompositorEvent::{TouchEvent, TouchpadPressureEvent};
 use script_traits::{CompositorEvent, ConstellationControlMsg, EventResult};
 use script_traits::{InitialScriptState, MouseButton, MouseEventType, MozBrowserEvent, NewLayoutInfo};
-use script_traits::{LayoutMsg, OpaqueScriptLayoutChannel, ScriptMsg as ConstellationMsg};
+use script_traits::{LayoutMsg, ScriptMsg as ConstellationMsg};
 use script_traits::{ScriptThreadFactory, ScriptToCompositorMsg, TimerEvent, TimerEventRequest, TimerSource};
 use script_traits::{TouchEventType, TouchId};
-use std::any::Any;
 use std::borrow::ToOwned;
 use std::cell::{Cell, RefCell};
 use std::collections::HashSet;
@@ -428,22 +427,16 @@ impl<'a> Drop for ScriptMemoryFailsafe<'a> {
 }
 
 impl ScriptThreadFactory for ScriptThread {
-    fn create_layout_channel() -> OpaqueScriptLayoutChannel {
+    type Message = layout_interface::Msg;
-        let (chan, port) = channel();
-        ScriptLayoutChan::new(chan, port)
-    }
-
-    fn clone_layout_channel(pair: &OpaqueScriptLayoutChannel)
-                            -> Box<Any + Send> {
-        box pair.sender() as Box<Any + Send>
-    }
 
     fn create(state: InitialScriptState,
-              layout_chan: &OpaqueScriptLayoutChannel,
+              load_data: LoadData)
-              load_data: LoadData) {
+              -> (Sender<layout_interface::Msg>, Receiver<layout_interface::Msg>) {
         let panic_chan = state.panic_chan.clone();
         let (script_chan, script_port) = channel();
-        let layout_chan = LayoutChan(layout_chan.sender());
+
+        let (sender, receiver) = channel();
+        let layout_chan = LayoutChan(sender.clone());
         let pipeline_id = state.id;
         thread::spawn_named_with_send_on_panic(format!("ScriptThread {:?}", state.id),
                                                thread_state::SCRIPT,
@@ -479,6 +472,8 @@ impl ScriptThreadFactory for ScriptThread {
             // This must always be the very last operation performed before the thread completes
             failsafe.neuter();
         }, Some(pipeline_id), panic_chan);
+
+        (sender, receiver)
     }
 }
 
@@ -1103,9 +1098,8 @@ impl ScriptThread {
             content_process_shutdown_chan,
         } = new_layout_info;
 
-        let layout_pair = ScriptThread::create_layout_channel();
+        let layout_pair = channel();
-        let layout_chan = LayoutChan(*ScriptThread::clone_layout_channel(
+        let layout_chan = LayoutChan(layout_pair.0.clone());
-            &layout_pair).downcast::<Sender<layout_interface::Msg>>().unwrap());
 
         let layout_creation_info = NewLayoutThreadInfo {
             id: new_pipeline_id,

components/script_traits/lib.rs

@@ -53,6 +53,7 @@ use net_traits::response::HttpsState;
 use profile_traits::mem;
 use std::any::Any;
 use std::collections::HashMap;
+use std::sync::mpsc::{Sender, Receiver};
 use url::Url;
 use util::ipc::OptionalOpaqueIpcSender;
 
@@ -246,10 +247,6 @@ pub enum TouchpadPressurePhase {
     AfterSecondClick,
 }
 
-/// An opaque wrapper around script<->layout channels to avoid leaking message types into
-/// crates that don't need to know about them.
-pub struct OpaqueScriptLayoutChannel(pub (Box<Any + Send>, Box<Any + Send>));
-
 /// Requests a TimerEvent-Message be sent after the given duration.
 #[derive(Deserialize, Serialize)]
 pub struct TimerEventRequest(pub IpcSender<TimerEvent>,
@@ -344,14 +341,12 @@ pub struct InitialScriptState {
 /// This trait allows creating a `ScriptThread` without depending on the `script`
 /// crate.
 pub trait ScriptThreadFactory {
+    /// Type of message sent from script to layout.
+    type Message;
     /// Create a `ScriptThread`.
     fn create(state: InitialScriptState,
-              layout_chan: &OpaqueScriptLayoutChannel,
+              load_data: LoadData)
-              load_data: LoadData);
+              -> (Sender<Self::Message>, Receiver<Self::Message>);
-    /// Create a script -> layout channel (`Sender`, `Receiver` pair).
-    fn create_layout_channel() -> OpaqueScriptLayoutChannel;
-    /// Clone the `Sender` in `pair`.
-    fn clone_layout_channel(pair: &OpaqueScriptLayoutChannel) -> Box<Any + Send>;
 }
 
 /// Messages sent from the script thread to the compositor

components/servo/lib.rs

@@ -231,7 +231,8 @@ fn create_constellation(opts: opts::Opts,
         webrender_api_sender: webrender_api_sender,
     };
     let constellation_chan =
-        Constellation::<layout::layout_thread::LayoutThread,
+        Constellation::<script::layout_interface::Msg,
+                        layout::layout_thread::LayoutThread,
                         script::script_thread::ScriptThread>::start(initial_state);
 
     // Send the URL command to the constellation.
@@ -264,7 +265,8 @@ pub fn run_content_process(token: String) {
 
     script::init();
 
-    unprivileged_content.start_all::<layout::layout_thread::LayoutThread,
+    unprivileged_content.start_all::<script::layout_interface::Msg,
+                                     layout::layout_thread::LayoutThread,
                                      script::script_thread::ScriptThread>(true);
 }