From 2e6ad0a4a3f907f88c3ef16174db41bd1e302439 Mon Sep 17 00:00:00 2001 From: Tim Kuehn Date: Mon, 29 Jul 2013 20:22:01 -0700 Subject: [PATCH] fix borrow check failures by reverting to storing *mut Page in window --- src/components/main/compositing/mod.rs | 7 ++-- src/components/main/constellation.rs | 56 +++++++++---------------- src/components/msg/constellation_msg.rs | 1 - src/components/script/dom/document.rs | 10 ++--- src/components/script/dom/domparser.rs | 2 +- src/components/script/dom/element.rs | 10 ++--- src/components/script/dom/window.rs | 21 ++++++---- src/components/script/script_task.rs | 2 +- 8 files changed, 46 insertions(+), 63 deletions(-) diff --git a/src/components/main/compositing/mod.rs b/src/components/main/compositing/mod.rs index ce95cda0c3c..b13c5cbc8d0 100644 --- a/src/components/main/compositing/mod.rs +++ b/src/components/main/compositing/mod.rs @@ -13,7 +13,7 @@ use windowing::{QuitWindowEvent, MouseWindowClickEvent, MouseWindowMouseDownEven use servo_msg::compositor_msg::{RenderListener, LayerBuffer, LayerBufferSet, RenderState}; use servo_msg::compositor_msg::{ReadyState, ScriptListener}; -use servo_msg::constellation_msg::{CompositorAck, ConstellationChan, PipelineId}; +use servo_msg::constellation_msg::PipelineId; use servo_msg::constellation_msg; use gfx::render_task::ReRenderMsg; use gfx::opts::Opts; @@ -141,7 +141,7 @@ pub enum Msg { /// Alerts the compositor to the current status of rendering. ChangeRenderState(RenderState), /// Sets the channel to the current layout and render tasks, along with their id - SetIds(SendableFrameTree, ConstellationChan) + SetIds(SendableFrameTree), } /// Azure surface wrapping to work with the layers infrastructure. @@ -345,9 +345,8 @@ impl CompositorTask { ChangeReadyState(ready_state) => window.set_ready_state(ready_state), ChangeRenderState(render_state) => window.set_render_state(render_state), - SetIds(frame_tree, response_chan) => { + SetIds(frame_tree) => { pipeline = Some(frame_tree.pipeline); - response_chan.send(CompositorAck(pipeline.get_ref().id.clone())); } GetSize(chan) => { diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index f3fcc792526..7c1c8a1fa50 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -13,7 +13,7 @@ use std::task; use geom::size::Size2D; use gfx::opts::Opts; use pipeline::Pipeline; -use servo_msg::constellation_msg::{CompositorAck, ConstellationChan, ExitMsg}; +use servo_msg::constellation_msg::{ConstellationChan, ExitMsg}; use servo_msg::constellation_msg::{InitLoadUrlMsg, LoadIframeUrlMsg, LoadUrlMsg}; use servo_msg::constellation_msg::{Msg, NavigateMsg}; use servo_msg::constellation_msg::{PipelineId, RendererReadyMsg, ResizedWindowBroadcast}; @@ -283,16 +283,6 @@ impl Constellation { fn handle_request(&mut self, request: Msg) -> bool { match request { - // Acknowledgement from the compositor that it has updated its active pipeline id - CompositorAck(id) => { - let pending_index = do self.pending_frames.rposition |frame_change| { - frame_change.after.pipeline.id == id - }.expect("Constellation: received compositor ack for frame tree not currently - pending compositor ack. This is a bug."); - let frame_tree = self.pending_frames.swap_remove(pending_index).after; - self.grant_paint_permission(frame_tree); - } - ExitMsg(sender) => { for self.pipelines.iter().advance |(_id, ref pipeline)| { pipeline.exit(); @@ -473,11 +463,21 @@ impl Constellation { // Handle a forward or back request NavigateMsg(direction) => { debug!("received message to navigate %?", direction); + + // TODO(tkuehn): what is the "critical point" beyond which pending frames + // should not be cleared? Currently, the behavior is that forward/back + // navigation always has navigation priority, and after that new page loading is + // first come, first served. let destination_frame = match direction { constellation_msg::Forward => { if self.navigation_context.next.is_empty() { debug!("no next page to navigate to"); return true + } else { + let old = self.current_frame().get_ref(); + for old.iter().advance |frame| { + frame.pipeline.revoke_paint_permission(); + } } self.navigation_context.forward() } @@ -485,6 +485,11 @@ impl Constellation { if self.navigation_context.previous.is_empty() { debug!("no previous page to navigate to"); return true + } else { + let old = self.current_frame().get_ref(); + for old.iter().advance |frame| { + frame.pipeline.revoke_paint_permission(); + } } self.navigation_context.back() } @@ -494,23 +499,8 @@ impl Constellation { let pipeline = &frame.pipeline; pipeline.reload(Some(constellation_msg::Navigate)); } - self.compositor_chan.send(SetIds(destination_frame.to_sendable(), self.chan.clone())); + self.grant_paint_permission(destination_frame); - let before_id = self.current_frame().get_ref().pipeline.id.clone(); - self.pending_frames.clear(); - self.pending_frames.push(FrameChange { - before: Some(before_id), - after: destination_frame, - }); - - // TODO(tkuehn): what is the "critical point" beyond which pending frames - // should not be cleared? Currently, the behavior is that forward/back - // navigation always has navigation priority, and after that new page loading is - // first come, first served. - let old = self.current_frame().get_ref(); - for old.iter().advance |frame| { - frame.pipeline.revoke_paint_permission(); - } } // Notification that rendering has finished and is requesting permission to paint. @@ -527,6 +517,7 @@ impl Constellation { for current_frame.iter().advance |frame| { frame.pipeline.grant_paint_permission(); } + self.compositor_chan.send(SetIds(current_frame.to_sendable())); return true; } } @@ -564,10 +555,6 @@ impl Constellation { if to_add.parent.is_some() { next_frame_tree.replace_child(revoke_id, to_add); } - self.pending_frames.push(FrameChange { - before: Some(revoke_id), - after: next_frame_tree - }); } None => { @@ -580,13 +567,9 @@ impl Constellation { active. This is a bug."); parent.children.push(to_add.take()); } - self.pending_frames.push(FrameChange { - before: None, - after: next_frame_tree - }); } } - self.compositor_chan.send(SetIds(next_frame_tree.to_sendable(), self.chan.clone())); + self.grant_paint_permission(next_frame_tree); } } @@ -623,6 +606,7 @@ impl Constellation { // Grants a frame tree permission to paint; optionally updates navigation to reflect a new page fn grant_paint_permission(&mut self, frame_tree: @mut FrameTree) { // Give permission to paint to the new frame and all child frames + self.compositor_chan.send(SetIds(frame_tree.to_sendable())); for frame_tree.iter().advance |frame| { frame.pipeline.grant_paint_permission(); } diff --git a/src/components/msg/constellation_msg.rs b/src/components/msg/constellation_msg.rs index c261f94dc42..6f71f8a0454 100644 --- a/src/components/msg/constellation_msg.rs +++ b/src/components/msg/constellation_msg.rs @@ -27,7 +27,6 @@ impl ConstellationChan { } pub enum Msg { - CompositorAck(PipelineId), ExitMsg(Chan<()>), InitLoadUrlMsg(Url), LoadUrlMsg(PipelineId, Url, Future>), diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs index 85ad2b86b3f..80f677e35cf 100644 --- a/src/components/script/dom/document.rs +++ b/src/components/script/dom/document.rs @@ -88,7 +88,7 @@ impl Document { parent: Element::new(HTMLHtmlElementTypeId, ~"html") }; - let cx = _owner.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*_owner.page).js_info.get_ref().js_compartment.cx.ptr}; let root = unsafe { Node::as_abstract_node(cx, root) }; Document(root, None) } @@ -130,7 +130,7 @@ impl Document { } }; let win = self.window.get_ref(); - let cx = win.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*win.page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); HTMLCollection::new(elements, cx, scope) @@ -138,7 +138,7 @@ impl Document { pub fn GetElementsByTagNameNS(&self, _ns: DOMString, _tag: DOMString) -> @mut HTMLCollection { let win = self.window.get_ref(); - let cx = win.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*win.page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); HTMLCollection::new(~[], cx, scope) @@ -146,7 +146,7 @@ impl Document { pub fn GetElementsByClassName(&self, _class: DOMString) -> @mut HTMLCollection { let win = self.window.get_ref(); - let cx = win.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*win.page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); HTMLCollection::new(~[], cx, scope) @@ -289,7 +289,7 @@ impl Document { } }; let win = self.window.get_ref(); - let cx = win.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*win.page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); HTMLCollection::new(elements, cx, scope) diff --git a/src/components/script/dom/domparser.rs b/src/components/script/dom/domparser.rs index 6042a0dfc8e..0b8ba1f0326 100644 --- a/src/components/script/dom/domparser.rs +++ b/src/components/script/dom/domparser.rs @@ -22,7 +22,7 @@ impl DOMParser { }; // TODO(tkuehn): This just handles the top-level page. Need to handle subframes. - let cx = owner.page.js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*owner.page).js_info.get_ref().js_compartment.cx.ptr}; let cache = owner.get_wrappercache(); let scope = cache.get_wrapper(); parser.wrap_object_shared(cx, scope); diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index 04987389576..40529dc2ceb 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -179,9 +179,9 @@ impl<'self> Element { let (port, chan) = comm::stream(); // TODO(tkuehn): currently just queries top-level page layout. Needs to query // subframe layout if this element is in a subframe. Probably need an ID field. - match page.query_layout(ContentBoxesQuery(node, chan), port) { + match unsafe {(*page).query_layout(ContentBoxesQuery(node, chan), port)} { Ok(ContentBoxesResponse(rects)) => { - let cx = (*page).js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); let rects = do rects.map |r| { @@ -225,11 +225,9 @@ impl<'self> Element { let node = self.parent.abstract.get(); assert!(node.is_element()); let (port, chan) = comm::stream(); - // TODO(tkuehn): currently just queries top-level page layout. Needs to query - // subframe layout if this element is in a subframe. Probably need an ID field. - match page.query_layout(ContentBoxQuery(node, chan), port) { + match unsafe{(*page).query_layout(ContentBoxQuery(node, chan), port)} { Ok(ContentBoxResponse(rect)) => { - let cx = (*page).js_info.get_ref().js_compartment.cx.ptr; + let cx = unsafe {(*page).js_info.get_ref().js_compartment.cx.ptr}; let cache = win.get_wrappercache(); let scope = cache.get_wrapper(); Some(ClientRect::new( diff --git a/src/components/script/dom/window.rs b/src/components/script/dom/window.rs index e1d7b4f423d..94298fcbc04 100644 --- a/src/components/script/dom/window.rs +++ b/src/components/script/dom/window.rs @@ -25,10 +25,9 @@ pub enum TimerControlMsg { TimerMessage_TriggerExit //XXXjdm this is just a quick hack to talk to the script task } -//FIXME If we're going to store the script task, find a way to do so safely. Currently it's -// only used for querying layout from arbitrary script. +//FIXME If we're going to store the page, find a way to do so safely. pub struct Window { - page: @mut Page, + page: *mut Page, script_chan: ScriptChan, compositor: @ScriptListener, wrapper: WrapperCache, @@ -91,13 +90,14 @@ impl Window { } pub fn content_changed(&self) { - self.page.reflow_all(ReflowForScriptQuery, self.script_chan.clone(), self.compositor); + unsafe { + (*self.page).reflow_all(ReflowForScriptQuery, self.script_chan.clone(), self.compositor); + } } - pub fn new(page: @mut Page, script_chan: ScriptChan, compositor: @ScriptListener) + pub fn new(page: *mut Page, script_chan: ScriptChan, compositor: @ScriptListener) -> @mut Window { let script_chan_clone = script_chan.clone(); - let page_id = page.id.clone(); let win = @mut Window { page: page, script_chan: script_chan, @@ -109,7 +109,7 @@ impl Window { loop { match timer_port.recv() { TimerMessage_Close => break, - TimerMessage_Fire(td) => script_chan_clone.chan.send(FireTimerMsg(page_id.clone(), td)), + TimerMessage_Fire(td) => unsafe {script_chan_clone.chan.send(FireTimerMsg((*page).id.clone(), td))}, TimerMessage_TriggerExit => script_chan_clone.chan.send(ExitMsg), } } @@ -118,8 +118,11 @@ impl Window { }, }; - let compartment = page.js_info.get_ref().js_compartment; - window::create(compartment, win); + unsafe { + // TODO(tkuehn): This just grabs the top-level page. Need to handle subframes. + let compartment = (*page).js_info.get_ref().js_compartment; + window::create(compartment, win); + } win } } diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 4a93901bc34..7e4c1aefb12 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -601,7 +601,7 @@ impl ScriptTask { let HtmlParserResult {root, js_port, style_port, iframe_port} = html_parsing_result; // Create the window and document objects. - let window = Window::new(page, self.chan.clone(), self.compositor); + let window = Window::new(&mut *page, self.chan.clone(), self.compositor); let document = Document(root, Some(window)); // Tie the root into the document.