Initial window sizes are mandatory.

jdm · jdm · commit af4cc5c05574 · 2018-12-09T14:16:14.000-05:00
diff --git a/components/compositing/lib.rs b/components/compositing/lib.rs
@@ -27,7 +27,6 @@ pub mod windowing;
 
 pub struct SendableFrameTree {
     pub pipeline: CompositionPipeline,
-    pub size: Option<TypedSize2D<f32, CSSPixel>>,
     pub children: Vec<SendableFrameTree>,
 }
 
diff --git 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<TypedSize2D<f32, CSSPixel>>,
+    pub size: TypedSize2D<f32, CSSPixel>,
 
     /// 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<PipelineId>,
+        size: TypedSize2D<f32, CSSPixel>,
         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
@@ -734,7 +734,7 @@ where
         top_level_browsing_context_id: TopLevelBrowsingContextId,
         parent_pipeline_id: Option<PipelineId>,
         opener: Option<BrowsingContextId>,
-        initial_window_size: Option<TypedSize2D<f32, CSSPixel>>,
+        initial_window_size: TypedSize2D<f32, CSSPixel>,
         // 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<PipelineId>,
+        size: TypedSize2D<f32, CSSPixel>,
         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,22 @@ 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);
+        // The none branch should be unreachable.
+        let browsing_context = match self.browsing_contexts.get(&browsing_context_id) {
+            Some(context) => context,
+            None => return,
+        };
+        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);
+        // the none branch should be unreachable.
+        let pipeline = match self.pipelines.get(&pipeline_id) {
+            Some(p) => p,
+            None => return,
+        };
+        let pipeline_url = pipeline.url.clone();
+        let opener = pipeline.opener;
 
         self.close_browsing_context_children(
             browsing_context_id,
@@ -1616,10 +1626,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 +1646,7 @@ where
             load_data.clone(),
             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 +1745,7 @@ where
             top_level_browsing_context_id,
             None,
             None,
-            Some(window_size),
+            window_size,
             load_data.clone(),
             sandbox,
             is_private,
@@ -3275,6 +3283,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 +3621,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 +3721,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 +4007,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
@@ -152,7 +152,7 @@ pub struct InitialPipelineState {
     pub mem_profiler_chan: profile_mem::ProfilerChan,
 
     /// Information about the initial window size.
-    pub window_size: Option<TypedSize2D<f32, CSSPixel>>,
+    pub window_size: TypedSize2D<f32, CSSPixel>,
 
     /// Information about the device pixel ratio.
     pub device_pixel_ratio: TypedScale<f32, CSSPixel, DevicePixel>,
@@ -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();
 
@@ -477,7 +476,7 @@ pub struct UnprivilegedPipelineContent {
     resource_threads: ResourceThreads,
     time_profiler_chan: time::ProfilerChan,
     mem_profiler_chan: profile_mem::ProfilerChan,
-    window_size: Option<WindowSizeData>,
+    window_size: WindowSizeData,
     script_chan: IpcSender<ConstellationControlMsg>,
     load_data: LoadData,
     script_port: IpcReceiver<ConstellationControlMsg>,
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
@@ -2800,17 +2800,15 @@ 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<Device> {
-        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(
+        Device::new(
             MediaType::screen(),
             viewport_size,
             device_pixel_ratio,
-        ))
+        )
     }
 
     /// Remove a stylesheet owned by `owner` from the list of document sheets.
@@ -4177,7 +4175,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;
@@ -4212,10 +4210,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
@@ -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::<Node>().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
@@ -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)
     }
 
     /// <https://html.spec.whatwg.org/multipage/#normalise-the-source-densities>
@@ -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
@@ -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
diff --git a/components/script/dom/windowproxy.rs b/components/script/dom/windowproxy.rs
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
diff --git a/components/script_traits/lib.rs b/components/script_traits/lib.rs

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,6 @@ pub mod windowing;`
`27`	`27`
`28`	`28`	`pub struct SendableFrameTree {`
`29`	`29`	`pub pipeline: CompositionPipeline,`
`30`		`- pub size: Option<TypedSize2D<f32, CSSPixel>>,`
`31`	`30`	`pub children: Vec<SendableFrameTree>,`
`32`	`31`	`}`
`33`	`32`
Original file line number	Diff line number	Diff line change
`@@ -69,10 +69,8 @@ impl MediaQueryList {`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`pub fn evaluate(&self) -> bool {`
`72`		`- self.document.device().map_or(false, \|device\| {`
`73`		`- self.media_query_list`
`74`		`- .evaluate(&device, self.document.quirks_mode())`
`75`		`- })`
	`72`	`+ self.media_query_list`
	`73`	`+ .evaluate(&self.document.device(), self.document.quirks_mode())`
`76`	`74`	`}`
`77`	`75`	`}`
`78`	`76`