Auto merge of #6131 - glennw:jquery-exit-fix, r=jdm

This fixes a hang found while testing the jQuery test suite.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6131)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2015-05-19 21:44:45 -05:00
commit c51e9f0455
7 changed files with 74 additions and 21 deletions

View file

@ -584,6 +584,17 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
let document = self.Document().root();
NodeCast::from_ref(document.r()).teardown();
// The above code may not catch all DOM objects
// (e.g. DOM objects removed from the tree that haven't
// been collected yet). Forcing a GC here means that
// those DOM objects will be able to call dispose()
// to free their layout data before the layout task
// exits. Without this, those remaining objects try to
// send a message to free their layout data to the
// layout task when the script task is dropped,
// which causes a panic!
self.Gc();
*self.js_runtime.borrow_mut() = None;
*self.browser_context.borrow_mut() = None;
}

View file

@ -317,6 +317,9 @@ pub struct ScriptTask {
js_runtime: Rc<Runtime>,
mouse_over_targets: DOMRefCell<Vec<JS<Node>>>,
/// List of pipelines that have been owned and closed by this script task.
closed_pipelines: RefCell<HashSet<PipelineId>>,
}
/// In the event of task failure, all data on the stack runs its destructor. However, there
@ -386,7 +389,7 @@ impl ScriptTaskFactory for ScriptTask {
let ConstellationChan(const_chan) = constellation_chan.clone();
let (script_chan, script_port) = channel();
let layout_chan = LayoutChan(layout_chan.sender());
spawn_named_with_send_on_failure("ScriptTask", task_state::SCRIPT, move || {
spawn_named_with_send_on_failure(format!("ScriptTask {:?}", id), task_state::SCRIPT, move || {
let script_task = ScriptTask::new(box compositor as Box<ScriptListener>,
script_port,
NonWorkerScriptChan(script_chan),
@ -494,7 +497,8 @@ impl ScriptTask {
devtools_marker_sender: RefCell::new(None),
js_runtime: Rc::new(runtime),
mouse_over_targets: DOMRefCell::new(vec!())
mouse_over_targets: DOMRefCell::new(vec!()),
closed_pipelines: RefCell::new(HashSet::new()),
}
}
@ -1027,11 +1031,15 @@ impl ScriptTask {
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();
let page = page.find(pipeline_id).expect(
"ScriptTask: received a load message for a layout channel that is not associated \
with this script task. This is a bug.");
let window = page.window().root();
window.r().handle_reflow_complete_msg(reflow_id);
match page.find(pipeline_id) {
Some(page) => {
let window = page.window().root();
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
@ -1062,12 +1070,20 @@ impl ScriptTask {
/// Kick off the document and frame tree creation process using the result.
fn handle_page_fetch_complete(&self, id: PipelineId, subpage: Option<SubpageId>,
response: LoadResponse) {
// Any notification received should refer to an existing, in-progress load that is tracked.
let idx = self.incomplete_loads.borrow().iter().position(|load| {
load.pipeline_id == id && load.parent_info.map(|info| info.1) == subpage
}).unwrap();
let load = self.incomplete_loads.borrow_mut().remove(idx);
self.load(response, load);
});
// The matching in progress load structure may not exist if
// the pipeline exited before the page load completed.
match idx {
Some(idx) => {
let load = self.incomplete_loads.borrow_mut().remove(idx);
self.load(response, load);
}
None => {
assert!(self.closed_pipelines.borrow().contains(&id));
}
}
}
/// Handles a request for the window title.
@ -1080,11 +1096,31 @@ impl ScriptTask {
/// Handles a request to exit the script task and shut down layout.
/// Returns true if the script task should shut down and false otherwise.
fn handle_exit_pipeline_msg(&self, id: PipelineId, exit_type: PipelineExitType) -> bool {
if self.page.borrow().is_none() {
// The root page doesn't even exist yet, so it
// is safe to exit this script task.
// TODO(gw): This probably leaks resources!
return true;
self.closed_pipelines.borrow_mut().insert(id);
// Check if the exit message is for an in progress load.
let idx = self.incomplete_loads.borrow().iter().position(|load| {
load.pipeline_id == id
});
if let Some(idx) = idx {
let load = self.incomplete_loads.borrow_mut().remove(idx);
// Tell the layout task to begin shutting down, and wait until it
// processed this message.
let (response_chan, response_port) = channel();
let LayoutChan(chan) = load.layout_chan;
if chan.send(layout_interface::Msg::PrepareToExit(response_chan)).is_ok() {
debug!("shutting down layout for page {:?}", id);
response_port.recv().unwrap();
chan.send(layout_interface::Msg::ExitNow(exit_type)).ok();
}
let has_pending_loads = self.incomplete_loads.borrow().len() > 0;
let has_root_page = self.page.borrow().is_some();
// Exit if no pending loads and no root page
return !has_pending_loads && !has_root_page;
}
// If root is being exited, shut down all pages