Auto merge of #22395 - jdm:initial-iframe-size, r=Manishearth

Initialize iframe viewport immediately

This is probably a significant cause of unstable test results, and it finally bothered me enough to fix it. The solution isn't great for performance (a sync layout query every time a iframe is added to a document), but performance needs to follow correctness.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #14364 and fix #15689 and fix #15688.
- [x] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22395)
<!-- Reviewable:end -->

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<TypedSize2D<f32, CSSPixel>>,
     pub children: Vec<SendableFrameTree>,
 }
 

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,

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,20 @@ where
             EmbedderMsg::Panic(reason, backtrace),
         ));
 
-        let browsing_context = self.browsing_contexts.get(&browsing_context_id);
+        let browsing_context = match self.browsing_contexts.get(&browsing_context_id) {
-        let window_size = browsing_context.and_then(|ctx| ctx.size);
+            Some(context) => context,
-        let pipeline_id = browsing_context.map(|ctx| ctx.pipeline_id);
+            None => return warn!("failed browsing context is missing"),
-        let is_visible = browsing_context.map(|ctx| ctx.is_visible);
+        };
+        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 = match self.pipelines.get(&pipeline_id) {
-        let pipeline_url = pipeline.map(|pipeline| pipeline.url.clone());
+            Some(p) => p,
-        let opener = pipeline.and_then(|pipeline| pipeline.opener);
+            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 {
-            if pipeline_url == failure_url {
+            return error!("about:failure failed");
-                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
+            // size for the pipeline, then its painting should be up to date.
-            // *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 the rectangle for this pipeline is zero sized, it will
-            if let Some(size) = browsing_context.size {
+            // never be painted. In this case, don't query the layout
-                // If the rectangle for this pipeline is zero sized, it will
+            // thread as it won't contribute to the final output image.
-                // never be painted. In this case, don't query the layout
+            if browsing_context.size == TypedSize2D::zero() {
-                // thread as it won't contribute to the final output image.
+                continue;
-                if size == TypedSize2D::zero() {
+            }
-                    continue;
-                }
 
-                // Get the epoch that the compositor has drawn for this pipeline.
+            // Get the epoch that the compositor has drawn for this pipeline.
-                let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id);
+            let compositor_epoch = pipeline_states.get(&browsing_context.pipeline_id);
-                match compositor_epoch {
+            match compositor_epoch {
-                    Some(compositor_epoch) => {
+                Some(compositor_epoch) => {
-                        // Synchronously query the layout thread to see if the current
+                    // Synchronously query the layout thread to see if the current
-                        // epoch matches what the compositor has drawn. If they match
+                    // epoch matches what the compositor has drawn. If they match
-                        // (and script is idle) then this pipeline won't change again
+                    // (and script is idle) then this pipeline won't change again
-                        // and can be considered stable.
+                    // and can be considered stable.
-                        let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone());
+                    let message = LayoutControlMsg::GetCurrentEpoch(epoch_sender.clone());
-                        if let Err(e) = pipeline.layout_chan.send(message) {
+                    if let Err(e) = pipeline.layout_chan.send(message) {
-                            warn!("Failed to send GetCurrentEpoch ({}).", e);
+                        warn!("Failed to send GetCurrentEpoch ({}).", e);
-                        }
+                    }
-                        match epoch_receiver.recv() {
+                    match epoch_receiver.recv() {
-                            Err(e) => warn!("Failed to receive current epoch ({}).", e),
+                        Err(e) => warn!("Failed to receive current epoch ({}).", e),
-                            Ok(layout_thread_epoch) => {
+                        Ok(layout_thread_epoch) => {
-                                if layout_thread_epoch != *compositor_epoch {
+                            if layout_thread_epoch != *compositor_epoch {
-                                    return ReadyToSave::EpochMismatch;
+                                return ReadyToSave::EpochMismatch;
-                                }
+                            }
-                            },
+                        },
-                        }
+                    }
-                    },
+                },
-                    None => {
+                None => {
-                        // The compositor doesn't know about this pipeline yet.
+                    // The compositor doesn't know about this pipeline yet.
-                        // Assume it hasn't rendered yet.
+                    // Assume it hasn't rendered yet.
-                        return ReadyToSave::PipelineUnknown;
+                    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![],
                         };
 

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 = WindowSizeData {
-        let window_size = state.window_size.map(|size| WindowSizeData {
+            initial_viewport: state.window_size,
-            initial_viewport: size,
+            device_pixel_ratio: state.device_pixel_ratio,
-            device_pixel_ratio: 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<WindowSizeData>,
+    window_size: WindowSizeData,
     script_chan: IpcSender<ConstellationControlMsg>,
     load_data: LoadData,
     script_port: IpcReceiver<ConstellationControlMsg>,

components/gfx/platform/macos/font.rs

@@ -239,15 +239,14 @@ impl FontHandleMethods for FontHandle {
 
         let result = unsafe {
             self.ctfont
-                .get_glyphs_for_characters(&characters[0], &mut glyphs[0], count)
+                .get_glyphs_for_characters(characters.as_ptr(), glyphs.as_mut_ptr(), count)
         };
 
-        if !result {
+        if !result || glyphs[0] == 0 {
             // No glyph for this character
             return None;
         }
 
-        assert_ne!(glyphs[0], 0); // FIXME: error handling
         return Some(glyphs[0] as GlyphId);
     }
 

components/script/dom/bindings/callback.rs

@@ -4,12 +4,15 @@
 
 //! Base classes to work with IDL callbacks.
 
+use crate::dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
 use crate::dom::bindings::error::{report_pending_exception, Error, Fallible};
+use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::DomObject;
 use crate::dom::bindings::root::{Dom, DomRoot};
 use crate::dom::bindings::settings_stack::{AutoEntryScript, AutoIncumbentScript};
 use crate::dom::bindings::utils::AsCCharPtrPtr;
 use crate::dom::globalscope::GlobalScope;
+use crate::dom::window::Window;
 use js::jsapi::Heap;
 use js::jsapi::JSAutoCompartment;
 use js::jsapi::{AddRawValueRoot, IsCallable, JSContext, JSObject};
@@ -242,6 +245,9 @@ impl CallSetup {
     #[allow(unrooted_must_root)]
     pub fn new<T: CallbackContainer>(callback: &T, handling: ExceptionHandling) -> CallSetup {
         let global = unsafe { GlobalScope::from_object(callback.callback()) };
+        if let Some(window) = global.downcast::<Window>() {
+            window.Document().ensure_safe_to_run_script_or_layout();
+        }
         let cx = global.get_cx();
 
         let aes = AutoEntryScript::new(&global);

components/script/dom/bindings/trace.rs

@@ -49,6 +49,7 @@ use crate::dom::bindings::utils::WindowProxyHandler;
 use crate::dom::document::PendingRestyle;
 use crate::dom::htmlimageelement::SourceSet;
 use crate::dom::htmlmediaelement::MediaFrameRenderer;
+use crate::task::TaskBox;
 use crossbeam_channel::{Receiver, Sender};
 use cssparser::RGBA;
 use devtools_traits::{CSSError, TimelineMarkerType, WorkerId};
@@ -139,6 +140,8 @@ pub unsafe trait JSTraceable {
     unsafe fn trace(&self, trc: *mut JSTracer);
 }
 
+unsafe_no_jsmanaged_fields!(Box<dyn TaskBox>);
+
 unsafe_no_jsmanaged_fields!(CSSError);
 
 unsafe_no_jsmanaged_fields!(&'static Encoding);

components/script/dom/document.rs

@@ -101,6 +101,7 @@ use crate::dom::windowproxy::WindowProxy;
 use crate::fetch::FetchCanceller;
 use crate::script_runtime::{CommonScriptMsg, ScriptThreadEventCategory};
 use crate::script_thread::{MainThreadScriptMsg, ScriptThread};
+use crate::task::TaskBox;
 use crate::task_source::{TaskSource, TaskSourceName};
 use crate::timers::OneshotTimerCallback;
 use devtools_traits::ScriptToDevtoolsControlMsg;
@@ -410,6 +411,11 @@ pub struct Document {
     responsive_images: DomRefCell<Vec<Dom<HTMLImageElement>>>,
     /// Number of redirects for the document load
     redirect_count: Cell<u16>,
+    /// Number of outstanding requests to prevent JS or layout from running.
+    script_and_layout_blockers: Cell<u32>,
+    /// List of tasks to execute as soon as last script/layout blocker is removed.
+    #[ignore_malloc_size_of = "Measuring trait objects is hard"]
+    delayed_tasks: DomRefCell<Vec<Box<dyn TaskBox>>>,
 }
 
 #[derive(JSTraceable, MallocSizeOf)]
@@ -2695,9 +2701,51 @@ impl Document {
             fired_unload: Cell::new(false),
             responsive_images: Default::default(),
             redirect_count: Cell::new(0),
+            script_and_layout_blockers: Cell::new(0),
+            delayed_tasks: Default::default(),
         }
     }
 
+    /// Prevent any JS or layout from running until the corresponding call to
+    /// `remove_script_and_layout_blocker`. Used to isolate periods in which
+    /// the DOM is in an unstable state and should not be exposed to arbitrary
+    /// web content. Any attempts to invoke content JS or query layout during
+    /// that time will trigger a panic. `add_delayed_task` will cause the
+    /// provided task to be executed as soon as the last blocker is removed.
+    pub fn add_script_and_layout_blocker(&self) {
+        self.script_and_layout_blockers
+            .set(self.script_and_layout_blockers.get() + 1);
+    }
+
+    /// Terminate the period in which JS or layout is disallowed from running.
+    /// If no further blockers remain, any delayed tasks in the queue will
+    /// be executed in queue order until the queue is empty.
+    pub fn remove_script_and_layout_blocker(&self) {
+        assert!(self.script_and_layout_blockers.get() > 0);
+        self.script_and_layout_blockers
+            .set(self.script_and_layout_blockers.get() - 1);
+        while self.script_and_layout_blockers.get() == 0 && !self.delayed_tasks.borrow().is_empty()
+        {
+            let task = self.delayed_tasks.borrow_mut().remove(0);
+            task.run_box();
+        }
+    }
+
+    /// Enqueue a task to run as soon as any JS and layout blockers are removed.
+    pub fn add_delayed_task<T: 'static + TaskBox>(&self, task: T) {
+        self.delayed_tasks.borrow_mut().push(Box::new(task));
+    }
+
+    /// Assert that the DOM is in a state that will allow running content JS or
+    /// performing a layout operation.
+    pub fn ensure_safe_to_run_script_or_layout(&self) {
+        assert_eq!(
+            self.script_and_layout_blockers.get(),
+            0,
+            "Attempt to use script or layout while DOM not in a stable state"
+        );
+    }
+
     // https://dom.spec.whatwg.org/#dom-document-document
     pub fn Constructor(window: &Window) -> Fallible<DomRoot<Document>> {
         let doc = window.Document();
@@ -2806,17 +2854,11 @@ impl Document {
     ///
     /// FIXME(emilio): This really needs to be somehow more in sync with layout.
     /// Feels like a hack.
-    ///
+    pub fn device(&self) -> Device {
-    /// Also, shouldn't return an option, I'm quite sure.
+        let window_size = self.window().window_size();
-    pub fn device(&self) -> Option<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)
-            MediaType::screen(),
-            viewport_size,
-            device_pixel_ratio,
-        ))
     }
 
     /// Remove a stylesheet owned by `owner` from the list of document sheets.
@@ -4183,7 +4225,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 +4260,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() {
+        let viewport = window.window_size().initial_viewport;
-            Some(size) => size.initial_viewport,
-            None => return vec![],
-        };
 
         if self.browsing_context().is_none() {
             return vec![];

components/script/dom/htmliframeelement.rs

@@ -26,6 +26,7 @@ use crate::dom::windowproxy::WindowProxy;
 use crate::script_thread::ScriptThread;
 use crate::task_source::TaskSource;
 use dom_struct::dom_struct;
+use euclid::TypedSize2D;
 use html5ever::{LocalName, Prefix};
 use ipc_channel::ipc;
 use msg::constellation_msg::{BrowsingContextId, PipelineId, TopLevelBrowsingContextId};
@@ -34,6 +35,7 @@ use script_layout_interface::message::ReflowGoal;
 use script_traits::IFrameSandboxState::{IFrameSandboxed, IFrameUnsandboxed};
 use script_traits::{
     IFrameLoadInfo, IFrameLoadInfoWithData, JsEvalResult, LoadData, UpdatePipelineIdReason,
+    WindowSizeData,
 };
 use script_traits::{NewLayoutInfo, ScriptMsg};
 use servo_config::prefs::PREFS;
@@ -192,7 +194,16 @@ impl HTMLIFrameElement {
                     load_data: load_data.unwrap(),
                     pipeline_port: pipeline_receiver,
                     content_process_shutdown_chan: None,
-                    window_size: None,
+                    window_size: WindowSizeData {
+                        initial_viewport: {
+                            let rect = self.upcast::<Node>().bounding_content_box_or_zero();
+                            TypedSize2D::new(
+                                rect.size.width.to_f32_px(),
+                                rect.size.height.to_f32_px(),
+                            )
+                        },
+                        device_pixel_ratio: window.device_pixel_ratio(),
+                    },
                     layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize,
                 };
 
@@ -612,18 +623,22 @@ impl VirtualMethods for HTMLIFrameElement {
             s.bind_to_tree(tree_in_doc);
         }
 
-        // https://html.spec.whatwg.org/multipage/#the-iframe-element
+        let iframe = Trusted::new(self);
-        // "When an iframe element is inserted into a document that has
+        document_from_node(self).add_delayed_task(task!(IFrameDelayedInitialize: move || {
-        // a browsing context, the user agent must create a new
+                let this = iframe.root();
-        // browsing context, set the element's nested browsing context
+                // https://html.spec.whatwg.org/multipage/#the-iframe-element
-        // to the newly-created browsing context, and then process the
+                // "When an iframe element is inserted into a document that has
-        // iframe attributes for the "first time"."
+                // a browsing context, the user agent must create a new
-        if self.upcast::<Node>().is_in_doc_with_browsing_context() {
+                // browsing context, set the element's nested browsing context
-            debug!("iframe bound to browsing context.");
+                // to the newly-created browsing context, and then process the
-            debug_assert!(tree_in_doc, "is_in_doc_with_bc, but not tree_in_doc");
+                // iframe attributes for the "first time"."
-            self.create_nested_browsing_context();
+                if this.upcast::<Node>().is_in_doc_with_browsing_context() {
-            self.process_the_iframe_attributes(ProcessingMode::FirstTime);
+                    debug!("iframe bound to browsing context.");
-        }
+                    debug_assert!(tree_in_doc, "is_in_doc_with_bc, but not tree_in_doc");
+                    this.create_nested_browsing_context();
+                    this.process_the_iframe_attributes(ProcessingMode::FirstTime);
+                }
+            }));
     }
 
     fn unbind_from_tree(&self, context: &UnbindContext) {

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;
-            let device_den = device.device_pixel_ratio().get() as f64;
+        for (index, image_source) in img_sources.iter().enumerate() {
-            for (index, image_source) in img_sources.iter().enumerate() {
+            let current_den = image_source.descriptor.den.unwrap();
-                let current_den = image_source.descriptor.den.unwrap();
+            if current_den < best_candidate.0 && current_den >= device_den {
-                if current_den < best_candidate.0 && current_den >= device_den {
+                best_candidate = (current_den, index);
-                    best_candidate = (current_den, index);
-                }
             }
         }
         let selected_source = img_sources.remove(best_candidate.1).clone();

components/script/dom/htmlscriptelement.rs

@@ -777,7 +777,10 @@ impl VirtualMethods for HTMLScriptElement {
         }
 
         if tree_in_doc && !self.parser_inserted.get() {
-            self.prepare();
+            let script = Trusted::new(self);
+            document_from_node(self).add_delayed_task(task!(ScriptDelayedInitialize: move || {
+                    script.root().prepare();
+                }));
         }
     }
 

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
-            self.media_query_list
+            .evaluate(&self.document.device(), self.document.quirks_mode())
-                .evaluate(&device, self.document.quirks_mode())
-        })
     }
 }
 

components/script/dom/node.rs

@@ -1504,8 +1504,11 @@ impl Node {
 
     // https://dom.spec.whatwg.org/#concept-node-adopt
     pub fn adopt(node: &Node, document: &Document) {
+        document.add_script_and_layout_blocker();
+
         // Step 1.
         let old_doc = node.owner_doc();
+        old_doc.add_script_and_layout_blocker();
         // Step 2.
         node.remove_self();
         // Step 3.
@@ -1530,6 +1533,9 @@ impl Node {
                 vtable_for(&descendant).adopting_steps(&old_doc);
             }
         }
+
+        old_doc.remove_script_and_layout_blocker();
+        document.remove_script_and_layout_blocker();
     }
 
     // https://dom.spec.whatwg.org/#concept-node-ensure-pre-insertion-validity
@@ -1685,6 +1691,7 @@ impl Node {
         child: Option<&Node>,
         suppress_observers: SuppressObserver,
     ) {
+        node.owner_doc().add_script_and_layout_blocker();
         debug_assert!(&*node.owner_doc() == &*parent.owner_doc());
         debug_assert!(child.map_or(true, |child| Some(parent) == child.GetParentNode().r()));
 
@@ -1774,10 +1781,12 @@ impl Node {
             };
             MutationObserver::queue_a_mutation_record(&parent, mutation);
         }
+        node.owner_doc().remove_script_and_layout_blocker();
     }
 
     // https://dom.spec.whatwg.org/#concept-node-replace-all
     pub fn replace_all(node: Option<&Node>, parent: &Node) {
+        parent.owner_doc().add_script_and_layout_blocker();
         // Step 1.
         if let Some(node) = node {
             Node::adopt(node, &*parent.owner_doc());
@@ -1819,6 +1828,7 @@ impl Node {
             };
             MutationObserver::queue_a_mutation_record(&parent, mutation);
         }
+        parent.owner_doc().remove_script_and_layout_blocker();
     }
 
     // https://dom.spec.whatwg.org/#concept-node-pre-remove
@@ -1839,6 +1849,7 @@ impl Node {
 
     // https://dom.spec.whatwg.org/#concept-node-remove
     fn remove(node: &Node, parent: &Node, suppress_observers: SuppressObserver) {
+        parent.owner_doc().add_script_and_layout_blocker();
         assert!(
             node.GetParentNode()
                 .map_or(false, |node_parent| &*node_parent == parent)
@@ -1884,6 +1895,7 @@ impl Node {
             };
             MutationObserver::queue_a_mutation_record(&parent, mutation);
         }
+        parent.owner_doc().remove_script_and_layout_blocker();
     }
 
     // https://dom.spec.whatwg.org/#concept-node-clone

components/script/dom/window.rs

@@ -216,7 +216,7 @@ pub struct Window {
     layout_rpc: Box<LayoutRPC + Send + 'static>,
 
     /// The current size of the window, in pixels.
-    window_size: Cell<Option<WindowSizeData>>,
+    window_size: Cell<WindowSizeData>,
 
     /// 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
+        // TODO Step 4 - determine if a window has a viewport
-        if self.window_size.get().is_none() {
-            return;
-        }
 
         // Step 5
         //TODO remove scrollbar width
@@ -1322,9 +1323,7 @@ impl Window {
     }
 
     pub fn device_pixel_ratio(&self) -> TypedScale<f32, CSSPixel, DevicePixel> {
-        self.window_size
+        self.window_size.get().device_pixel_ratio
-            .get()
-            .map_or(TypedScale::new(1.0), |data| data.device_pixel_ratio)
     }
 
     fn client_window(&self) -> (TypedSize2D<u32, CSSPixel>, TypedPoint2D<i32, CSSPixel>) {
@@ -1362,6 +1361,7 @@ impl Window {
     /// Returns true if layout actually happened, false otherwise.
     #[allow(unsafe_code)]
     pub fn force_reflow(&self, reflow_goal: ReflowGoal, reason: ReflowReason) -> bool {
+        self.Document().ensure_safe_to_run_script_or_layout();
         // Check if we need to unsuppress reflow. Note that this needs to be
         // *before* any early bailouts, or reflow might never be unsuppresed!
         match reason {
@@ -1369,12 +1369,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 +1411,7 @@ impl Window {
             },
             document: self.Document().upcast::<Node>().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(),
@@ -1504,19 +1498,18 @@ impl Window {
     /// may happen in the only case a query reflow may bail out, that is, if the
     /// viewport size is not present). See #11223 for an example of that.
     pub fn reflow(&self, reflow_goal: ReflowGoal, reason: ReflowReason) -> bool {
+        self.Document().ensure_safe_to_run_script_or_layout();
         let for_display = reflow_goal == ReflowGoal::Full;
 
         let mut issued_reflow = false;
         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
+            // We shouldn't need a reflow immediately after a
-            // dirty. Otherwise, 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 +1794,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<WindowSizeData> {
+    pub fn window_size(&self) -> WindowSizeData {
         self.window_size.get()
     }
 
@@ -2021,7 +2014,7 @@ impl Window {
         layout_chan: Sender<Msg>,
         pipelineid: PipelineId,
         parent_info: Option<PipelineId>,
-        window_size: Option<WindowSizeData>,
+        window_size: WindowSizeData,
         origin: MutableOrigin,
         navigation_start: u64,
         navigation_start_precise: u64,

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);

components/script/script_thread.rs

@@ -182,7 +182,7 @@ struct InProgressLoad {
     /// The opener, if this is an auxiliary.
     opener: Option<BrowsingContextId>,
     /// The current window size associated with this pipeline.
-    window_size: Option<WindowSizeData>,
+    window_size: WindowSizeData,
     /// Channel to the layout thread associated with this pipeline.
     layout_chan: Sender<message::Msg>,
     /// The activity level of the document (inactive, active or fully active).
@@ -210,7 +210,7 @@ impl InProgressLoad {
         parent_info: Option<PipelineId>,
         opener: Option<BrowsingContextId>,
         layout_chan: Sender<message::Msg>,
-        window_size: Option<WindowSizeData>,
+        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");

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<WindowSizeData>,
+    pub window_size: WindowSizeData,
     /// A port on which layout can receive messages from the pipeline.
     pub pipeline_port: IpcReceiver<LayoutControlMsg>,
     /// 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<IpcSender<ScriptToDevtoolsControlMsg>>,
     /// Information about the initial window size.
-    pub window_size: Option<WindowSizeData>,
+    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.

tests/wpt/metadata/css/cssom-view/matchMedia.xht.ini

@@ -4,18 +4,9 @@
   [window.matchMedia exists]
     expected: FAIL
 
-  [MediaQueryList.matches for "(max-width: 199px), all and (min-width: 200px)"]
-    expected: FAIL
-
   [MediaQueryList.matches for "(min-aspect-ratio: 1/1)"]
     expected: FAIL
 
-  [MediaQueryList.matches for "(width: 200px)"]
-    expected: FAIL
-
-  [MediaQueryList.matches for "(min-width: 150px)"]
-    expected: FAIL
-
   [Resize iframe from 200x100 to 200x50, then to 100x50]
     expected: NOTRUN
 

tests/wpt/metadata/css/cssom/getComputedStyle-detached-subtree.html.ini

@@ -1,2 +1,8 @@
 [getComputedStyle-detached-subtree.html]
   expected: ERROR
+  [getComputedStyle returns no style for element in non-rendered iframe (display: none) from iframe's window]
+    expected: FAIL
+
+  [getComputedStyle returns no style for element in non-rendered iframe (display: none)]
+    expected: FAIL
+