From aecb48d5d1c9bb07fcefae8748d82526aa4ba1c9 Mon Sep 17 00:00:00 2001 From: Connor Brewster Date: Thu, 6 Dec 2018 11:59:09 -0600 Subject: [PATCH] Cleanup constellation after nll --- components/constellation/constellation.rs | 245 ++++++++++------------ 1 file changed, 110 insertions(+), 135 deletions(-) diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 62ebebf9ebe2..828db1ce88eb 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -1599,20 +1599,14 @@ where EmbedderMsg::Panic(reason, backtrace), )); - let (window_size, pipeline_id, is_visible) = { - let browsing_context = self.browsing_contexts.get(&browsing_context_id); - let window_size = browsing_context.and_then(|ctx| ctx.size); - let pipeline_id = browsing_context.map(|ctx| ctx.pipeline_id); - let is_visible = browsing_context.map(|ctx| ctx.is_visible); - (window_size, pipeline_id, is_visible) - }; + let browsing_context = self.browsing_contexts.get(&browsing_context_id); + let window_size = browsing_context.and_then(|ctx| ctx.size); + let pipeline_id = browsing_context.map(|ctx| ctx.pipeline_id); + let is_visible = browsing_context.map(|ctx| ctx.is_visible); - let (pipeline_url, opener) = { - let pipeline = pipeline_id.and_then(|id| self.pipelines.get(&id)); - let pipeline_url = pipeline.map(|pipeline| pipeline.url.clone()); - let opener = pipeline.and_then(|pipeline| pipeline.opener); - (pipeline_url, opener) - }; + let pipeline = pipeline_id.and_then(|id| self.pipelines.get(&id)); + let pipeline_url = pipeline.map(|pipeline| pipeline.url.clone()); + let opener = pipeline.and_then(|pipeline| pipeline.opener); self.close_browsing_context_children( browsing_context_id, @@ -1855,65 +1849,56 @@ where replace, } = load_info.info; - let (load_data, is_private) = { - // If no url is specified, reload. - let old_pipeline = load_info - .old_pipeline_id - .and_then(|id| self.pipelines.get(&id)); - let load_data = load_info.load_data.unwrap_or_else(|| { - let url = match old_pipeline { - Some(old_pipeline) => old_pipeline.url.clone(), - None => ServoUrl::parse("about:blank").expect("infallible"), - }; - - // TODO - loaddata here should have referrer info (not None, None) - LoadData::new(url, Some(parent_pipeline_id), None, None) - }); - - let is_parent_private = { - let parent_browsing_context_id = match self.pipelines.get(&parent_pipeline_id) { - Some(pipeline) => pipeline.browsing_context_id, - None => { - return warn!( - "Script loaded url in iframe {} in closed parent pipeline {}.", - browsing_context_id, parent_pipeline_id, - ) - }, - }; - let is_parent_private = - match self.browsing_contexts.get(&parent_browsing_context_id) { - Some(ctx) => ctx.is_private, - None => { - return warn!( - "Script loaded url in iframe {} in closed parent browsing context {}.", - browsing_context_id, - parent_browsing_context_id, - ) - }, - }; - is_parent_private + // If no url is specified, reload. + let old_pipeline = load_info + .old_pipeline_id + .and_then(|id| self.pipelines.get(&id)); + let load_data = load_info.load_data.unwrap_or_else(|| { + let url = match old_pipeline { + Some(old_pipeline) => old_pipeline.url.clone(), + None => ServoUrl::parse("about:blank").expect("infallible"), }; - let is_private = is_private || is_parent_private; - (load_data, is_private) - }; + // TODO - loaddata here should have referrer info (not None, None) + LoadData::new(url, Some(parent_pipeline_id), None, None) + }); - let (replace, window_size, is_visible) = { - let browsing_context = match self.browsing_contexts.get(&browsing_context_id) { - Some(ctx) => ctx, + let is_parent_private = { + let parent_browsing_context_id = match self.pipelines.get(&parent_pipeline_id) { + Some(pipeline) => pipeline.browsing_context_id, None => { return warn!( - "Script loaded url in iframe with closed browsing context {}.", - browsing_context_id, + "Script loaded url in iframe {} in closed parent pipeline {}.", + browsing_context_id, parent_pipeline_id, ) }, }; - let replace = if replace { - Some(NeedsToReload::No(browsing_context.pipeline_id)) - } else { - None + let is_parent_private = match self.browsing_contexts.get(&parent_browsing_context_id) { + Some(ctx) => ctx.is_private, + None => { + return warn!( + "Script loaded url in iframe {} in closed parent browsing context {}.", + browsing_context_id, parent_browsing_context_id, + ) + }, }; - (replace, browsing_context.size, browsing_context.is_visible) + is_parent_private + }; + let is_private = is_private || is_parent_private; + + let browsing_context = match self.browsing_contexts.get(&browsing_context_id) { + Some(ctx) => ctx, + None => { + return warn!( + "Script loaded url in iframe with closed browsing context {}.", + browsing_context_id, + ) + }, + }; + let replace = if replace { + Some(NeedsToReload::No(browsing_context.pipeline_id)) + } else { + None }; // Create the new pipeline, attached to the parent and push to pending changes @@ -1923,11 +1908,11 @@ where top_level_browsing_context_id, Some(parent_pipeline_id), None, - window_size, + browsing_context.size, load_data.clone(), load_info.sandbox, is_private, - is_visible, + browsing_context.is_visible, ); self.add_pending_change(SessionHistoryChange { top_level_browsing_context_id: top_level_browsing_context_id, @@ -1958,40 +1943,34 @@ where // TODO: Referrer? let load_data = LoadData::new(url.clone(), Some(parent_pipeline_id), None, None); - let (pipeline, is_private, is_visible) = { - let (script_sender, parent_browsing_context_id) = match self - .pipelines - .get(&parent_pipeline_id) - { + let (script_sender, parent_browsing_context_id) = + match self.pipelines.get(&parent_pipeline_id) { Some(pipeline) => (pipeline.event_loop.clone(), pipeline.browsing_context_id), None => return warn!("Script loaded url in closed iframe {}.", parent_pipeline_id), }; - let (is_parent_private, is_parent_visible) = - match self.browsing_contexts.get(&parent_browsing_context_id) { - Some(ctx) => (ctx.is_private, ctx.is_visible), - None => { - return warn!( - "New iframe {} loaded in closed parent browsing context {}.", - browsing_context_id, parent_browsing_context_id, - ) - }, - }; - let is_private = is_private || is_parent_private; - let pipeline = Pipeline::new( - new_pipeline_id, - browsing_context_id, - top_level_browsing_context_id, - None, - script_sender, - layout_sender, - self.compositor_proxy.clone(), - url, - is_parent_visible, - load_data, - ); - - (pipeline, is_private, is_parent_visible) - }; + let (is_parent_private, is_parent_visible) = + match self.browsing_contexts.get(&parent_browsing_context_id) { + Some(ctx) => (ctx.is_private, ctx.is_visible), + None => { + return warn!( + "New iframe {} loaded in closed parent browsing context {}.", + browsing_context_id, parent_browsing_context_id, + ) + }, + }; + let is_private = is_private || is_parent_private; + let pipeline = Pipeline::new( + new_pipeline_id, + browsing_context_id, + top_level_browsing_context_id, + None, + script_sender, + layout_sender, + self.compositor_proxy.clone(), + url, + is_parent_visible, + load_data, + ); assert!(!self.pipelines.contains_key(&new_pipeline_id)); self.pipelines.insert(new_pipeline_id, pipeline); @@ -2004,7 +1983,7 @@ where new_browsing_context_info: Some(NewBrowsingContextInfo { parent_pipeline_id: Some(parent_pipeline_id), is_private: is_private, - is_visible: is_visible, + is_visible: is_parent_visible, }), }); } @@ -2026,42 +2005,38 @@ where // TODO: Referrer? let load_data = LoadData::new(url.clone(), None, None, None); - let (pipeline, is_private, is_visible) = { - let (script_sender, opener_browsing_context_id) = - match self.pipelines.get(&opener_pipeline_id) { - Some(pipeline) => (pipeline.event_loop.clone(), pipeline.browsing_context_id), - None => { - return warn!( - "Auxiliary loaded url in closed iframe {}.", - opener_pipeline_id - ) - }, - }; - let (is_opener_private, is_opener_visible) = - match self.browsing_contexts.get(&opener_browsing_context_id) { - Some(ctx) => (ctx.is_private, ctx.is_visible), - None => { - return warn!( - "New auxiliary {} loaded in closed opener browsing context {}.", - new_browsing_context_id, opener_browsing_context_id, - ) - }, - }; - let pipeline = Pipeline::new( - new_pipeline_id, - new_browsing_context_id, - new_top_level_browsing_context_id, - Some(opener_browsing_context_id), - script_sender, - layout_sender, - self.compositor_proxy.clone(), - url, - is_opener_visible, - load_data, - ); - - (pipeline, is_opener_private, is_opener_visible) - }; + let (script_sender, opener_browsing_context_id) = + match self.pipelines.get(&opener_pipeline_id) { + Some(pipeline) => (pipeline.event_loop.clone(), pipeline.browsing_context_id), + None => { + return warn!( + "Auxiliary loaded url in closed iframe {}.", + opener_pipeline_id + ) + }, + }; + let (is_opener_private, is_opener_visible) = + match self.browsing_contexts.get(&opener_browsing_context_id) { + Some(ctx) => (ctx.is_private, ctx.is_visible), + None => { + return warn!( + "New auxiliary {} loaded in closed opener browsing context {}.", + new_browsing_context_id, opener_browsing_context_id, + ) + }, + }; + let pipeline = Pipeline::new( + new_pipeline_id, + new_browsing_context_id, + new_top_level_browsing_context_id, + Some(opener_browsing_context_id), + script_sender, + layout_sender, + self.compositor_proxy.clone(), + url, + is_opener_visible, + load_data, + ); assert!(!self.pipelines.contains_key(&new_pipeline_id)); self.pipelines.insert(new_pipeline_id, pipeline); @@ -2080,8 +2055,8 @@ where new_browsing_context_info: Some(NewBrowsingContextInfo { // Auxiliary browsing contexts are always top-level. parent_pipeline_id: None, - is_private: is_private, - is_visible: is_visible, + is_private: is_opener_private, + is_visible: is_opener_visible, }), }); }