From 326933cff4267951b4036f8f853969bc94316acd Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 15 Feb 2017 09:47:55 -0600 Subject: [PATCH 1/2] Link frame entries together that share pipelines Various frame-related cleanup --- components/constellation/constellation.rs | 163 ++++++++++------------ components/constellation/frame.rs | 132 ++++++++++++------ 2 files changed, 168 insertions(+), 127 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 16475c1a7fa5..1f8da322e5dd 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -75,7 +75,7 @@ use devtools_traits::{ChromeToDevtoolsControlMsg, DevtoolsControlMsg}; use euclid::scale_factor::ScaleFactor; use euclid::size::{Size2D, TypedSize2D}; use event_loop::EventLoop; -use frame::{Frame, FrameChange, FrameState, FrameTreeIterator, FullFrameTreeIterator}; +use frame::{Frame, FrameChange, FrameState, FrameTreeIterator, FullFrameTreeIterator, ReplaceOrUpdate}; use gfx::font_cache_thread::FontCacheThread; use gfx_traits::Epoch; use ipc_channel::SerializeError; @@ -110,7 +110,6 @@ use servo_remutex::ReentrantMutex; use servo_url::ServoUrl; use std::borrow::ToOwned; use std::collections::{HashMap, VecDeque}; -use std::iter::once; use std::marker::PhantomData; use std::process; use std::rc::{Rc, Weak}; @@ -614,7 +613,7 @@ impl Constellation .map(|pipeline| pipeline.visible); let prev_visibility = self.frames.get(&frame_id) - .and_then(|frame| self.pipelines.get(&frame.pipeline_id)) + .and_then(|frame| self.pipelines.get(&frame.pipeline_id())) .map(|pipeline| pipeline.visible) .or(parent_visibility); @@ -714,7 +713,7 @@ impl Constellation /// frame in the frame tree, sorted reverse chronologically. fn joint_session_past<'a>(&self, frame_id_root: FrameId) -> impl Iterator { let mut past: Vec<(Instant, FrameState)> = self.full_frame_tree_iter(frame_id_root) - .flat_map(|frame| frame.prev.iter().rev().scan(frame.instant, |prev_instant, entry| { + .flat_map(|frame| frame.prev.iter().rev().scan(frame.current.instant, |prev_instant, entry| { let instant = *prev_instant; *prev_instant = entry.instant; Some((instant, entry.clone())) @@ -1249,7 +1248,7 @@ impl Constellation // Notify the browser chrome that the pipeline has failed self.trigger_mozbrowsererror(top_level_frame_id, reason, backtrace); - let pipeline_id = self.frames.get(&top_level_frame_id).map(|frame| frame.pipeline_id); + let pipeline_id = self.frames.get(&top_level_frame_id).map(|frame| frame.pipeline_id()); let pipeline_url = pipeline_id.and_then(|id| self.pipelines.get(&id).map(|pipeline| pipeline.url.clone())); let parent_info = pipeline_id.and_then(|id| self.pipelines.get(&id).and_then(|pipeline| pipeline.parent_info)); let window_size = pipeline_id.and_then(|id| self.pipelines.get(&id).and_then(|pipeline| pipeline.size)); @@ -1409,7 +1408,8 @@ impl Constellation }; let replace = if load_info.info.replace { - self.frames.get(&load_info.info.frame_id).map(|frame| frame.current()) + self.frames.get(&load_info.info.frame_id).map(|frame| frame.current.clone()) + .map(|entry| ReplaceOrUpdate::Replace(entry)) } else { None }; @@ -1467,7 +1467,8 @@ impl Constellation }; let replace = if replace { - self.frames.get(&frame_id).map(|frame| frame.current()) + self.frames.get(&frame_id).map(|frame| frame.current.clone()) + .map(|entry| ReplaceOrUpdate::Replace(entry)) } else { None }; @@ -1534,7 +1535,7 @@ impl Constellation match self.frames.get(&self.root_frame_id) { None => warn!("Alert sent after root frame closure."), - Some(root_frame) => match self.pipelines.get(&root_frame.pipeline_id) { + Some(root_frame) => match self.pipelines.get(&root_frame.pipeline_id()) { None => warn!("Alert sent after root pipeline closure."), Some(root_pipeline) => root_pipeline.trigger_mozbrowser_event(Some(top_level_frame_id), event), } @@ -1611,7 +1612,8 @@ impl Constellation let root_frame_id = self.root_frame_id; let sandbox = IFrameSandboxState::IFrameUnsandboxed; let replace = if replace { - self.frames.get(&frame_id).map(|frame| frame.current()) + self.frames.get(&frame_id).map(|frame| frame.current.clone()) + .map(|entry| ReplaceOrUpdate::Replace(entry)) } else { None }; @@ -1709,7 +1711,7 @@ impl Constellation // frame's current pipeline. If neither exist, fall back to sending to // the compositor below. let root_pipeline_id = self.frames.get(&self.root_frame_id) - .map(|root_frame| root_frame.pipeline_id); + .map(|root_frame| root_frame.pipeline_id()); let pipeline_id = self.focus_pipeline_id.or(root_pipeline_id); match pipeline_id { @@ -1734,7 +1736,7 @@ impl Constellation fn handle_reload_msg(&mut self) { // Send Reload constellation msg to root script channel. let root_pipeline_id = self.frames.get(&self.root_frame_id) - .map(|root_frame| root_frame.pipeline_id); + .map(|root_frame| root_frame.pipeline_id()); if let Some(pipeline_id) = root_pipeline_id { let msg = ConstellationControlMsg::Reload(pipeline_id); @@ -1779,7 +1781,7 @@ impl Constellation resp_chan: IpcSender>) { let frame_id = frame_id.unwrap_or(self.root_frame_id); let current_pipeline_id = self.frames.get(&frame_id) - .map(|frame| frame.pipeline_id); + .map(|frame| frame.pipeline_id()); let pipeline_id_loaded = self.pending_frames.iter().rev() .find(|x| x.old_pipeline_id == current_pipeline_id) .map(|x| x.new_pipeline_id) @@ -1830,9 +1832,7 @@ impl Constellation fn handle_remove_iframe_msg(&mut self, frame_id: FrameId) -> Vec { let result = self.full_frame_tree_iter(frame_id) - .flat_map(|frame| frame.next.iter().chain(frame.prev.iter()) - .filter_map(|entry| entry.pipeline_id) - .chain(once(frame.pipeline_id))) + .flat_map(|frame| frame.entry_iter().filter_map(|entry| entry.pipeline_id())) .collect(); self.close_frame(frame_id, ExitPipelineMode::Normal); result @@ -1845,9 +1845,7 @@ impl Constellation }; let child_pipeline_ids: Vec = self.full_frame_tree_iter(frame_id) - .flat_map(|frame| frame.prev.iter().chain(frame.next.iter()) - .filter_map(|entry| entry.pipeline_id) - .chain(once(frame.pipeline_id))) + .flat_map(|frame| frame.entry_iter().filter_map(|entry| entry.pipeline_id())) .collect(); for id in child_pipeline_ids { @@ -1948,7 +1946,7 @@ impl Constellation }, WebDriverCommandMsg::TakeScreenshot(pipeline_id, reply) => { let current_pipeline_id = self.frames.get(&self.root_frame_id) - .map(|root_frame| root_frame.pipeline_id); + .map(|root_frame| root_frame.pipeline_id()); if Some(pipeline_id) == current_pipeline_id { self.compositor_proxy.send(ToCompositorMsg::CreatePng(reply)); } else { @@ -1964,33 +1962,34 @@ impl Constellation fn traverse_to_entry(&mut self, entry: FrameState) { // Step 1. let frame_id = entry.frame_id; - let pipeline_id = match entry.pipeline_id { + let pipeline_id = match entry.pipeline_id() { Some(pipeline_id) => pipeline_id, None => { // If there is no pipeline, then the document for this // entry has been discarded, so we navigate to the entry // URL instead. When the document has activated, it will - // traverse to the entry, but with the new pipeline id. + // traverse to the entry, but the new pipeline will exist now. debug!("Reloading document {} for frame {}.", entry.url, frame_id); // TODO: referrer? let load_data = LoadData::new(entry.url.clone(), None, None); // TODO: save the sandbox state so it can be restored here. let sandbox = IFrameSandboxState::IFrameUnsandboxed; - let new_pipeline_id = PipelineId::new(); let (old_pipeline_id, parent_info, window_size, is_private) = match self.frames.get(&frame_id) { - Some(frame) => match self.pipelines.get(&frame.pipeline_id) { - Some(pipeline) => (frame.pipeline_id, pipeline.parent_info, pipeline.size, pipeline.is_private), - None => (frame.pipeline_id, None, None, false), + Some(frame) => match self.pipelines.get(&frame.pipeline_id()) { + Some(pipeline) => (frame.pipeline_id(), pipeline.parent_info, + pipeline.size, pipeline.is_private), + None => (frame.pipeline_id(), None, None, false), }, None => return warn!("no frame to traverse"), }; + let new_pipeline_id = PipelineId::new(); self.new_pipeline(new_pipeline_id, frame_id, parent_info, window_size, load_data, sandbox, is_private); self.pending_frames.push(FrameChange { frame_id: frame_id, old_pipeline_id: Some(old_pipeline_id), new_pipeline_id: new_pipeline_id, url: entry.url.clone(), - replace: Some(entry), + replace: Some(ReplaceOrUpdate::Update(entry)), }); return; } @@ -2003,29 +2002,8 @@ impl Constellation let old_pipeline_id = match self.frames.get_mut(&frame_id) { Some(frame) => { - let old_pipeline_id = frame.pipeline_id; - let mut curr_entry = frame.current(); - - if entry.instant > frame.instant { - // We are traversing to the future. - while let Some(next) = frame.next.pop() { - frame.prev.push(curr_entry); - curr_entry = next; - if entry.instant <= curr_entry.instant { break; } - } - } else if entry.instant < frame.instant { - // We are traversing to the past. - while let Some(prev) = frame.prev.pop() { - frame.next.push(curr_entry); - curr_entry = prev; - if entry.instant >= curr_entry.instant { break; } - } - } - - debug_assert_eq!(entry.instant, curr_entry.instant); - - frame.update_current(pipeline_id, &entry); - + let old_pipeline_id = frame.pipeline_id(); + frame.traverse_to_entry(entry.clone()); old_pipeline_id }, None => return warn!("no frame to traverse"), @@ -2112,20 +2090,29 @@ impl Constellation } } - let (evicted_id, new_frame, navigated, location_changed) = if let Some(mut entry) = frame_change.replace { + let (evicted_id, new_frame, navigated, location_changed) = if let Some(replace) = frame_change.replace { debug!("Replacing pipeline in existing frame."); - let evicted_id = entry.pipeline_id; - entry.replace_pipeline(frame_change.new_pipeline_id, frame_change.url.clone()); + let (evicted_id, entry) = match replace { + ReplaceOrUpdate::Replace(mut entry) => { + let evicted_id = entry.pipeline_id(); + entry.replace_pipeline(frame_change.new_pipeline_id, frame_change.url.clone()); + (evicted_id, entry) + }, + ReplaceOrUpdate::Update(entry) => { + entry.update_pipeline(frame_change.new_pipeline_id); + (None, entry) + }, + }; self.traverse_to_entry(entry); (evicted_id, false, None, false) } else if let Some(frame) = self.frames.get_mut(&frame_change.frame_id) { debug!("Adding pipeline to existing frame."); - let old_pipeline_id = frame.pipeline_id; + let old_pipeline_id = frame.pipeline_id(); frame.load(frame_change.new_pipeline_id, frame_change.url.clone()); let evicted_id = frame.prev.len() .checked_sub(PREFS.get("session-history.max-length").as_u64().unwrap_or(20) as usize) - .and_then(|index| frame.prev.get_mut(index)) - .and_then(|entry| entry.pipeline_id.take()); + .and_then(|index| frame.prev.get(index)) + .and_then(|entry| frame.discard_entry(entry)); (evicted_id, false, Some(old_pipeline_id), true) } else { debug!("Adding pipeline to new frame."); @@ -2192,7 +2179,7 @@ impl Constellation if let Some(frame) = self.frames.get(&self.root_frame_id) { // Send Resize (or ResizeInactive) messages to each // pipeline in the frame tree. - let pipeline_id = frame.pipeline_id; + let pipeline_id = frame.pipeline_id(); let pipeline = match self.pipelines.get(&pipeline_id) { None => return warn!("Pipeline {:?} resized after closing.", pipeline_id), Some(pipeline) => pipeline, @@ -2202,10 +2189,13 @@ impl Constellation new_size, size_type )); - let pipelines = frame.prev.iter().chain(frame.next.iter()) - .filter_map(|entry| entry.pipeline_id) + + // TODO(cbrewster): Once we allow entries to share pipeline ids, we need to dedup. + let inactive_pipelines = frame.entry_iter() + .filter_map(|entry| entry.pipeline_id()) + .filter(|id| id != &pipeline_id) .filter_map(|pipeline_id| self.pipelines.get(&pipeline_id)); - for pipeline in pipelines { + for pipeline in inactive_pipelines { let _ = pipeline.event_loop.send(ConstellationControlMsg::ResizeInactive( pipeline.id, new_size @@ -2274,7 +2264,7 @@ impl Constellation // are met, then the output image should not change and a reftest // screenshot can safely be written. for frame in self.current_frame_tree_iter(self.root_frame_id) { - let pipeline_id = frame.pipeline_id; + let pipeline_id = frame.pipeline_id(); debug!("Checking readiness of frame {}, pipeline {}.", frame.id, pipeline_id); let pipeline = match self.pipelines.get(&pipeline_id) { @@ -2302,7 +2292,7 @@ impl Constellation } // See if this pipeline has reached idle script state yet. - match self.document_states.get(&frame.pipeline_id) { + match self.document_states.get(&frame.pipeline_id()) { Some(&DocumentState::Idle) => {} Some(&DocumentState::Pending) | None => { return ReadyToSave::DocumentLoading; @@ -2322,7 +2312,7 @@ impl Constellation } // Get the epoch that the compositor has drawn for this pipeline. - let compositor_epoch = pipeline_states.get(&frame.pipeline_id); + let compositor_epoch = pipeline_states.get(&frame.pipeline_id()); match compositor_epoch { Some(compositor_epoch) => { // Synchronously query the layout thread to see if the current @@ -2359,7 +2349,7 @@ impl Constellation loop { if let Some(ancestor) = self.pipelines.get(&ancestor_id) { if let Some(frame) = self.frames.get(&ancestor.frame_id) { - if frame.pipeline_id == ancestor_id { + if frame.pipeline_id() == ancestor_id { if let Some((parent_id, FrameType::IFrame)) = ancestor.parent_info { ancestor_id = parent_id; continue; @@ -2389,7 +2379,7 @@ impl Constellation }; for child_id in &pipeline.children { if let Some(child) = self.frames.get(child_id) { - self.set_activity(child.pipeline_id, child_activity); + self.set_activity(child.pipeline_id(), child_activity); } } } @@ -2410,7 +2400,7 @@ impl Constellation None => continue, }; for entry in evicted { - if let Some(pipeline_id) = entry.pipeline_id { + if let Some(pipeline_id) = entry.pipeline_id() { self.close_pipeline(pipeline_id, DiscardBrowsingContext::No, ExitPipelineMode::Normal); } } @@ -2421,7 +2411,7 @@ impl Constellation fn close_frame(&mut self, frame_id: FrameId, exit_mode: ExitPipelineMode) { debug!("Closing frame {}.", frame_id); let parent_info = self.frames.get(&frame_id) - .and_then(|frame| self.pipelines.get(&frame.pipeline_id)) + .and_then(|frame| self.pipelines.get(&frame.pipeline_id())) .and_then(|pipeline| pipeline.parent_info); self.close_frame_children(frame_id, DiscardBrowsingContext::Yes, exit_mode); @@ -2454,9 +2444,9 @@ impl Constellation .collect(); if let Some(frame) = self.frames.get(&frame_id) { - pipelines_to_close.extend(frame.next.iter().filter_map(|state| state.pipeline_id)); - pipelines_to_close.push(frame.pipeline_id); - pipelines_to_close.extend(frame.prev.iter().filter_map(|state| state.pipeline_id)); + pipelines_to_close.extend(frame.next.iter().filter_map(|state| state.pipeline_id())); + pipelines_to_close.push(frame.pipeline_id()); + pipelines_to_close.extend(frame.prev.iter().filter_map(|state| state.pipeline_id())); } for pipeline_id in pipelines_to_close { @@ -2476,8 +2466,19 @@ impl Constellation let frames_to_close = { let mut frames_to_close = vec!(); - if let Some(pipeline) = self.pipelines.get(&pipeline_id) { - frames_to_close.extend_from_slice(&pipeline.children); + match self.pipelines.get(&pipeline_id) { + Some(pipeline) => { + frames_to_close.extend_from_slice(&pipeline.children); + // Inform script, compositor that this pipeline has exited. + // Note, we don't remove the pipeline now, we wait for the message to come back from + // the pipeline. + match exit_mode { + ExitPipelineMode::Normal => pipeline.exit(dbc), + ExitPipelineMode::Force => pipeline.force_exit(dbc), + } + debug!("Closed pipeline {:?}.", pipeline_id); + }, + None => return warn!("Closing pipeline {:?} twice.", pipeline_id), } frames_to_close @@ -2488,13 +2489,6 @@ impl Constellation self.close_frame(*child_frame, exit_mode); } - // Note, we don't remove the pipeline now, we wait for the message to come back from - // the pipeline. - let pipeline = match self.pipelines.get(&pipeline_id) { - Some(pipeline) => pipeline, - None => return warn!("Closing pipeline {:?} twice.", pipeline_id), - }; - // Remove this pipeline from pending frames if it hasn't loaded yet. let pending_index = self.pending_frames.iter().position(|frame_change| { frame_change.new_pipeline_id == pipeline_id @@ -2502,13 +2496,6 @@ impl Constellation if let Some(pending_index) = pending_index { self.pending_frames.remove(pending_index); } - - // Inform script, compositor that this pipeline has exited. - match exit_mode { - ExitPipelineMode::Normal => pipeline.exit(dbc), - ExitPipelineMode::Force => pipeline.force_exit(dbc), - } - debug!("Closed pipeline {:?}.", pipeline_id); } // Randomly close a pipeline -if --random-pipeline-closure-probability is set @@ -2540,7 +2527,7 @@ impl Constellation // Convert a frame to a sendable form to pass to the compositor fn frame_to_sendable(&self, frame_id: FrameId) -> Option { self.frames.get(&frame_id).and_then(|frame: &Frame| { - self.pipelines.get(&frame.pipeline_id).map(|pipeline: &Pipeline| { + self.pipelines.get(&frame.pipeline_id()).map(|pipeline: &Pipeline| { let mut frame_tree = SendableFrameTree { pipeline: pipeline.to_sendable(), size: pipeline.size, @@ -2621,7 +2608,7 @@ impl Constellation match self.frames.get(&top_level_frame_id) { None => warn!("Mozbrowser error after top-level frame closed."), - Some(frame) => match self.pipelines.get(&frame.pipeline_id) { + Some(frame) => match self.pipelines.get(&frame.pipeline_id()) { None => warn!("Mozbrowser error after top-level pipeline closed."), Some(pipeline) => match pipeline.parent_info { None => pipeline.trigger_mozbrowser_event(None, event), @@ -2648,7 +2635,7 @@ impl Constellation pipeline_id: PipelineId, root_frame_id: FrameId) -> bool { self.current_frame_tree_iter(root_frame_id) - .any(|current_frame| current_frame.pipeline_id == pipeline_id) + .any(|current_frame| current_frame.pipeline_id() == pipeline_id) } } diff --git a/components/constellation/frame.rs b/components/constellation/frame.rs index daa443b2b80f..9616053c7c12 100644 --- a/components/constellation/frame.rs +++ b/components/constellation/frame.rs @@ -5,9 +5,11 @@ use msg::constellation_msg::{FrameId, PipelineId}; use pipeline::Pipeline; use servo_url::ServoUrl; +use std::cell::Cell; use std::collections::HashMap; use std::iter::once; use std::mem::replace; +use std::rc::Rc; use std::time::Instant; /// A frame in the frame tree. @@ -23,14 +25,8 @@ pub struct Frame { /// The frame id. pub id: FrameId, - /// The timestamp for the current session history entry - pub instant: Instant, - - /// The pipeline for the current session history entry - pub pipeline_id: PipelineId, - - /// The URL for the current session history entry - pub url: ServoUrl, + /// The current session history entry. + pub current: FrameState, /// The past session history, ordered chronologically. pub prev: Vec, @@ -45,31 +41,23 @@ impl Frame { pub fn new(id: FrameId, pipeline_id: PipelineId, url: ServoUrl) -> Frame { Frame { id: id, - pipeline_id: pipeline_id, - instant: Instant::now(), - url: url, + current: FrameState::new(pipeline_id, url, id), prev: vec!(), next: vec!(), } } - /// Get the current frame state. - pub fn current(&self) -> FrameState { - FrameState { - instant: self.instant, - frame_id: self.id, - pipeline_id: Some(self.pipeline_id), - url: self.url.clone(), - } + /// The active pipeline id of the current session history entry + pub fn pipeline_id(&self) -> PipelineId { + self.current.pipeline_id().expect("Active pipeline cannot be None") } /// Set the current frame entry, and push the current frame entry into the past. pub fn load(&mut self, pipeline_id: PipelineId, url: ServoUrl) { - let current = self.current(); + let current = self.current.clone(); self.prev.push(current); - self.instant = Instant::now(); - self.pipeline_id = pipeline_id; - self.url = url; + self.current.instant = Instant::now(); + self.current.replace_pipeline(pipeline_id, url); } /// Set the future to be empty. @@ -77,11 +65,44 @@ impl Frame { replace(&mut self.next, vec!()) } - /// Update the current entry of the Frame from an entry that has been traversed to. - pub fn update_current(&mut self, pipeline_id: PipelineId, entry: &FrameState) { - self.pipeline_id = pipeline_id; - self.instant = entry.instant; - self.url = entry.url.clone(); + pub fn discard_entry(&self, entry: &FrameState) -> Option { + // Ensure that we are not discarding the active pipeline. + if entry.pipeline_id() != Some(self.pipeline_id()) { + return entry.discard_pipeline() + } + None + } + + pub fn traverse_to_entry(&mut self, entry: FrameState) { + if entry.pipeline_id().is_none() { + return warn!("Attempted to traverse frame {} to entry with a discarded pipeline.", entry.frame_id); + } + debug_assert!(entry.pipeline_id().is_some()); + let mut curr_entry = self.current.clone(); + + if entry.instant > self.current.instant { + // We are traversing to the future. + while let Some(next) = self.next.pop() { + self.prev.push(curr_entry); + curr_entry = next; + if entry.instant <= curr_entry.instant { break; } + } + } else if entry.instant < self.current.instant { + // We are traversing to the past. + while let Some(prev) = self.prev.pop() { + self.next.push(curr_entry); + curr_entry = prev; + if entry.instant >= curr_entry.instant { break; } + } + } + + debug_assert_eq!(entry.instant, curr_entry.instant); + + self.current = entry; + } + + pub fn entry_iter<'a>(&'a self) -> impl Iterator { + self.prev.iter().chain(self.next.iter()).chain(once(&self.current)) } } @@ -95,24 +116,58 @@ pub struct FrameState { /// The timestamp for when the session history entry was created pub instant: Instant, - /// The pipeline for the document in the session history, - /// None if the entry has been discarded - pub pipeline_id: Option, - /// The URL for this entry, used to reload the pipeline if it has been discarded pub url: ServoUrl, /// The frame that this session history entry is part of pub frame_id: FrameId, + + /// The pipeline for the document in the session history + pipeline_id: Rc>>, } impl FrameState { + fn new(pipeline_id: PipelineId, url: ServoUrl, frame_id: FrameId) -> FrameState { + FrameState { + instant: Instant::now(), + pipeline_id: Rc::new(Cell::new(Some(pipeline_id))), + url: url, + frame_id: frame_id, + } + } + /// Updates the entry's pipeline and url. This is used when navigating with replacement /// enabled. pub fn replace_pipeline(&mut self, pipeline_id: PipelineId, url: ServoUrl) { - self.pipeline_id = Some(pipeline_id); + self.pipeline_id = Rc::new(Cell::new(Some(pipeline_id))); self.url = url; } + + fn discard_pipeline(&self) -> Option { + let old_pipeline_id = self.pipeline_id.get(); + self.pipeline_id.set(None); + old_pipeline_id + } + + pub fn update_pipeline(&self, pipeline_id: PipelineId) { + self.pipeline_id.set(Some(pipeline_id)); + } + + pub fn pipeline_id(&self) -> Option { + self.pipeline_id.get() + } +} + +/// Whether the pipeline should be updated or replaced when a naviagtion with replacement +/// matures. +pub enum ReplaceOrUpdate { + /// Replaces the pipeline of the entry. This does not affect any other entries even + /// if they share a pipeline id. + Replace(FrameState), + /// Updates the pipeline of the entry. This will update the pipeline of all entries + /// that share the same pipeline. This is only used when reloading a document that + /// was discarded from the distant history. + Update(FrameState), } /// Represents a pending change in the frame tree, that will be applied @@ -133,7 +188,7 @@ pub struct FrameChange { /// Is the new document replacing the current document (e.g. a reload) /// or pushing it into the session history (e.g. a navigation)? - pub replace: Option, + pub replace: Option, } /// An iterator over a frame tree, returning the fully active frames in @@ -168,10 +223,10 @@ impl<'a> Iterator for FrameTreeIterator<'a> { continue; }, }; - let pipeline = match self.pipelines.get(&frame.pipeline_id) { + let pipeline = match self.pipelines.get(&frame.pipeline_id()) { Some(pipeline) => pipeline, None => { - warn!("Pipeline {:?} iterated after closure.", frame.pipeline_id); + warn!("Pipeline {:?} iterated after closure.", frame.pipeline_id()); continue; }, }; @@ -213,9 +268,8 @@ impl<'a> Iterator for FullFrameTreeIterator<'a> { continue; }, }; - let child_frame_ids = frame.prev.iter().chain(frame.next.iter()) - .filter_map(|entry| entry.pipeline_id) - .chain(once(frame.pipeline_id)) + let child_frame_ids = frame.entry_iter() + .filter_map(|entry| entry.pipeline_id()) .filter_map(|pipeline_id| pipelines.get(&pipeline_id)) .flat_map(|pipeline| pipeline.children.iter()); self.stack.extend(child_frame_ids); From e64556b8317e9762490c121f115182100f0b6205 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Wed, 15 Feb 2017 10:24:32 -0600 Subject: [PATCH 2/2] Add constellation frame unit tests --- Cargo.lock | 10 ++++ components/constellation/lib.rs | 1 + ports/servo/Cargo.toml | 1 + tests/unit/constellation/Cargo.toml | 15 ++++++ tests/unit/constellation/frame.rs | 71 +++++++++++++++++++++++++++++ tests/unit/constellation/lib.rs | 9 ++++ 6 files changed, 107 insertions(+) create mode 100644 tests/unit/constellation/Cargo.toml create mode 100644 tests/unit/constellation/frame.rs create mode 100644 tests/unit/constellation/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 73737969de29..b36a85b5f62f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -483,6 +483,15 @@ dependencies = [ "webvr_traits 0.0.1", ] +[[package]] +name = "constellation_tests" +version = "0.0.1" +dependencies = [ + "constellation 0.0.1", + "msg 0.0.1", + "servo_url 0.0.1", +] + [[package]] name = "content-blocker" version = "0.2.2" @@ -2504,6 +2513,7 @@ dependencies = [ "backtrace 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "browserhtml 0.1.17 (git+https://github.com/browserhtml/browserhtml?branch=crate)", "compiletest_helper 0.0.1", + "constellation_tests 0.0.1", "gfx_tests 0.0.1", "glutin_app 0.0.1", "layout_tests 0.0.1", diff --git a/components/constellation/lib.rs b/components/constellation/lib.rs index 92417412887b..e1856dd6c092 100644 --- a/components/constellation/lib.rs +++ b/components/constellation/lib.rs @@ -53,3 +53,4 @@ pub use constellation::{Constellation, FromCompositorLogger, FromScriptLogger, I pub use pipeline::UnprivilegedPipelineContent; #[cfg(not(target_os = "windows"))] pub use sandboxing::content_process_sandbox_profile; +pub use frame::Frame; diff --git a/ports/servo/Cargo.toml b/ports/servo/Cargo.toml index 082d17db90bc..83b64bf49d2a 100644 --- a/ports/servo/Cargo.toml +++ b/ports/servo/Cargo.toml @@ -15,6 +15,7 @@ bench = false [dev-dependencies] compiletest_helper = {path = "../../tests/compiletest/helper"} +constellation_tests = {path = "../../tests/unit/constellation"} gfx_tests = {path = "../../tests/unit/gfx"} layout_tests = {path = "../../tests/unit/layout"} net_tests = {path = "../../tests/unit/net"} diff --git a/tests/unit/constellation/Cargo.toml b/tests/unit/constellation/Cargo.toml new file mode 100644 index 000000000000..7dfa6dc5295f --- /dev/null +++ b/tests/unit/constellation/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "constellation_tests" +version = "0.0.1" +authors = ["The Servo Project Developers"] +license = "MPL-2.0" + +[lib] +name = "constellation_tests" +path = "lib.rs" +doctest = false + +[dependencies] +constellation = {path = "../../../components/constellation"} +msg = {path = "../../../components/msg"} +servo_url = {path = "../../../components/url"} diff --git a/tests/unit/constellation/frame.rs b/tests/unit/constellation/frame.rs new file mode 100644 index 000000000000..83b2b89a8afb --- /dev/null +++ b/tests/unit/constellation/frame.rs @@ -0,0 +1,71 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +use constellation::Frame; +use msg::constellation_msg::{PipelineId, PipelineNamespace, PipelineNamespaceId, FrameId}; +use servo_url::ServoUrl; + +#[test] +fn test_entry_replacement() { + PipelineNamespace::install(PipelineNamespaceId(0)); + let pipeline_id = PipelineId::new(); + let frame_id = FrameId::new(); + let url = ServoUrl::parse("about:blank").expect("Infallible"); + let mut frame = Frame::new(frame_id, pipeline_id, url.clone()); + + frame.prev.push(frame.current.clone()); + + let new_pipeline_id = PipelineId::new(); + // Unlink this entry from the previous entry + frame.current.replace_pipeline(new_pipeline_id, url); + + assert_eq!(frame.prev.len(), 1); + let prev_entry = frame.prev.pop().expect("No previous entry!"); + assert_ne!(prev_entry.pipeline_id(), frame.current.pipeline_id()); +} + +#[test] +fn test_entry_update() { + PipelineNamespace::install(PipelineNamespaceId(0)); + let pipeline_id = PipelineId::new(); + let frame_id = FrameId::new(); + FrameId::install(frame_id); + let url = ServoUrl::parse("about:blank").expect("Infallible"); + let mut frame = Frame::new(frame_id, pipeline_id, url); + + // A clone will link the two entry's pipelines + frame.prev.push(frame.current.clone()); + + let new_pipeline_id = PipelineId::new(); + frame.current.update_pipeline(new_pipeline_id); + assert_eq!(frame.pipeline_id(), new_pipeline_id); + + assert_eq!(frame.prev.len(), 1); + let prev_entry = frame.prev.pop().expect("No previous entry!"); + assert_eq!(prev_entry.pipeline_id(), frame.current.pipeline_id()); +} + +#[test] +fn test_entry_discard() { + PipelineNamespace::install(PipelineNamespaceId(0)); + let pipeline_id = PipelineId::new(); + let frame_id = FrameId::new(); + FrameId::install(frame_id); + let url = ServoUrl::parse("about:blank").expect("Infallible"); + let mut frame = Frame::new(frame_id, pipeline_id, url.clone()); + + // A clone will link the two entry's pipelines + frame.prev.push(frame.current.clone()); + + assert_eq!(frame.prev.len(), 1); + // Cannot discard because this entry shares the same pipeline as the current entry. + let evicted_id = frame.discard_entry(&frame.prev[0]); + assert!(evicted_id.is_none()); + + let new_pipeline_id = PipelineId::new(); + frame.current.replace_pipeline(new_pipeline_id, url); + // Discard should work now that current is no longer linked to this entry. + let evicted_id = frame.discard_entry(&frame.prev[0]); + assert_eq!(evicted_id, Some(pipeline_id)); +} diff --git a/tests/unit/constellation/lib.rs b/tests/unit/constellation/lib.rs new file mode 100644 index 000000000000..4ec4553e8414 --- /dev/null +++ b/tests/unit/constellation/lib.rs @@ -0,0 +1,9 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +extern crate constellation; +extern crate msg; +extern crate servo_url; + +#[cfg(test)] mod frame;