Make iframes block the enclosing document's load event. Fixes #6663.

components/compositing/constellation.rs

@@ -832,7 +832,7 @@ impl<LTF: LayoutThreadFactory, STF: ScriptThreadFactory> Constellation<LTF, STF>
         let parent_pipeline = self.pipeline(subframe_parent.0);
         let msg = ConstellationControlMsg::DispatchFrameLoadEvent {
             target: pipeline_id,
-            parent: subframe_parent.0
+            parent: subframe_parent.0,
         };
         parent_pipeline.script_chan.send(msg).unwrap();
     }

components/script/document_loader.rs

@@ -5,10 +5,13 @@
 //! Tracking of pending loads in a document.
 //! https://html.spec.whatwg.org/multipage/#the-end
 
+use dom::bindings::js::JS;
+use dom::document::Document;
 use msg::constellation_msg::PipelineId;
 use net_traits::AsyncResponseTarget;
 use net_traits::{PendingAsyncLoad, ResourceThread, LoadContext};
 use std::sync::Arc;
+use std::thread;
 use url::Url;
 
 #[derive(JSTraceable, PartialEq, Clone, Debug, HeapSizeOf)]
@@ -41,6 +44,50 @@ impl LoadType {
     }
 }
 
+/// Canary value ensuring that manually added blocking loads (ie. ones that weren't
+/// created via DocumentLoader::prepare_async_load) are always removed by the time
+/// that the owner is destroyed.
+#[derive(JSTraceable, HeapSizeOf)]
+#[must_root]
+pub struct LoadBlocker {
+    /// The document whose load event is blocked by this object existing.
+    doc: JS<Document>,
+    /// The load that is blocking the document's load event.
+    load: Option<LoadType>,
+}
+
+impl LoadBlocker {
+    /// Mark the document's load event as blocked on this new load.
+    pub fn new(doc: &Document, load: LoadType) -> LoadBlocker {
+        doc.add_blocking_load(load.clone());
+        LoadBlocker {
+            doc: JS::from_ref(doc),
+            load: Some(load),
+        }
+    }
+
+    /// Remove this load from the associated document's list of blocking loads.
+    pub fn terminate(blocker: &mut Option<LoadBlocker>) {
+        if let Some(this) = blocker.as_mut() {
+            this.doc.finish_load(this.load.take().unwrap());
+        }
+        *blocker = None;
+    }
+
+    /// Return the url associated with this load.
+    pub fn url(&self) -> Option<&Url> {
+        self.load.as_ref().map(LoadType::url)
+    }
+}
+
+impl Drop for LoadBlocker {
+    fn drop(&mut self) {
+        if !thread::panicking() {
+            assert!(self.load.is_none());
+        }
+    }
+}
+
 #[derive(JSTraceable, HeapSizeOf)]
 pub struct DocumentLoader {
     /// We use an `Arc<ResourceThread>` here in order to avoid file descriptor exhaustion when there
@@ -73,12 +120,16 @@ impl DocumentLoader {
         }
     }
 
+    pub fn add_blocking_load(&mut self, load: LoadType) {
+        self.blocking_loads.push(load);
+    }
+
     /// Create a new pending network request, which can be initiated at some point in
     /// the future.
     pub fn prepare_async_load(&mut self, load: LoadType) -> PendingAsyncLoad {
         let context = load.to_load_context();
         let url = load.url().clone();
-        self.blocking_loads.push(load);
+        self.add_blocking_load(load);
         PendingAsyncLoad::new(context, (*self.resource_thread).clone(), url, self.pipeline)
     }
 

components/script/dom/document.rs

@@ -1231,6 +1231,11 @@ impl Document {
                            ReflowReason::RequestAnimationFrame);
     }
 
