Bug 1612437 - Remove ipc-channel from WebRender. r=gw

Original patch from Josh Mattews

Differential Revision: https://phabricator.services.mozilla.com/D61606

[ghsync] From https://hg.mozilla.org/mozilla-central/rev/814b4d05ad3edf0821a4d788b3c7404b4636e408

ci-scripts/linux-debug-tests.sh

@@ -15,10 +15,6 @@ set -o xtrace
 
 CARGOFLAGS=${CARGOFLAGS:-"--verbose"}  # default to --verbose if not set
 
-pushd webrender_api
-cargo test ${CARGOFLAGS} --features "ipc"
-popd
-
 pushd webrender
 cargo build ${CARGOFLAGS} --no-default-features
 cargo build ${CARGOFLAGS} --no-default-features --features capture

ci-scripts/macos-debug-tests.sh

@@ -22,10 +22,6 @@ set -o xtrace
 CARGOFLAGS=${CARGOFLAGS:-"--verbose"}  # default to --verbose if not set
 CARGOTESTFLAGS=${CARGOTESTFLAGS:-""}
 
-pushd webrender_api
-cargo check ${CARGOFLAGS} --features "ipc"
-popd
-
 pushd webrender
 cargo check ${CARGOFLAGS} --no-default-features
 cargo check ${CARGOFLAGS} --no-default-features --features capture

servo-tidy.toml

