From 231a37be24ee62cef69d69e0d1919b7b7f80cfed Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Sun, 9 Dec 2018 14:16:14 -0500 Subject: [PATCH] Initial window sizes are mandatory. --- components/compositing/lib.rs | 3 - components/constellation/browsingcontext.rs | 5 +- components/constellation/constellation.rs | 109 ++++++++++---------- components/constellation/pipeline.rs | 13 ++- components/script/dom/document.rs | 19 +--- components/script/dom/htmliframeelement.rs | 4 +- components/script/dom/htmlimageelement.rs | 23 ++--- components/script/dom/mediaquerylist.rs | 6 +- components/script/dom/window.rs | 37 +++---- components/script/dom/windowproxy.rs | 2 +- components/script/script_thread.rs | 6 +- components/script_traits/lib.rs | 4 +- 12 files changed, 101 insertions(+), 130 deletions(-) diff --git a/components/compositing/lib.rs b/components/compositing/lib.rs index 8715b52eb10..8ffbb9196b2 100644 --- a/components/compositing/lib.rs +++ b/components/compositing/lib.rs @@ -11,12 +11,10 @@ pub use crate::compositor::IOCompositor; pub use crate::compositor::RenderNotifier; pub use crate::compositor::ShutdownState; pub use crate::compositor_thread::CompositorProxy; -use euclid::TypedSize2D; use ipc_channel::ipc::IpcSender; use msg::constellation_msg::PipelineId; use msg::constellation_msg::TopLevelBrowsingContextId; use script_traits::{ConstellationControlMsg, LayoutControlMsg}; -use style_traits::CSSPixel; mod compositor; pub mod compositor_thread; @@ -27,7 +25,6 @@ pub mod windowing; pub struct SendableFrameTree { pub pipeline: CompositionPipeline, - pub size: Option>, pub children: Vec, } diff --git a/components/constellation/browsingcontext.rs b/components/constellation/browsingcontext.rs index 99875f3b431..e11aada3b2a 100644 --- a/components/constellation/browsingcontext.rs +++ b/components/constellation/browsingcontext.rs @@ -41,7 +41,7 @@ pub struct BrowsingContext { pub top_level_id: TopLevelBrowsingContextId, /// The size of the frame. - pub size: Option>, + pub size: TypedSize2D, /// Whether this browsing context is in private browsing mode. pub is_private: bool, @@ -70,6 +70,7 @@ impl BrowsingContext { top_level_id: TopLevelBrowsingContextId, pipeline_id: PipelineId, parent_pipeline_id: Option, + size: TypedSize2D, is_private: bool, is_visible: bool, ) -> BrowsingContext { @@ -78,7 +79,7 @@ impl BrowsingContext { BrowsingContext { id: id, top_level_id: top_level_id, - size: None, + size: size, is_private: is_private, is_visible: is_visible, pipeline_id: pipeline_id, diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs index 8195e4efc77..a54169d66db 100644 --- a/components/constellation/constellation.rs +++ b/components/constellation/constellation.rs @@ -734,7 +734,7 @@ where top_level_browsing_context_id: TopLevelBrowsingContextId, parent_pipeline_id: Option, opener: Option, - initial_window_size: Option>, + initial_window_size: TypedSize2D, // TODO: we have to provide ownership of the LoadData // here, because it will be send on an ipc channel, // and ipc channels take onership of their data. @@ -889,6 +889,7 @@ where top_level_id: TopLevelBrowsingContextId, pipeline_id: PipelineId, parent_pipeline_id: Option, + size: TypedSize2D, is_private: bool, is_visible: bool, ) { @@ -898,6 +899,7 @@ where top_level_id, pipeline_id, parent_pipeline_id, + size, is_private, is_visible, ); @@ -1599,14 +1601,20 @@ where EmbedderMsg::Panic(reason, backtrace), )); - 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 browsing_context = match self.browsing_contexts.get(&browsing_context_id) { + Some(context) => context, + None => return warn!("failed browsing context is missing"), + }; + let window_size = browsing_context.size; + let pipeline_id = browsing_context.pipeline_id; + let is_visible = browsing_context.is_visible; - 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); + let pipeline = match self.pipelines.get(&pipeline_id) { + Some(p) => p, + None => return warn!("failed pipeline is missing"), + }; + let pipeline_url = pipeline.url.clone(); + let opener = pipeline.opener; self.close_browsing_context_children( browsing_context_id, @@ -1616,10 +1624,8 @@ where let failure_url = ServoUrl::parse("about:failure").expect("infallible"); - if let Some(pipeline_url) = pipeline_url { - if pipeline_url == failure_url { - return error!("about:failure failed"); - } + if pipeline_url == failure_url { + return error!("about:failure failed"); } warn!("creating replacement pipeline for about:failure"); @@ -1638,7 +1644,7 @@ where load_data, sandbox, is_private, - is_visible.unwrap_or(true), + is_visible, ); self.add_pending_change(SessionHistoryChange { top_level_browsing_context_id: top_level_browsing_context_id, @@ -1737,7 +1743,7 @@ where top_level_browsing_context_id, None, None, - Some(window_size), + window_size, load_data, sandbox, is_private, @@ -3275,6 +3281,7 @@ where change.top_level_browsing_context_id, change.new_pipeline_id, new_context_info.parent_pipeline_id, + self.window_size.initial_viewport, //XXXjdm is this valid? new_context_info.is_private, new_context_info.is_visible, ); @@ -3612,44 +3619,41 @@ where } // Check the visible rectangle for this pipeline. If the constellation has received a - // size for the pipeline, then its painting should be up to date. If the constellation - // *hasn't* received a size, it could be that the layer was hidden by script before the - // compositor discovered it, so we just don't check the layer. - if let Some(size) = browsing_context.size { - // If the rectangle for this pipeline is zero sized, it will - // never be painted. In this case, don't query the layout - // thread as it won't contribute to the final output image. - if size == TypedSize2D::zero() { - continue; - } + // size for the pipeline, then its painting should be up to date. + // + // If the rectangle for this pipeline is zero sized, it will + // never be painted. In this case, don't query the layout + // thread as it won't contribute to the final output image. + if browsing_context.size == TypedSize2D::zero() { + continue; + } - // Get the epoch that the compositor has drawn for this pipeline. - let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id); - match compositor_epoch { - Some(compositor_epoch) => { - // Synchronously query the layout thread to see if the current - // epoch matches what the compositor has drawn. If they match - // (and script is idle) then this pipeline won't change again - // and can be considered stable. - let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone()); - if let Err(e) = pipeline.layout_chan.send(message) { - warn!("Failed to send GetCurrentEpoch ({}).", e); - } - match epoch_receiver.recv() { - Err(e) => warn!("Failed to receive current epoch ({}).", e), - Ok(layout_thread_epoch) => { - if layout_thread_epoch != *compositor_epoch { - return ReadyToSave::EpochMismatch; - } - }, - } - }, - None => { - // The compositor doesn't know about this pipeline yet. - // Assume it hasn't rendered yet. - return ReadyToSave::PipelineUnknown; - }, - } + // Get the epoch that the compositor has drawn for this pipeline. + let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id); + match compositor_epoch { + Some(compositor_epoch) => { + // Synchronously query the layout thread to see if the current + // epoch matches what the compositor has drawn. If they match + // (and script is idle) then this pipeline won't change again + // and can be considered stable. + let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone()); + if let Err(e) = pipeline.layout_chan.send(message) { + warn!("Failed to send GetCurrentEpoch ({}).", e); + } + match epoch_receiver.recv() { + Err(e) => warn!("Failed to receive current epoch ({}).", e), + Ok(layout_thread_epoch) => { + if layout_thread_epoch != *compositor_epoch { + return ReadyToSave::EpochMismatch; + } + }, + } + }, + None => { + // The compositor doesn't know about this pipeline yet. + // Assume it hasn't rendered yet. + return ReadyToSave::PipelineUnknown; + }, } } @@ -3715,7 +3719,7 @@ where browsing_context_id: BrowsingContextId, ) { if let Some(browsing_context) = self.browsing_contexts.get_mut(&browsing_context_id) { - browsing_context.size = Some(new_size.initial_viewport); + browsing_context.size = new_size.initial_viewport; // Send Resize (or ResizeInactive) messages to each pipeline in the frame tree. let pipeline_id = browsing_context.pipeline_id; let pipeline = match self.pipelines.get(&pipeline_id) { @@ -4001,7 +4005,6 @@ where .map(|pipeline| { let mut frame_tree = SendableFrameTree { pipeline: pipeline.to_sendable(), - size: browsing_context.size, children: vec![], }; diff --git a/components/constellation/pipeline.rs b/components/constellation/pipeline.rs index c051f8124fa..4a04adec0dd 100644 --- a/components/constellation/pipeline.rs +++ b/components/constellation/pipeline.rs @@ -152,7 +152,7 @@ pub struct InitialPipelineState { pub mem_profiler_chan: profile_mem::ProfilerChan, /// Information about the initial window size. - pub window_size: Option>, + pub window_size: TypedSize2D, /// Information about the device pixel ratio. pub device_pixel_ratio: TypedScale, @@ -200,11 +200,10 @@ impl Pipeline { let (layout_content_process_shutdown_chan, layout_content_process_shutdown_port) = ipc::channel().expect("Pipeline layout content shutdown chan"); - let device_pixel_ratio = state.device_pixel_ratio; - let window_size = state.window_size.map(|size| WindowSizeData { - initial_viewport: size, - device_pixel_ratio: device_pixel_ratio, - }); + let window_size = WindowSizeData { + initial_viewport: state.window_size, + device_pixel_ratio: state.device_pixel_ratio, + }; let url = state.load_data.url.clone(); @@ -475,7 +474,7 @@ pub struct UnprivilegedPipelineContent { resource_threads: ResourceThreads, time_profiler_chan: time::ProfilerChan, mem_profiler_chan: profile_mem::ProfilerChan, - window_size: Option, + window_size: WindowSizeData, script_chan: IpcSender, load_data: LoadData, script_port: IpcReceiver, diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 8fa7e33e784..5ee154658d7 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -2806,17 +2806,11 @@ impl Document { /// /// FIXME(emilio): This really needs to be somehow more in sync with layout. /// Feels like a hack. - /// - /// Also, shouldn't return an option, I'm quite sure. - pub fn device(&self) -> Option { - let window_size = self.window().window_size()?; + pub fn device(&self) -> Device { + let window_size = self.window().window_size(); let viewport_size = window_size.initial_viewport; let device_pixel_ratio = window_size.device_pixel_ratio; - Some(Device::new( - MediaType::screen(), - viewport_size, - device_pixel_ratio, - )) + Device::new(MediaType::screen(), viewport_size, device_pixel_ratio) } /// Remove a stylesheet owned by `owner` from the list of document sheets. @@ -4183,7 +4177,7 @@ impl DocumentMethods for Document { let y = *y as f32; let point = &Point2D::new(x, y); let window = window_from_node(self); - let viewport = window.window_size()?.initial_viewport; + let viewport = window.window_size().initial_viewport; if self.browsing_context().is_none() { return None; @@ -4218,10 +4212,7 @@ impl DocumentMethods for Document { let y = *y as f32; let point = &Point2D::new(x, y); let window = window_from_node(self); - let viewport = match window.window_size() { - Some(size) => size.initial_viewport, - None => return vec![], - }; + let viewport = window.window_size().initial_viewport; if self.browsing_context().is_none() { return vec![]; diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs index 93a14f16fee..ae87091e0a3 100644 --- a/components/script/dom/htmliframeelement.rs +++ b/components/script/dom/htmliframeelement.rs @@ -194,7 +194,7 @@ impl HTMLIFrameElement { load_data: load_data.unwrap(), pipeline_port: pipeline_receiver, content_process_shutdown_chan: None, - window_size: Some(WindowSizeData { + window_size: WindowSizeData { initial_viewport: { let rect = self.upcast::().bounding_content_box_or_zero(); TypedSize2D::new( @@ -203,7 +203,7 @@ impl HTMLIFrameElement { ) }, device_pixel_ratio: window.device_pixel_ratio(), - }), + }, layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize, }; diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs index 9abb640f105..60e27304aae 100644 --- a/components/script/dom/htmlimageelement.rs +++ b/components/script/dom/htmlimageelement.rs @@ -634,21 +634,14 @@ impl HTMLImageElement { ) -> Au { let document = document_from_node(self); let device = document.device(); - if !device.is_some() { - return Au(1); - } let quirks_mode = document.quirks_mode(); //FIXME https://github.com/whatwg/html/issues/3832 - source_size_list.evaluate(&device.unwrap(), quirks_mode) + source_size_list.evaluate(&device, quirks_mode) } /// https://html.spec.whatwg.org/multipage/#matches-the-environment fn matches_environment(&self, media_query: String) -> bool { let document = document_from_node(self); - let device = match document.device() { - Some(device) => device, - None => return false, - }; let quirks_mode = document.quirks_mode(); let document_url = &document.url(); // FIXME(emilio): This should do the same that we do for other media @@ -668,7 +661,7 @@ impl HTMLImageElement { let mut parserInput = ParserInput::new(&media_query); let mut parser = Parser::new(&mut parserInput); let media_list = MediaList::parse(&context, &mut parser); - media_list.evaluate(&device, quirks_mode) + media_list.evaluate(&document.device(), quirks_mode) } /// @@ -740,13 +733,11 @@ impl HTMLImageElement { // Step 5 let mut best_candidate = max; let device = document_from_node(self).device(); - if let Some(device) = device { - let device_den = device.device_pixel_ratio().get() as f64; - for (index, image_source) in img_sources.iter().enumerate() { - let current_den = image_source.descriptor.den.unwrap(); - if current_den < best_candidate.0 && current_den >= device_den { - best_candidate = (current_den, index); - } + let device_den = device.device_pixel_ratio().get() as f64; + for (index, image_source) in img_sources.iter().enumerate() { + let current_den = image_source.descriptor.den.unwrap(); + if current_den < best_candidate.0 && current_den >= device_den { + best_candidate = (current_den, index); } } let selected_source = img_sources.remove(best_candidate.1).clone(); diff --git a/components/script/dom/mediaquerylist.rs b/components/script/dom/mediaquerylist.rs index 3e485466c64..1730845a4df 100644 --- a/components/script/dom/mediaquerylist.rs +++ b/components/script/dom/mediaquerylist.rs @@ -69,10 +69,8 @@ impl MediaQueryList { } pub fn evaluate(&self) -> bool { - self.document.device().map_or(false, |device| { - self.media_query_list - .evaluate(&device, self.document.quirks_mode()) - }) + self.media_query_list + .evaluate(&self.document.device(), self.document.quirks_mode()) } } diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index d8353157cc1..785404454bd 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -216,7 +216,7 @@ pub struct Window { layout_rpc: Box, /// The current size of the window, in pixels. - window_size: Cell>, + window_size: Cell, /// A handle for communicating messages to the bluetooth thread. #[ignore_malloc_size_of = "channels are hard"] @@ -958,7 +958,9 @@ impl WindowMethods for Window { fn InnerHeight(&self) -> i32 { self.window_size .get() - .and_then(|e| e.initial_viewport.height.to_i32()) + .initial_viewport + .height + .to_i32() .unwrap_or(0) } @@ -967,7 +969,9 @@ impl WindowMethods for Window { fn InnerWidth(&self) -> i32 { self.window_size .get() - .and_then(|e| e.initial_viewport.width.to_i32()) + .initial_viewport + .width + .to_i32() .unwrap_or(0) } @@ -1245,10 +1249,7 @@ impl Window { let xfinite = if x_.is_finite() { x_ } else { 0.0f64 }; let yfinite = if y_.is_finite() { y_ } else { 0.0f64 }; - // Step 4 - if self.window_size.get().is_none() { - return; - } + // TODO Step 4 - determine if a window has a viewport // Step 5 //TODO remove scrollbar width @@ -1322,9 +1323,7 @@ impl Window { } pub fn device_pixel_ratio(&self) -> TypedScale { - self.window_size - .get() - .map_or(TypedScale::new(1.0), |data| data.device_pixel_ratio) + self.window_size.get().device_pixel_ratio } fn client_window(&self) -> (TypedSize2D, TypedPoint2D) { @@ -1369,12 +1368,6 @@ impl Window { _ => (), } - // If there is no window size, we have nothing to do. - let window_size = match self.window_size.get() { - Some(window_size) => window_size, - None => return false, - }; - let for_display = reflow_goal == ReflowGoal::Full; if for_display && self.suppress_reflow.get() { debug!( @@ -1417,7 +1410,7 @@ impl Window { }, document: self.Document().upcast::().to_trusted_node_address(), stylesheets_changed, - window_size, + window_size: self.window_size.get(), reflow_goal, script_join_chan: join_chan, dom_count: self.Document().dom_count(), @@ -1510,13 +1503,11 @@ impl Window { if !for_display || self.Document().needs_reflow() { issued_reflow = self.force_reflow(reflow_goal, reason); - // If window_size is `None`, we don't reflow, so the document stays - // dirty. Otherwise, we shouldn't need a reflow immediately after a + // We shouldn't need a reflow immediately after a // reflow, except if we're waiting for a deferred paint. assert!( !self.Document().needs_reflow() || (!for_display && self.Document().needs_paint()) || - self.window_size.get().is_none() || self.suppress_reflow.get() ); } else { @@ -1801,10 +1792,10 @@ impl Window { } pub fn set_window_size(&self, size: WindowSizeData) { - self.window_size.set(Some(size)); + self.window_size.set(size); } - pub fn window_size(&self) -> Option { + pub fn window_size(&self) -> WindowSizeData { self.window_size.get() } @@ -2021,7 +2012,7 @@ impl Window { layout_chan: Sender, pipelineid: PipelineId, parent_info: Option, - window_size: Option, + window_size: WindowSizeData, origin: MutableOrigin, navigation_start: u64, navigation_start_precise: u64, diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs index 80850e804b5..65362ee0d17 100644 --- a/components/script/dom/windowproxy.rs +++ b/components/script/dom/windowproxy.rs @@ -289,7 +289,7 @@ impl WindowProxy { load_data: load_data, pipeline_port: pipeline_receiver, content_process_shutdown_chan: None, - window_size: None, + window_size: window.window_size(), layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize, }; let constellation_msg = ScriptMsg::ScriptNewAuxiliary(load_info, pipeline_sender); diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index f7bc6b9b26c..134a6b47709 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -182,7 +182,7 @@ struct InProgressLoad { /// The opener, if this is an auxiliary. opener: Option, /// The current window size associated with this pipeline. - window_size: Option, + window_size: WindowSizeData, /// Channel to the layout thread associated with this pipeline. layout_chan: Sender, /// The activity level of the document (inactive, active or fully active). @@ -210,7 +210,7 @@ impl InProgressLoad { parent_info: Option, opener: Option, layout_chan: Sender, - window_size: Option, + window_size: WindowSizeData, url: ServoUrl, origin: MutableOrigin, ) -> InProgressLoad { @@ -1852,7 +1852,7 @@ impl ScriptThread { } let mut loads = self.incomplete_loads.borrow_mut(); if let Some(ref mut load) = loads.iter_mut().find(|load| load.pipeline_id == id) { - load.window_size = Some(size); + load.window_size = size; return; } warn!("resize sent to nonexistent pipeline"); diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs index 51e3417e274..2dfc2378d2f 100644 --- a/components/script_traits/lib.rs +++ b/components/script_traits/lib.rs @@ -190,7 +190,7 @@ pub struct NewLayoutInfo { /// Network request data which will be initiated by the script thread. pub load_data: LoadData, /// Information about the initial window size. - pub window_size: Option, + pub window_size: WindowSizeData, /// A port on which layout can receive messages from the pipeline. pub pipeline_port: IpcReceiver, /// A shutdown channel so that layout can tell the content process to shut down when it's done. @@ -566,7 +566,7 @@ pub struct InitialScriptState { /// A channel to the developer tools, if applicable. pub devtools_chan: Option>, /// Information about the initial window size. - pub window_size: Option, + pub window_size: WindowSizeData, /// The ID of the pipeline namespace for this script thread. pub pipeline_namespace_id: PipelineNamespaceId, /// A ping will be sent on this channel once the script thread shuts down.