Auto merge of #17068 - gterzian:remove_compositorproxy, r=asajeffrey

Separate waking the event loop, from communicating with Compositor

<!-- Please describe your changes on the following line: -->

@paulrouget first step of #15934, Glutin only for now, please take a look...

If this makes sense, I will also update the code for ports other than Glutin...

One question: one do I add the `warn!` macro to `servolib`? Also perhaps we would also want to add `box`, since I had to switch to using `Box::new`...

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

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17068)
<!-- Reviewable:end -->

components/compositing/compositor.rs

@@ -103,7 +103,7 @@ pub struct IOCompositor<Window: WindowMethods> {
     window: Rc<Window>,
 
     /// The port on which we receive messages.
-    port: Box<CompositorReceiver>,
+    port: CompositorReceiver,
 
     /// The root pipeline.
     root_pipeline: Option<CompositionPipeline>,
@@ -133,7 +133,7 @@ pub struct IOCompositor<Window: WindowMethods> {
     /// The device pixel ratio for this window.
     scale_factor: ScaleFactor<f32, DeviceIndependentPixel, DevicePixel>,
 
-    channel_to_self: Box<CompositorProxy + Send>,
+    channel_to_self: CompositorProxy,
 
     /// A handle to the delayed composition timer.
     delayed_composition_timer: DelayedCompositionTimerProxy,
@@ -312,11 +312,11 @@ fn initialize_png(gl: &gl::Gl, width: usize, height: usize) -> RenderTargetInfo
 }
 
 struct RenderNotifier {
-    compositor_proxy: Box<CompositorProxy>,
+    compositor_proxy: CompositorProxy,
 }
 
 impl RenderNotifier {
-    fn new(compositor_proxy: Box<CompositorProxy>,
+    fn new(compositor_proxy: CompositorProxy,
            _: Sender<ConstellationMsg>) -> RenderNotifier {
         RenderNotifier {
             compositor_proxy: compositor_proxy,
@@ -336,7 +336,7 @@ impl webrender_traits::RenderNotifier for RenderNotifier {
 
 // Used to dispatch functions from webrender to the main thread's event loop.
 struct CompositorThreadDispatcher {
-    compositor_proxy: Box<CompositorProxy>
+    compositor_proxy: CompositorProxy
 }
 
 impl webrender_traits::RenderDispatcher for CompositorThreadDispatcher {

components/compositing/compositor_thread.rs

@@ -22,40 +22,54 @@ use style_traits::viewport::ViewportConstraints;
 use webrender;
 use webrender_traits;
 
-/// Sends messages to the compositor. This is a trait supplied by the port because the method used
-/// to communicate with the compositor may have to kick OS event loops awake, communicate cross-
-/// process, and so forth.
-pub trait CompositorProxy : 'static + Send {
-    /// Sends a message to the compositor.
-    fn send(&self, msg: Msg);
-    /// Clones the compositor proxy.
-    fn clone_compositor_proxy(&self) -> Box<CompositorProxy + 'static + Send>;
+
+/// Used to wake up the event loop, provided by the servo port/embedder.
+pub trait EventLoopWaker : 'static + Send {
+    fn clone(&self) -> Box<EventLoopWaker + Send>;
+    fn wake(&self);
+}
+
+/// Sends messages to the compositor.
+pub struct CompositorProxy {
+    pub sender: Sender<Msg>,
+    pub event_loop_waker: Box<EventLoopWaker>,
+}
+
+impl CompositorProxy {
+    pub fn send(&self, msg: Msg) {
+        // Send a message and kick the OS event loop awake.
+        if let Err(err) = self.sender.send(msg) {
+            warn!("Failed to send response ({}).", err);
+        }
+        self.event_loop_waker.wake();
+    }
+    pub fn clone_compositor_proxy(&self) -> CompositorProxy {
+        CompositorProxy {
+            sender: self.sender.clone(),
+            event_loop_waker: self.event_loop_waker.clone(),
+        }
+    }
 }
 
-/// The port that the compositor receives messages on. As above, this is a trait supplied by the
-/// Servo port.
-pub trait CompositorReceiver : 'static {
-    /// Receives the next message inbound for the compositor. This must not block.
-    fn try_recv_compositor_msg(&mut self) -> Option<Msg>;
-    /// Synchronously waits for, and returns, the next message inbound for the compositor.
-    fn recv_compositor_msg(&mut self) -> Msg;
+/// The port that the compositor receives messages on.
+pub struct CompositorReceiver {
+    pub receiver: Receiver<Msg>
 }
 
-/// A convenience implementation of `CompositorReceiver` for a plain old Rust `Receiver`.
-impl CompositorReceiver for Receiver<Msg> {
-    fn try_recv_compositor_msg(&mut self) -> Option<Msg> {
-        self.try_recv().ok()
+impl CompositorReceiver {
+    pub fn try_recv_compositor_msg(&mut self) -> Option<Msg> {
+        self.receiver.try_recv().ok()
     }
-    fn recv_compositor_msg(&mut self) -> Msg {
-        self.recv().unwrap()
+    pub fn recv_compositor_msg(&mut self) -> Msg {
+        self.receiver.recv().unwrap()
     }
 }
 
 pub trait RenderListener {
     fn recomposite(&mut self, reason: CompositingReason);
 }
 
-impl RenderListener for Box<CompositorProxy + 'static> {
+impl RenderListener for CompositorProxy {
     fn recomposite(&mut self, reason: CompositingReason) {
         self.send(Msg::Recomposite(reason));
     }
@@ -173,9 +187,9 @@ impl Debug for Msg {
 /// Data used to construct a compositor.
 pub struct InitialCompositorState {
     /// A channel to the compositor.
-    pub sender: Box<CompositorProxy + Send>,
+    pub sender: CompositorProxy,
     /// A port on which messages inbound to the compositor can be received.
-    pub receiver: Box<CompositorReceiver>,
+    pub receiver: CompositorReceiver,
     /// A channel to the constellation.
     pub constellation_chan: Sender<ConstellationMsg>,
     /// A channel to the time profiler thread.

components/compositing/delayed_composition.rs

@@ -23,7 +23,7 @@ pub struct DelayedCompositionTimerProxy {
 }
 
 struct DelayedCompositionTimer {
-    compositor_proxy: Box<CompositorProxy>,
+    compositor_proxy: CompositorProxy,
     receiver: Receiver<ToDelayedCompositionTimerMsg>,
 }
 
@@ -33,7 +33,7 @@ enum ToDelayedCompositionTimerMsg {
 }
 
 impl DelayedCompositionTimerProxy {
-    pub fn new(compositor_proxy: Box<CompositorProxy + Send>) -> DelayedCompositionTimerProxy {
+    pub fn new(compositor_proxy: CompositorProxy) -> DelayedCompositionTimerProxy {
         let (to_timer_sender, to_timer_receiver) = channel();
         Builder::new().spawn(move || {
             let mut timer = DelayedCompositionTimer {

components/compositing/windowing.rs

@@ -4,7 +4,7 @@
 
 //! Abstract windowing methods. The concrete implementations of these can be found in `platform/`.
 
-use compositor_thread::{CompositorProxy, CompositorReceiver};
+use compositor_thread::EventLoopWaker;
 use euclid::{Point2D, Size2D};
 use euclid::point::TypedPoint2D;
 use euclid::rect::TypedRect;
@@ -144,12 +144,8 @@ pub trait WindowMethods {
     /// Returns the scale factor of the system (device pixels / device independent pixels).
     fn hidpi_factor(&self) -> ScaleFactor<f32, DeviceIndependentPixel, DevicePixel>;
 
-    /// Creates a channel to the compositor. The dummy parameter is needed because we don't have
-    /// UFCS in Rust yet.
-    ///
-    /// This is part of the windowing system because its implementation often involves OS-specific
-    /// magic to wake the up window's event loop.
-    fn create_compositor_channel(&self) -> (Box<CompositorProxy + Send>, Box<CompositorReceiver>);
+    /// Returns a thread-safe object to wake up the window's event loop.
+    fn create_event_loop_waker(&self) -> Box<EventLoopWaker>;
 
     /// Requests that the window system prepare a composite. Typically this will involve making
     /// some type of platform-specific graphics context current. Returns true if the composite may

components/constellation/constellation.rs

@@ -172,7 +172,7 @@ pub struct Constellation<Message, LTF, STF> {
 
     /// A channel (the implementation of which is port-specific) for the
     /// constellation to send messages to the compositor thread.
-    compositor_proxy: Box<CompositorProxy>,
+    compositor_proxy: CompositorProxy,
 
     /// Channels for the constellation to send messages to the public
     /// resource-related threads.  There are two groups of resource
@@ -302,7 +302,7 @@ pub struct Constellation<Message, LTF, STF> {
 /// State needed to construct a constellation.
 pub struct InitialConstellationState {
     /// A channel through which messages can be sent to the compositor.
-    pub compositor_proxy: Box<CompositorProxy + Send>,
+    pub compositor_proxy: CompositorProxy,
 
     /// A channel to the debugger, if applicable.
     pub debugger_chan: Option<debugger::Sender>,

components/constellation/pipeline.rs

@@ -69,7 +69,7 @@ pub struct Pipeline {
     pub layout_chan: IpcSender<LayoutControlMsg>,
 
     /// A channel to the compositor.
-    pub compositor_proxy: Box<CompositorProxy + 'static + Send>,
+    pub compositor_proxy: CompositorProxy,
 
     /// The most recently loaded URL in this pipeline.
     /// Note that this URL can change, for example if the page navigates
@@ -123,7 +123,7 @@ pub struct InitialPipelineState {
     pub scheduler_chan: IpcSender<TimerSchedulerMsg>,
 
     /// A channel to the compositor.
-    pub compositor_proxy: Box<CompositorProxy + 'static + Send>,
+    pub compositor_proxy: CompositorProxy,
 
     /// A channel to the developer tools, if applicable.
     pub devtools_chan: Option<Sender<DevtoolsControlMsg>>,
@@ -303,7 +303,7 @@ impl Pipeline {
                parent_info: Option<(PipelineId, FrameType)>,
                event_loop: Rc<EventLoop>,
                layout_chan: IpcSender<LayoutControlMsg>,
-               compositor_proxy: Box<CompositorProxy + 'static + Send>,
+               compositor_proxy: CompositorProxy,
                is_private: bool,
                url: ServoUrl,
                visible: bool)

components/servo/lib.rs

@@ -68,8 +68,8 @@ fn webdriver(_port: u16, _constellation: Sender<ConstellationMsg>) { }
 
 use bluetooth::BluetoothThreadFactory;
 use bluetooth_traits::BluetoothRequest;
-use compositing::{CompositorProxy, IOCompositor};
-use compositing::compositor_thread::InitialCompositorState;
+use compositing::IOCompositor;
+use compositing::compositor_thread::{self, CompositorProxy, CompositorReceiver, InitialCompositorState};
 use compositing::windowing::WindowEvent;
 use compositing::windowing::WindowMethods;
 use constellation::{Constellation, InitialConstellationState, UnprivilegedPipelineContent};
@@ -97,7 +97,7 @@ use std::borrow::Cow;
 use std::cmp::max;
 use std::path::PathBuf;
 use std::rc::Rc;
-use std::sync::mpsc::Sender;
+use std::sync::mpsc::{Sender, channel};
 use webrender::renderer::RendererKind;
 use webvr::{WebVRThread, WebVRCompositorHandler};
 
@@ -134,7 +134,7 @@ impl<Window> Browser<Window> where Window: WindowMethods + 'static {
         // messages to client may need to pump a platform-specific event loop
         // to deliver the message.
         let (compositor_proxy, compositor_receiver) =
-            window.create_compositor_channel();
+            create_compositor_channel(window.create_event_loop_waker());
         let supports_clipboard = window.supports_clipboard();
         let time_profiler_chan = profile_time::Profiler::create(&opts.time_profiling,
                                                                 opts.time_profiler_trace_path.clone());
@@ -273,10 +273,22 @@ impl<Window> Browser<Window> where Window: WindowMethods + 'static {
     }
 }
 
+fn create_compositor_channel(event_loop_waker: Box<compositor_thread::EventLoopWaker>)
+    -> (CompositorProxy, CompositorReceiver) {
+    let (sender, receiver) = channel();
+    (CompositorProxy {
+         sender: sender,
+         event_loop_waker: event_loop_waker,
+        },
+     CompositorReceiver {
+         receiver: receiver
+     })
+}
+
 fn create_constellation(user_agent: Cow<'static, str>,
                         config_dir: Option<PathBuf>,
                         url: Option<ServoUrl>,
-                        compositor_proxy: Box<CompositorProxy + Send>,
+                        compositor_proxy: CompositorProxy,
                         time_profiler_chan: time::ProfilerChan,
                         mem_profiler_chan: mem::ProfilerChan,
                         debugger_chan: Option<debugger::Sender>,

ports/cef/window.rs

@@ -17,7 +17,7 @@ use render_handler::CefRenderHandlerExtensions;
 use types::{cef_cursor_handle_t, cef_cursor_type_t, cef_rect_t};
 use wrappers::CefWrap;
 
-use compositing::compositor_thread::{self, CompositorProxy, CompositorReceiver};
+use compositing::compositor_thread::EventLoopWaker;
 use compositing::windowing::{WindowEvent, WindowMethods};
 use euclid::point::{Point2D, TypedPoint2D};
 use euclid::rect::TypedRect;
@@ -295,13 +295,17 @@ impl WindowMethods for Window {
         }
     }
 
-    fn create_compositor_channel(&self)
-                                 -> (Box<CompositorProxy+Send>, Box<CompositorReceiver>) {
-        let (sender, receiver) = channel();
-        (box CefCompositorProxy {
-             sender: sender,
-         } as Box<CompositorProxy+Send>,
-         box receiver as Box<CompositorReceiver>)
+    fn create_event_loop_waker(&self) -> Box<EventLoopWaker> {
+        struct CefEventLoopWaker;
+        impl EventLoopWaker for CefEventLoopWaker {
+            fn wake(&self) {
+                app_wakeup();
+            }
+            fn clone(&self) -> Box<EventLoopWaker + Send> {
+                box CefEventLoopWaker
+            }
+        }
+        box CefEventLoopWaker
     }
 
     fn prepare_for_composite(&self, width: usize, height: usize) -> bool {
@@ -500,23 +504,6 @@ impl WindowMethods for Window {
     }
 }
 
-struct CefCompositorProxy {
-    sender: Sender<compositor_thread::Msg>,
-}
-
-impl CompositorProxy for CefCompositorProxy {
-    fn send(&self, msg: compositor_thread::Msg) {
-        self.sender.send(msg).unwrap();
-        app_wakeup();
-    }
-
-    fn clone_compositor_proxy(&self) -> Box<CompositorProxy+Send> {
-        box CefCompositorProxy {
-            sender: self.sender.clone(),
-        } as Box<CompositorProxy+Send>
-    }
-}
-
 #[cfg(target_os="macos")]
 pub fn app_wakeup() {
     use cocoa::appkit::{NSApp, NSApplication, NSApplicationDefined};