Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 06:00:15 +01:00
Code Issues Projects Releases Packages Wiki Activity

auto merge of #2130 : mbrubeck/servo/frametree-copy-fix, r=larsbergstrom

Browse source 
We are calling `get` when we should call `borrow` or `borrow_mut`.  This is causing bugs with iframes because we append the child frame to a copy of the parent's `children` vector rather than the original.

r? @larsbergstrom
This commit is contained in:
bors-servo 2014-04-16 12:55:16 -04:00
commit f6cd507e97
1 changed files with 38 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

76
src/components/main/constellation.rs
View file

@ -62,7 +62,7 @@ pub struct FrameTree {
impl Clone for FrameTree { impl Clone for FrameTree {
fn clone(&self) -> FrameTree { fn clone(&self) -> FrameTree {
let children = self.children let children = self.children
.get() .borrow()
.iter() .iter()
.map(|child_frame_tree| child_frame_tree.clone()) .map(|child_frame_tree| child_frame_tree.clone())
.collect(); .collect();
@ -107,23 +107,23 @@ enum ReplaceResult {
impl FrameTree { impl FrameTree {
fn contains(&self, id: PipelineId) -> bool { fn contains(&self, id: PipelineId) -> bool {
self.iter().any(|frame_tree| id == frame_tree.pipeline.get().id) self.iter().any(|frame_tree| id == frame_tree.pipeline.borrow().id)
} }
/// Returns the frame tree whose key is id /// Returns the frame tree whose key is id
fn find(&self, id: PipelineId) -> Option<Rc<FrameTree>> { fn find(&self, id: PipelineId) -> Option<Rc<FrameTree>> {
self.iter().find(|frame_tree| id == frame_tree.pipeline.get().id) self.iter().find(|frame_tree| id == frame_tree.pipeline.borrow().id)
} }
/// Replaces a node of the frame tree in place. Returns the node that was removed or the original node /// Replaces a node of the frame tree in place. Returns the node that was removed or the original node
/// if the node to replace could not be found. /// if the node to replace could not be found.
fn replace_child(&self, id: PipelineId, new_child: Rc<FrameTree>) -> ReplaceResult { fn replace_child(&self, id: PipelineId, new_child: Rc<FrameTree>) -> ReplaceResult {
for frame_tree in self.iter() { for frame_tree in self.iter() {
let mut children = frame_tree.children.get(); let mut children = frame_tree.children.borrow_mut();
let mut child = children.mut_iter() let mut child = children.mut_iter()
.find(|child| child.frame_tree.pipeline.get().id == id); .find(|child| child.frame_tree.pipeline.borrow().id == id);
for child in child.mut_iter() { for child in child.mut_iter() {
new_child.parent.set(child.frame_tree.parent.get().clone()); new_child.parent.set(child.frame_tree.parent.get());
return ReplacedNode(replace(&mut child.frame_tree, new_child)); return ReplacedNode(replace(&mut child.frame_tree, new_child));
} }
} }
@ -132,8 +132,8 @@ impl FrameTree {
fn to_sendable(&self) -> SendableFrameTree { fn to_sendable(&self) -> SendableFrameTree {
let sendable_frame_tree = SendableFrameTree { let sendable_frame_tree = SendableFrameTree {
pipeline: self.pipeline.get().to_sendable(), pipeline: self.pipeline.borrow().to_sendable(),
children: self.children.get().iter().map(|frame_tree| frame_tree.to_sendable()).collect(), children: self.children.borrow().iter().map(|frame_tree| frame_tree.to_sendable()).collect(),
}; };
sendable_frame_tree sendable_frame_tree
} }
@ -165,7 +165,7 @@ impl Iterator<Rc<FrameTree>> for FrameTreeIterator {
fn next(&mut self) -> Option<Rc<FrameTree>> { fn next(&mut self) -> Option<Rc<FrameTree>> {
if !self.stack.is_empty() { if !self.stack.is_empty() {
let next = self.stack.pop(); let next = self.stack.pop();
for cft in next.get_ref().children.get().rev_iter() { for cft in next.get_ref().children.borrow().rev_iter() {
self.stack.push(cft.frame_tree.clone()); self.stack.push(cft.frame_tree.clone());
} }
Some(next.unwrap()) Some(next.unwrap())
@ -217,7 +217,7 @@ impl NavigationContext {
/// Loads a new set of page frames, returning all evicted frame trees /// Loads a new set of page frames, returning all evicted frame trees
pub fn load(&mut self, frame_tree: Rc<FrameTree>) -> ~[Rc<FrameTree>] { pub fn load(&mut self, frame_tree: Rc<FrameTree>) -> ~[Rc<FrameTree>] {
debug!("navigating to {:?}", frame_tree.pipeline.get().id); debug!("navigating to {:?}", frame_tree.pipeline.borrow().id);
let evicted = replace(&mut self.next, ~[]); let evicted = replace(&mut self.next, ~[]);
if self.current.is_some() { if self.current.is_some() {
self.previous.push(self.current.take_unwrap()); self.previous.push(self.current.take_unwrap());
@ -463,7 +463,7 @@ impl Constellation {
// Returns true if a child frame tree's subpage id matches the given subpage id // Returns true if a child frame tree's subpage id matches the given subpage id
let subpage_eq = |child_frame_tree: & &mut ChildFrameTree| { let subpage_eq = |child_frame_tree: & &mut ChildFrameTree| {
child_frame_tree.frame_tree.pipeline.get(). child_frame_tree.frame_tree.pipeline.borrow().
subpage_id.expect("Constellation: subpage_id.expect("Constellation:
child frame does not have a subpage id. This should not be possible.") child frame does not have a subpage id. This should not be possible.")
== subpage_id == subpage_id
@ -479,10 +479,10 @@ impl Constellation {
child_frame_tree.rect = Some(rect.clone()); child_frame_tree.rect = Some(rect.clone());
// NOTE: work around borrowchk issues // NOTE: work around borrowchk issues
let pipeline = &child_frame_tree.frame_tree.pipeline; let pipeline = &child_frame_tree.frame_tree.pipeline;
if !already_sent.contains(&pipeline.get().id) { if !already_sent.contains(&pipeline.borrow().id) {
let Size2D { width, height } = rect.size; let Size2D { width, height } = rect.size;
if is_active { if is_active {
let pipeline = pipeline.get(); let pipeline = pipeline.borrow();
let ScriptChan(ref script_chan) = pipeline.script_chan; let ScriptChan(ref script_chan) = pipeline.script_chan;
script_chan.send(ResizeMsg(pipeline.id, Size2D { script_chan.send(ResizeMsg(pipeline.id, Size2D {
width: width as uint, width: width as uint,
@ -492,7 +492,7 @@ impl Constellation {
LayerId::null(), LayerId::null(),
rect)); rect));
} else { } else {
already_sent.insert(pipeline.get().id); already_sent.insert(pipeline.borrow().id);
} }
}; };
}; };
@ -502,7 +502,7 @@ impl Constellation {
debug!("Constellation: Sending size for frame in current frame tree."); debug!("Constellation: Sending size for frame in current frame tree.");
let source_frame = current_frame.find(pipeline_id); let source_frame = current_frame.find(pipeline_id);
for source_frame in source_frame.iter() { for source_frame in source_frame.iter() {
let mut children = source_frame.children.get(); let mut children = source_frame.children.borrow_mut();
let found_child = children.mut_iter().find(|child| subpage_eq(child)); let found_child = children.mut_iter().find(|child| subpage_eq(child));
found_child.map(|child| update_child_rect(child, true)); found_child.map(|child| update_child_rect(child, true));
} }
@ -510,7 +510,7 @@ impl Constellation {
// Update all frames with matching pipeline- and subpage-ids // Update all frames with matching pipeline- and subpage-ids
for frame_tree in frames.iter() { for frame_tree in frames.iter() {
let mut children = frame_tree.children.get(); let mut children = frame_tree.children.borrow_mut();
let found_child = children.mut_iter().find(|child| subpage_eq(child)); let found_child = children.mut_iter().find(|child| subpage_eq(child));
found_child.map(|child| update_child_rect(child, false)); found_child.map(|child| update_child_rect(child, false));
} }
@ -594,7 +594,7 @@ impl Constellation {
let pipeline_wrapped = Rc::new(pipeline); let pipeline_wrapped = Rc::new(pipeline);
let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id)); let rect = self.pending_sizes.pop(&(source_pipeline_id, subpage_id));
for frame_tree in frame_trees.iter() { for frame_tree in frame_trees.iter() {
frame_tree.children.get().push(ChildFrameTree { frame_tree.children.borrow_mut().push(ChildFrameTree {
frame_tree: Rc::new(FrameTree { frame_tree: Rc::new(FrameTree {
pipeline: RefCell::new(pipeline_wrapped.clone()), pipeline: RefCell::new(pipeline_wrapped.clone()),
parent: RefCell::new(Some(source_pipeline.clone())), parent: RefCell::new(Some(source_pipeline.clone())),
@ -630,7 +630,7 @@ impl Constellation {
// changes would be overriden by changing the subframe associated with source_id. // changes would be overriden by changing the subframe associated with source_id.
let parent = source_frame.parent.clone(); let parent = source_frame.parent.clone();
let subpage_id = source_frame.pipeline.get().subpage_id; let subpage_id = source_frame.pipeline.borrow().subpage_id;
let next_pipeline_id = self.get_next_pipeline_id(); let next_pipeline_id = self.get_next_pipeline_id();
let pipeline = Pipeline::create(next_pipeline_id, let pipeline = Pipeline::create(next_pipeline_id,
@ -673,7 +673,7 @@ impl Constellation {
} else { } else {
let old = self.current_frame().get_ref(); let old = self.current_frame().get_ref();
for frame in old.iter() { for frame in old.iter() {
frame.pipeline.get().revoke_paint_permission(); frame.pipeline.borrow().revoke_paint_permission();
} }
} }
self.navigation_context.forward() self.navigation_context.forward()
@ -685,7 +685,7 @@ impl Constellation {
} else { } else {
let old = self.current_frame().get_ref(); let old = self.current_frame().get_ref();
for frame in old.iter() { for frame in old.iter() {
frame.pipeline.get().revoke_paint_permission(); frame.pipeline.borrow().revoke_paint_permission();
} }
} }
self.navigation_context.back() self.navigation_context.back()
@ -693,7 +693,7 @@ impl Constellation {
}; };
for frame in destination_frame.iter() { for frame in destination_frame.iter() {
frame.pipeline.get().reload(); frame.pipeline.borrow().reload();
} }
self.grant_paint_permission(destination_frame, constellation_msg::Navigate); self.grant_paint_permission(destination_frame, constellation_msg::Navigate);
@ -711,7 +711,7 @@ impl Constellation {
// impossible to occur. // impossible to occur.
if current_frame.contains(pipeline_id) { if current_frame.contains(pipeline_id) {
for frame in current_frame.iter() { for frame in current_frame.iter() {
frame.pipeline.get().grant_paint_permission(); frame.pipeline.borrow().grant_paint_permission();
} }
return; return;
} }
@ -721,7 +721,7 @@ impl Constellation {
// If it is not found, it simply means that this pipeline will not receive // If it is not found, it simply means that this pipeline will not receive
// permission to paint. // permission to paint.
let pending_index = self.pending_frames.iter().rposition(|frame_change| { let pending_index = self.pending_frames.iter().rposition(|frame_change| {
frame_change.after.pipeline.get().id == pipeline_id frame_change.after.pipeline.borrow().id == pipeline_id
}); });
for &pending_index in pending_index.iter() { for &pending_index in pending_index.iter() {
let frame_change = self.pending_frames.swap_remove(pending_index).unwrap(); let frame_change = self.pending_frames.swap_remove(pending_index).unwrap();
@ -749,7 +749,7 @@ impl Constellation {
frame not contained in the current frame. This is a bug"); frame not contained in the current frame. This is a bug");
for frame in to_revoke.iter() { for frame in to_revoke.iter() {
frame.pipeline.get().revoke_paint_permission(); frame.pipeline.borrow().revoke_paint_permission();
} }
// If to_add is not the root frame, then replace revoked_frame with it. // If to_add is not the root frame, then replace revoked_frame with it.
@ -757,10 +757,10 @@ impl Constellation {
// NOTE: work around borrowchk issue // NOTE: work around borrowchk issue
let mut flag = false; let mut flag = false;
{ {
if to_add.parent.get().is_some() { if to_add.parent.borrow().is_some() {
debug!("Constellation: replacing {:?} with {:?} in {:?}", debug!("Constellation: replacing {:?} with {:?} in {:?}",
revoke_id, to_add.pipeline.get().id, revoke_id, to_add.pipeline.borrow().id,
next_frame_tree.pipeline.get().id); next_frame_tree.pipeline.borrow().id);
flag = true; flag = true;
} }
} }
@ -772,15 +772,15 @@ impl Constellation {
None => { None => {
// Add to_add to parent's children, if it is not the root // Add to_add to parent's children, if it is not the root
let parent = &to_add.parent; let parent = &to_add.parent;
for parent in parent.get().iter() { for parent in parent.borrow().iter() {
let subpage_id = to_add.pipeline.get().subpage_id let subpage_id = to_add.pipeline.borrow().subpage_id
.expect("Constellation: .expect("Constellation:
Child frame's subpage id is None. This should be impossible."); Child frame's subpage id is None. This should be impossible.");
let rect = self.pending_sizes.pop(&(parent.id, subpage_id)); let rect = self.pending_sizes.pop(&(parent.id, subpage_id));
let parent = next_frame_tree.find(parent.id).expect( let parent = next_frame_tree.find(parent.id).expect(
"Constellation: pending frame has a parent frame that is not "Constellation: pending frame has a parent frame that is not
active. This is a bug."); active. This is a bug.");
parent.children.get().push(ChildFrameTree { parent.children.borrow_mut().push(ChildFrameTree {
frame_tree: to_add.clone(), frame_tree: to_add.clone(),
rect: rect, rect: rect,
}); });
@ -797,14 +797,14 @@ impl Constellation {
let mut already_seen = HashSet::new(); let mut already_seen = HashSet::new();
for frame_tree in self.current_frame().iter() { for frame_tree in self.current_frame().iter() {
debug!("constellation sending resize message to active frame"); debug!("constellation sending resize message to active frame");
let pipeline = &frame_tree.pipeline.get(); let pipeline = &frame_tree.pipeline.borrow();
let ScriptChan(ref chan) = pipeline.script_chan; let ScriptChan(ref chan) = pipeline.script_chan;
chan.try_send(ResizeMsg(pipeline.id, new_size)); chan.try_send(ResizeMsg(pipeline.id, new_size));
already_seen.insert(pipeline.id); already_seen.insert(pipeline.id);
} }
for frame_tree in self.navigation_context.previous.iter() for frame_tree in self.navigation_context.previous.iter()
.chain(self.navigation_context.next.iter()) { .chain(self.navigation_context.next.iter()) {
let pipeline = &frame_tree.pipeline.get(); let pipeline = &frame_tree.pipeline.borrow();
if !already_seen.contains(&pipeline.id) { if !already_seen.contains(&pipeline.id) {
debug!("constellation sending resize message to inactive frame"); debug!("constellation sending resize message to inactive frame");
let ScriptChan(ref chan) = pipeline.script_chan; let ScriptChan(ref chan) = pipeline.script_chan;
@ -817,9 +817,9 @@ impl Constellation {
// initial window size gets sent to the first page loaded, giving it permission to reflow.) // initial window size gets sent to the first page loaded, giving it permission to reflow.)
for change in self.pending_frames.iter() { for change in self.pending_frames.iter() {
let frame_tree = &change.after; let frame_tree = &change.after;
if frame_tree.parent.get().is_none() { if frame_tree.parent.borrow().is_none() {
debug!("constellation sending resize message to pending outer frame"); debug!("constellation sending resize message to pending outer frame");
let pipeline = frame_tree.pipeline.get(); let pipeline = frame_tree.pipeline.borrow();
let ScriptChan(ref chan) = pipeline.script_chan; let ScriptChan(ref chan) = pipeline.script_chan;
chan.send(ResizeMsg(pipeline.id, new_size)) chan.send(ResizeMsg(pipeline.id, new_size))
} }
@ -833,7 +833,7 @@ impl Constellation {
// TODO(tkuehn): should only exit once per unique script task, // TODO(tkuehn): should only exit once per unique script task,
// and then that script task will handle sub-exits // and then that script task will handle sub-exits
for frame_tree in frame_tree.iter() { for frame_tree in frame_tree.iter() {
let pipeline = frame_tree.pipeline.get(); let pipeline = frame_tree.pipeline.borrow();
pipeline.exit(); pipeline.exit();
self.pipelines.remove(&pipeline.id); self.pipelines.remove(&pipeline.id);
} }
@ -841,10 +841,10 @@ impl Constellation {
fn handle_evicted_frames(&mut self, evicted: ~[Rc<FrameTree>]) { fn handle_evicted_frames(&mut self, evicted: ~[Rc<FrameTree>]) {
for frame_tree in evicted.iter() { for frame_tree in evicted.iter() {
if !self.navigation_context.contains(frame_tree.pipeline.get().id) { if !self.navigation_context.contains(frame_tree.pipeline.borrow().id) {
self.close_pipelines(frame_tree.clone()); self.close_pipelines(frame_tree.clone());
} else { } else {
let frames = frame_tree.children.get().iter() let frames = frame_tree.children.borrow().iter()
.map(|child| child.frame_tree.clone()).collect(); .map(|child| child.frame_tree.clone()).collect();
self.handle_evicted_frames(frames); self.handle_evicted_frames(frames);
} }
@ -875,7 +875,7 @@ impl Constellation {
Some(()) => { Some(()) => {
let mut iter = frame_tree.iter(); let mut iter = frame_tree.iter();
for frame in iter { for frame in iter {
frame.pipeline.get().grant_paint_permission(); frame.pipeline.borrow().grant_paint_permission();
} }
} }
None => {} // message has been discarded, probably shutting down None => {} // message has been discarded, probably shutting down