Auto merge of #8155 - Ms2ger:join, r=jdm

Remove Window::layout_join_port.



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

components/layout/layout_task.rs

@@ -1251,11 +1251,7 @@ impl LayoutTask {
         }
 
         // Tell script that we're done.
-        //
-        // FIXME(pcwalton): This should probably be *one* channel, but we can't fix this without
-        // either select or a filtered recv() that only looks for messages of a given type.
         data.script_join_chan.send(()).unwrap();
-        data.script_chan.send(ConstellationControlMsg::ReflowComplete(self.id, data.id)).unwrap();
     }
 
     fn set_visible_rects<'a>(&'a self,

components/script/dom/window.rs

@@ -63,11 +63,10 @@ use std::collections::HashSet;
 use std::default::Default;
 use std::ffi::CString;
 use std::io::{Write, stderr, stdout};
-use std::mem as std_mem;
 use std::rc::Rc;
 use std::sync::Arc;
 use std::sync::mpsc::TryRecvError::{Disconnected, Empty};
-use std::sync::mpsc::{Receiver, Sender, channel};
+use std::sync::mpsc::{Sender, channel};
 use string_cache::Atom;
 use time;
 use timers::{ActiveTimers, IsInterval, TimerCallback};
@@ -182,10 +181,6 @@ pub struct Window {
     #[ignore_heap_size_of = "trait objects are hard"]
     layout_rpc: Box<LayoutRPC + 'static>,
 
-    /// The port that we will use to join layout. If this is `None`, then layout is not running.
-    #[ignore_heap_size_of = "channels are hard"]
-    layout_join_port: DOMRefCell<Option<Receiver<()>>>,
-
     /// The current size of the window, in pixels.
     window_size: Cell<Option<WindowSizeData>>,
 
@@ -914,11 +909,6 @@ impl Window {
         // Layout will let us know when it's done.
         let (join_chan, join_port) = channel();
 
-        {
-            let mut layout_join_port = self.layout_join_port.borrow_mut();
-            *layout_join_port = Some(join_port);
-        }
-
         let last_reflow_id = &self.last_reflow_id;
         last_reflow_id.set(last_reflow_id.get() + 1);
 
@@ -946,7 +936,21 @@ impl Window {
 
         debug!("script: layout forked");
 
-        self.join_layout();
+        // FIXME(cgaebel): this is racey. What if the compositor triggers a
+        // reflow between the "join complete" message and returning from this
+        // function?
+        match join_port.try_recv() {
+            Err(Empty) => {
+                info!("script: waiting on layout");
+                join_port.recv().unwrap();
+            }
+            Ok(_) => {}
+            Err(Disconnected) => {
+                panic!("Layout task failed while script was waiting for a result.");
+            }
+        }
+
+        debug!("script: layout joined");
 
         self.pending_reflow_count.set(0);
 
@@ -977,30 +981,6 @@ impl Window {
         self.force_reflow(goal, query_type, reason)
     }
 
-    // FIXME(cgaebel): join_layout is racey. What if the compositor triggers a
-    // reflow between the "join complete" message and returning from this
-    // function?
-
-    /// Sends a ping to layout and waits for the response. The response will arrive when the
-    /// layout task has finished any pending request messages.
-    pub fn join_layout(&self) {
-        let mut layout_join_port = self.layout_join_port.borrow_mut();
-        if let Some(join_port) = std_mem::replace(&mut *layout_join_port, None) {
-            match join_port.try_recv() {
-                Err(Empty) => {
-                    info!("script: waiting on layout");
-                    join_port.recv().unwrap();
-                }
-                Ok(_) => {}
-                Err(Disconnected) => {
-                    panic!("Layout task failed while script was waiting for a result.");
-                }
-            }
-
-            debug!("script: layout joined")
-        }
-    }
-
     pub fn layout(&self) -> &LayoutRPC {
         &*self.layout_rpc
     }
@@ -1009,7 +989,6 @@ impl Window {
         self.reflow(ReflowGoal::ForScriptQuery,
                     ReflowQueryType::ContentBoxQuery(content_box_request),
                     ReflowReason::Query);
-        self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough?
         let ContentBoxResponse(rect) = self.layout_rpc.content_box();
         rect
     }
@@ -1018,7 +997,6 @@ impl Window {
         self.reflow(ReflowGoal::ForScriptQuery,
                     ReflowQueryType::ContentBoxesQuery(content_boxes_request),
                     ReflowReason::Query);
-        self.join_layout(); //FIXME: is this necessary, or is layout_rpc's mutex good enough?
         let ContentBoxesResponse(rects) = self.layout_rpc.content_boxes();
         rects
     }
@@ -1055,13 +1033,6 @@ impl Window {
         (element, response.rect)
     }
 
-    pub fn handle_reflow_complete_msg(&self, reflow_id: u32) {
-        let last_reflow_id = self.last_reflow_id.get();
-        if last_reflow_id == reflow_id {
-            *self.layout_join_port.borrow_mut() = None;
-        }
-    }
-
     pub fn init_browsing_context(&self, doc: &Document, frame_element: Option<&Element>) {
         let mut browsing_context = self.browsing_context.borrow_mut();
         *browsing_context = Some(BrowsingContext::new(doc, frame_element));
@@ -1135,10 +1106,6 @@ impl Window {
         subpage_id
     }
 
-    pub fn layout_is_idle(&self) -> bool {
-        self.layout_join_port.borrow().is_none()
-    }
-
     pub fn get_pending_reflow_count(&self) -> u32 {
         self.pending_reflow_count.get()
     }
@@ -1320,7 +1287,6 @@ impl Window {
             next_subpage_id: Cell::new(SubpageId(0)),
             layout_chan: layout_chan,
             layout_rpc: layout_rpc,
-            layout_join_port: DOMRefCell::new(None),
             window_size: Cell::new(window_size),
             current_viewport: Cell::new(Rect::zero()),
             pending_reflow_count: Cell::new(0),

components/script/script_task.rs

@@ -713,7 +713,6 @@ impl ScriptTask {
                 for page in page.iter() {
                     // Only process a resize if layout is idle.
                     let window = page.window();
-                    if window.r().layout_is_idle() {
                     let resize_event = window.r().steal_resize_event();
                     match resize_event {
                         Some(size) => resizes.push((window.r().pipeline(), size)),
@@ -722,7 +721,6 @@ impl ScriptTask {
                 }
             }
         }
-        }
 
         for (id, size) in resizes {
             self.handle_event(id, ResizeEvent(size));
@@ -938,8 +936,6 @@ impl ScriptTask {
                 self.handle_navigate(pipeline_id, Some(subpage_id), load_data),
             ConstellationControlMsg::SendEvent(id, event) =>
                 self.handle_event(id, event),
-            ConstellationControlMsg::ReflowComplete(id, reflow_id) =>
-                self.handle_reflow_complete_msg(id, reflow_id),
             ConstellationControlMsg::ResizeInactive(id, new_size) =>
                 self.handle_resize_inactive_msg(id, new_size),
             ConstellationControlMsg::Viewport(..) =>
@@ -1386,21 +1382,6 @@ impl ScriptTask {
         frame_element.r().unwrap().update_subpage_id(new_subpage_id);
     }
 
-    /// Handles a notification that reflow completed.
-    fn handle_reflow_complete_msg(&self, pipeline_id: PipelineId, reflow_id: u32) {
-        debug!("Script: Reflow {:?} complete for {:?}", reflow_id, pipeline_id);
-        let page = self.root_page();
-        match page.find(pipeline_id) {
-            Some(page) => {
-                let window = page.window();
-                window.r().handle_reflow_complete_msg(reflow_id);
-            }
-            None => {
-                assert!(self.closed_pipelines.borrow().contains(&pipeline_id));
-            }
-        }
-    }
-
     /// Window was resized, but this script was not active, so don't reflow yet
     fn handle_resize_inactive_msg(&self, id: PipelineId, new_size: WindowSizeData) {
         let page = self.root_page();

components/script_traits/lib.rs

@@ -118,8 +118,6 @@ pub enum ConstellationControlMsg {
     ExitPipeline(PipelineId, PipelineExitType),
     /// Sends a DOM event.
     SendEvent(PipelineId, CompositorEvent),
-    /// Notifies script that reflow is finished.
-    ReflowComplete(PipelineId, u32),
     /// Notifies script of the viewport.
     Viewport(PipelineId, Rect<f32>),
     /// Requests that the script task immediately send the constellation the title of a pipeline.