+    pub fn add_blocking_load(&self, load: LoadType) {
+        let mut loader = self.loader.borrow_mut();
+        loader.add_blocking_load(load)
+    }
+
     pub fn prepare_async_load(&self, load: LoadType) -> PendingAsyncLoad {
         let mut loader = self.loader.borrow_mut();
         loader.prepare_async_load(load)

components/script/dom/htmliframeelement.rs

@@ -2,7 +2,9 @@
  * 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 document_loader::{LoadType, LoadBlocker};
 use dom::attr::{Attr, AttrValue};
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::BrowserElementBinding::BrowserElementIconChangeEventDetail;
 use dom::bindings::codegen::Bindings::BrowserElementBinding::BrowserElementSecurityChangeDetail;
 use dom::bindings::codegen::Bindings::BrowserElementBinding::BrowserShowModalPromptEventDetail;
@@ -21,7 +23,7 @@ use dom::element::{AttributeMutation, Element, RawLayoutElementHelpers};
 use dom::event::Event;
 use dom::eventtarget::EventTarget;
 use dom::htmlelement::HTMLElement;
-use dom::node::{Node, UnbindContext, window_from_node};
+use dom::node::{Node, UnbindContext, window_from_node, document_from_node};
 use dom::urlhelper::UrlHelper;
 use dom::virtualmethods::VirtualMethods;
 use dom::window::{ReflowReason, Window};
@@ -64,6 +66,7 @@ pub struct HTMLIFrameElement {
     subpage_id: Cell<Option<SubpageId>>,
     containing_page_pipeline_id: Cell<Option<PipelineId>>,
     sandbox: Cell<Option<u8>>,
+    load_blocker: DOMRefCell<Option<LoadBlocker>>,
 }
 
 impl HTMLIFrameElement {
@@ -101,6 +104,20 @@ impl HTMLIFrameElement {
             IFrameUnsandboxed
         };
 
+        let document = document_from_node(self);
+
+        let mut load_blocker = self.load_blocker.borrow_mut();
+        // Any oustanding load is finished from the point of view of the blocked
+        // document; the new navigation will continue blocking it.
+        LoadBlocker::terminate(&mut load_blocker);
+
+        //TODO(#9592): Deal with the case where an iframe is being reloaded so url is None.
+        //      The iframe should always have access to the nested context's active
+        //      document URL through the browsing context.
+        if let Some(ref url) = url {
+            *load_blocker = Some(LoadBlocker::new(&*document, LoadType::Subframe(url.clone())));
+        }
+
         let window = window_from_node(self);
         let window = window.r();
         let (new_subpage_id, old_subpage_id) = self.generate_new_subpage_id();
@@ -173,6 +190,7 @@ impl HTMLIFrameElement {
             subpage_id: Cell::new(None),
             containing_page_pipeline_id: Cell::new(None),
             sandbox: Cell::new(None),
+            load_blocker: DOMRefCell::new(None),
         }
     }
 
@@ -204,7 +222,11 @@ impl HTMLIFrameElement {
     }
 
     /// https://html.spec.whatwg.org/multipage/#iframe-load-event-steps steps 1-4
-    pub fn iframe_load_event_steps(&self) {
+    pub fn iframe_load_event_steps(&self, loaded_pipeline: PipelineId) {
+        // TODO(#9592): assert that the load blocker is present at all times when we
+        //              can guarantee that it's created for the case of iframe.reload().
+        assert_eq!(loaded_pipeline, self.pipeline().unwrap());
+
         // TODO A cross-origin child document would not be easily accessible
         //      from this script thread. It's unclear how to implement
         //      steps 2, 3, and 5 efficiently in this case.
@@ -213,6 +235,10 @@ impl HTMLIFrameElement {
 
         // Step 4
         self.upcast::<EventTarget>().fire_simple_event("load");
+
+        let mut blocker = self.load_blocker.borrow_mut();
+        LoadBlocker::terminate(&mut blocker);
+
         // TODO Step 5 - unset child document `mut iframe load` flag
 
         let window = window_from_node(self);
@@ -510,6 +536,9 @@ impl VirtualMethods for HTMLIFrameElement {
     fn unbind_from_tree(&self, context: &UnbindContext) {
         self.super_type().unwrap().unbind_from_tree(context);
 
+        let mut blocker = self.load_blocker.borrow_mut();
+        LoadBlocker::terminate(&mut blocker);
+
         // https://html.spec.whatwg.org/multipage/#a-browsing-context-is-discarded
         if let Some(pipeline_id) = self.pipeline_id.get() {
             let window = window_from_node(self);

components/script/script_thread.rs

@@ -1662,7 +1662,7 @@ impl ScriptThread {
         let page = get_page(&self.root_page(), containing_pipeline);
         let document = page.document();
         if let Some(iframe) = document.find_iframe_by_pipeline(id) {
-            iframe.iframe_load_event_steps();
+            iframe.iframe_load_event_steps(id);
         }
     }
 

components/script_traits/lib.rs

@@ -142,7 +142,7 @@ pub enum ConstellationControlMsg {
         /// The pipeline that has been marked as loaded.
         target: PipelineId,
         /// The pipeline that contains a frame loading the target pipeline.
-        parent: PipelineId
+        parent: PipelineId,
     },
     /// Notifies a parent frame that one of its child frames is now active.
     FramedContentChanged(PipelineId, SubpageId),

tests/wpt/metadata/XMLHttpRequest/send-entity-body-document.htm.ini

@@ -1,4 +1,23 @@
 [send-entity-body-document.htm]
   type: testharness
-  disabled: https://github.com/servo/servo/issues/8157
+  [XML document, windows-1252]
+    expected: FAIL
+
+  [HTML document, invalid UTF-8]
+    expected: FAIL
+
+  [HTML document, shift-jis]
+    expected: FAIL
+
+  [plain text file]
+    expected: FAIL
+
+  [image file]
+    expected: FAIL
+
+  [img tag]
+    expected: FAIL
+
+  [empty div]
+    expected: FAIL
 

tests/wpt/metadata/dom/ranges/Range-deleteContents.html.ini

@@ -35,4 +35,3 @@
 
   [Resulting cursor position for range 51 [paras[3\], 1, comment, 8\]]
     expected: FAIL
-

tests/wpt/metadata/dom/ranges/Range-extractContents.html.ini

@@ -14,4 +14,3 @@
 
   [Resulting cursor position for range 51 [paras[3\], 1, comment, 8\]]
     expected: FAIL
-

tests/wpt/metadata/html/browsers/history/the-location-interface/location_reload.html.ini

@@ -0,0 +1,3 @@
+[location_reload.html]
+  type: testharness
+  expected: CRASH

tests/wpt/metadata/html/infrastructure/terminology/plugins/text-plain.html.ini

@@ -1,3 +0,0 @@
-[text-plain.html]
-  type: testharness
-  disabled: https://github.com/servo/servo/issues/9286

tests/wpt/metadata/html/semantics/document-metadata/the-base-element/base_multiple.html.ini

@@ -1,6 +1,6 @@
 [base_multiple.html]
   type: testharness
-  expected: ERROR
+  expected: TIMEOUT
   [The attributes of the a element must be affected by the first base element]
-    expected: FAIL
+    expected: NOTRUN
 

tests/wpt/metadata/html/semantics/embedded-content/the-iframe-element/move_iframe_in_dom_03.html.ini

@@ -1,3 +0,0 @@
-[move_iframe_in_dom_03.html]
-  type: testharness
-  expected: TIMEOUT

tests/wpt/metadata/html/semantics/embedded-content/the-img-element/environment-changes/viewport-change.html.ini

@@ -1,3 +1,109 @@
 [viewport-change.html]
   type: testharness
-  expected: TIMEOUT
+  disabled: unsupported feature and prone to intermittent incorrect results
+  bug: 9560
+  [img (no src), onload, narrow]
+    expected: FAIL
+
+  [img (empty src), onload, narrow]
+    expected: FAIL
+
+  [img (src only) broken image, onload, narrow]
+    expected: FAIL
+
+  [img (src only) valid image, onload, narrow]
+    expected: FAIL
+
+  [img (srcset 1 cand) broken image, onload, narrow]
+    expected: FAIL
+
+  [img (srcset 1 cand) valid image, onload, narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img broken image, onload, narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img broken image, resize to wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img valid image, onload, narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img valid image, resize to wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img broken image, onload, narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img broken image, resize to wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img valid image, onload, narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img valid image, resize to wide]
+    expected: FAIL
+
+  [picture: same URL in source (max-width:500px) and img, onload, narrow]
+    expected: FAIL
+
+  [img (no src), onload, wide]
+    expected: FAIL
+
+  [img (empty src), onload, wide]
+    expected: FAIL
+
+  [img (src only) broken image, onload, wide]
+    expected: FAIL
+
+  [img (src only) valid image, onload, wide]
+    expected: FAIL
+
+  [img (srcset 1 cand) broken image, onload, wide]
+    expected: FAIL
+
+  [img (srcset 1 cand) valid image, onload, wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img broken image, onload, wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img broken image, resize to narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img valid image, onload, wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) broken image, img valid image, resize to narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img broken image, onload, wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img broken image, resize to narrow]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img valid image, onload, wide]
+    expected: FAIL
+
+  [picture: source (max-width:500px) valid image, img valid image, resize to narrow]
+    expected: FAIL
+
+  [picture: same URL in source (max-width:500px) and img, onload, wide]
+    expected: FAIL
+
+  [img (empty src), resize to wide]
+    expected: FAIL
+
+  [img (src only) broken image, resize to wide]
+    expected: FAIL
+
+  [img (src only) valid image, resize to wide]
+    expected: FAIL
+
+  [picture: same URL in source (max-width:500px) and img, resize to wide]
+    expected: FAIL
+
+  [picture: same URL in source (max-width:500px) and img, resize to narrow]
+    expected: FAIL
+

tests/wpt/metadata/html/semantics/selectors/pseudo-classes/focus.html.ini

@@ -6,3 +6,6 @@
   [editable elements are focusable]
     expected: FAIL
 
+  [':focus' doesn't match focused elements in iframe]
+    expected: FAIL
+

tests/wpt/metadata/workers/nested_worker.worker.js.ini

@@ -3,4 +3,6 @@
   expected: TIMEOUT
   [Nested worker]
     expected: FAIL
+  [Checking contents for text file]
+    expected: FAIL