From af448374f9a177046d3c119333f757a60f3aa502 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 15:54:20 +0200 Subject: [PATCH 1/6] Remove pointless join_layout calls. Code on the script thread can only observe self.layout_join_port being Some() between the time it is set in force_reflow, and the join_layout call later in that function, and no significant code is called in that code. As join_layout does nothing when called if self.layout_join_port is None, these two calls are useless. --- components/script/dom/window.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 5906a28cb99..d6c1201106e 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1009,7 +1009,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 +1017,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 } From 9d739cf684f0fcc59ae1e0e91ba804481e4d7aa2 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 15:54:38 +0200 Subject: [PATCH 2/6] Remove layout_is_idle. Code on the script thread can only observe self.layout_join_port being Some() between the time it is set in force_reflow, and the join_layout call later in that function, and no significant code is called in that code. This implies that layout_is_idle will always return true, so there is no reason for the function to exist. --- components/script/dom/window.rs | 4 ---- components/script/script_task.rs | 10 ++++------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index d6c1201106e..7f885988c4e 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1133,10 +1133,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() } diff --git a/components/script/script_task.rs b/components/script/script_task.rs index a090d1c91ef..8426363a69e 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -712,12 +712,10 @@ 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)), - None => () - } + let resize_event = window.r().steal_resize_event(); + match resize_event { + Some(size) => resizes.push((window.r().pipeline(), size)), + None => () } } } From 7cd3870977841a00122b2d6009c43eb976ce945d Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 15:57:46 +0200 Subject: [PATCH 3/6] Inline Window::join_layout into its only caller. I do not know if the FIXME comment is correct; I just copied it. --- components/script/dom/window.rs | 43 ++++++++++++++------------------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 7f885988c4e..75a66716043 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -946,7 +946,24 @@ 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? + 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") + } self.pending_reflow_count.set(0); @@ -977,30 +994,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 } From 2456ddf4bad67dbaee298b3ee84169918d46c397 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 15:59:52 +0200 Subject: [PATCH 4/6] Remove Window::handle_reflow_complete_msg and ScriptTask::handle_reflow_complete_msg. Code on the script thread can only observe self.layout_join_port being Some() between the time it is set in force_reflow, and the join_layout call later in that function, and no significant code is called in that code. Since these functions do nothing useful if layout_join_port is None, there is no point in keeping them. --- components/script/dom/window.rs | 7 ------- components/script/script_task.rs | 19 ++----------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 75a66716043..c292d0c4e20 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1046,13 +1046,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)); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 8426363a69e..98431ab1330 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -935,8 +935,8 @@ 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::ReflowComplete(_id, _reflow_id) => + (), ConstellationControlMsg::ResizeInactive(id, new_size) => self.handle_resize_inactive_msg(id, new_size), ConstellationControlMsg::Viewport(..) => @@ -1383,21 +1383,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(); From 9ad69586d17dae565e9b5a1ff2340eca6f161d43 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 16:06:54 +0200 Subject: [PATCH 5/6] Remove the ConstellationControlMsg::ReflowComplete message. --- components/layout/layout_task.rs | 4 ---- components/script/script_task.rs | 2 -- components/script_traits/lib.rs | 2 -- 3 files changed, 8 deletions(-) diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 505676e126b..e99768b28d4 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -1281,11 +1281,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, diff --git a/components/script/script_task.rs b/components/script/script_task.rs index 98431ab1330..8216cbfbacf 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -935,8 +935,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) => - (), ConstellationControlMsg::ResizeInactive(id, new_size) => self.handle_resize_inactive_msg(id, new_size), ConstellationControlMsg::Viewport(..) => diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 3c04fa36dda..6ed68b92dce 100644 --- a/components/script_traits/lib.rs +++ b/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), /// Requests that the script task immediately send the constellation the title of a pipeline. From 1608450e4e61b409ce45371406fd87155721bc16 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 22 Oct 2015 16:11:45 +0200 Subject: [PATCH 6/6] Remove Window::layout_join_port. It is only used in Window::force_reflow. --- components/script/dom/window.rs | 36 ++++++++++----------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index c292d0c4e20..afae2923bf5 100644 --- a/components/script/dom/window.rs +++ b/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, - /// 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>>, - /// The current size of the window, in pixels. window_size: Cell>, @@ -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); @@ -949,22 +939,19 @@ impl Window { // FIXME(cgaebel): this is racey. What if the compositor triggers a // reflow between the "join complete" message and returning from this // function? - 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."); - } + 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") } + debug!("script: layout joined"); + self.pending_reflow_count.set(0); if let Some(marker) = marker { @@ -1300,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),