@@ -8,7 +8,6 @@ check-alphabetical-order = false
 packages = [
     "core-graphics",
     "core-text",
-    "crossbeam-utils",
     "gl_generator",
     "lazy_static",
     "percent-encoding",

webrender/src/renderer.rs

@@ -45,7 +45,7 @@ use api::ExternalImage;
 use api::channel;
 use api::units::*;
 pub use api::DebugFlags;
-use api::channel::{MsgSender, PayloadReceiverHelperMethods};
+use api::channel::MsgSender;
 use crate::batch::{AlphaBatchContainer, BatchKind, BatchFeatures, BatchTextures, BrushBatchKind, ClipBatchList};
 #[cfg(any(feature = "capture", feature = "replay"))]
 use crate::capture::{CaptureConfig, ExternalCaptureImage, PlainExternalImage};

webrender_api/Cargo.toml

@@ -9,7 +9,6 @@ edition = "2018"
 
 [features]
 nightly = ["euclid/unstable", "serde/unstable"]
-ipc = ["ipc-channel"]
 serialize = []
 deserialize = []
 display_list_stats = []
@@ -19,7 +18,6 @@ app_units = "0.7"
 bitflags = "1.2"
 byteorder = "1.2.1"
 derive_more = "0.99"
-ipc-channel = {version = "0.12.0", optional = true}
 euclid = { version = "0.20.0", features = ["serde"] }
 malloc_size_of_derive = "0.1"
 serde = { version = "1.0", features = ["rc"] }

webrender_api/src/api.rs

@@ -6,7 +6,7 @@
 
 extern crate serde_bytes;
 
-use crate::channel::{self, MsgSender, Payload, PayloadSender, PayloadSenderHelperMethods};
+use crate::channel::{self, MsgSender, Payload, PayloadSender};
 use peek_poke::PeekPoke;
 use std::cell::Cell;
 use std::fmt;
@@ -1601,7 +1601,7 @@ impl RenderApi {
     #[doc(hidden)]
     pub fn send_payload(&self, data: &[u8]) {
         self.payload_sender
-            .send_payload(Payload::from_data(data))
+            .send(Payload::from_data(data))
             .unwrap();
     }
 
@@ -1629,7 +1629,7 @@ impl RenderApi {
     pub fn send_transaction(&self, document_id: DocumentId, transaction: Transaction) {
         let (msg, payloads) = transaction.finalize();
         for payload in payloads {
-            self.payload_sender.send_payload(payload).unwrap();
+            self.payload_sender.send(payload).unwrap();
         }
         self.api_sender.send(ApiMsg::UpdateDocuments(vec![document_id], vec![msg])).unwrap();
     }
@@ -1647,7 +1647,7 @@ impl RenderApi {
                     (msgs, document_payloads)
                 });
         for payload in document_payloads.drain(..).flatten() {
-            self.payload_sender.send_payload(payload).unwrap();
+            self.payload_sender.send(payload).unwrap();
         }
         self.api_sender.send(ApiMsg::UpdateDocuments(document_ids, msgs)).unwrap();
     }

webrender_api/src/channel.rs

@@ -4,9 +4,10 @@
 
 use crate::api::{Epoch, PipelineId};
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
-use std::io::{Cursor, Read};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use std::io::{self, Cursor, Error, ErrorKind, Read};
 use std::mem;
-use std::sync::mpsc::Receiver;
+use std::sync::mpsc;
 
 #[derive(Clone)]
 pub struct Payload {
@@ -39,9 +40,6 @@ impl Payload {
     }
     /// Convert the payload to a raw byte vector, in order for it to be
     /// efficiently shared via shmem, for example.
-    ///
-    /// TODO(emilio, #1049): Consider moving the IPC boundary to the
-    /// constellation in Servo and remove this complexity from WR.
     pub fn to_data(&self) -> Vec<u8> {
         Self::construct_data(self.epoch, self.pipeline_id, &self.display_list_data)
     }
@@ -71,23 +69,64 @@ impl Payload {
     }
 }
 
+pub type PayloadSender = MsgSender<Payload>;
+
+pub type PayloadReceiver = MsgReceiver<Payload>;
 
-/// A helper to handle the interface difference between `IpcBytesSender`
-/// and `Sender<Vec<u8>>`.
-pub trait PayloadSenderHelperMethods {
-    fn send_payload(&self, data: Payload) -> Result<(), Error>;
+pub struct MsgReceiver<T> {
+    rx: mpsc::Receiver<T>,
 }
 
-pub trait PayloadReceiverHelperMethods {
-    fn recv_payload(&self) -> Result<Payload, Error>;
+impl<T> MsgReceiver<T> {
+    pub fn recv(&self) -> Result<T, Error> {
+        use std::error::Error;
+        self.rx.recv().map_err(|e| io::Error::new(ErrorKind::Other, e.description()))
+    }
+
+    pub fn to_mpsc_receiver(self) -> mpsc::Receiver<T> {
+        self.rx
+    }
+}
 
-    // For an MPSC receiver, this is the identity function,
-    // for an IPC receiver, it routes to a new mpsc receiver
-    fn to_mpsc_receiver(self) -> Receiver<Payload>;
+#[derive(Clone)]
+pub struct MsgSender<T> {
+    tx: mpsc::Sender<T>,
 }
 
-#[cfg(not(feature = "ipc"))]
-include!("channel_mpsc.rs");
+impl<T> MsgSender<T> {
+    pub fn send(&self, data: T) -> Result<(), Error> {
+        self.tx.send(data).map_err(|_| Error::new(ErrorKind::Other, "cannot send on closed channel"))
+    }
+}
 
-#[cfg(feature = "ipc")]
-include!("channel_ipc.rs");
+pub fn payload_channel() -> Result<(PayloadSender, PayloadReceiver), Error> {
+    let (tx, rx) = mpsc::channel();
+    Ok((PayloadSender { tx }, PayloadReceiver { rx }))
+}
+
+pub fn msg_channel<T>() -> Result<(MsgSender<T>, MsgReceiver<T>), Error> {
+    let (tx, rx) = mpsc::channel();
+    Ok((MsgSender { tx }, MsgReceiver { rx }))
+}
+
+///
+/// These serialize methods are needed to satisfy the compiler
+/// which uses these implementations for the recording tool.
+/// The recording tool only outputs messages that don't contain
+/// Senders or Receivers, so in theory these should never be
+/// called in the in-process config. If they are called,
+/// there may be a bug in the messages that the replay tool is writing.
+///
+
+impl<T> Serialize for MsgSender<T> {
+    fn serialize<S: Serializer>(&self, _: S) -> Result<S::Ok, S::Error> {
+        unreachable!();
+    }
+}
+
+impl<'de, T> Deserialize<'de> for MsgSender<T> {
+    fn deserialize<D>(_: D) -> Result<MsgSender<T>, D::Error>
+                      where D: Deserializer<'de> {
+        unreachable!();
+    }
+}