Auto merge of #22121 - gterzian:remove_constellation_block_in_navigation, r=paulrouget

Remove sync constellation -> embedder communication

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #22042 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/22121)
<!-- Reviewable:end -->

components/compositing/windowing.rs

@@ -9,7 +9,7 @@ use euclid::TypedScale;
 #[cfg(feature = "gl")]
 use gleam::gl;
 use keyboard_types::KeyboardEvent;
-use msg::constellation_msg::{TopLevelBrowsingContextId, TraversalDirection};
+use msg::constellation_msg::{PipelineId, TopLevelBrowsingContextId, TraversalDirection};
 use script_traits::{MouseButton, TouchEventType, TouchId};
 use servo_geometry::DeviceIndependentPixel;
 use servo_url::ServoUrl;
@@ -49,6 +49,8 @@ pub enum WindowEvent {
     Refresh,
     /// Sent when the window is resized.
     Resize,
+    /// Sent when a navigation request from script is allowed/refused.
+    AllowNavigationResponse(PipelineId, bool),
     /// Sent when a new URL is to be loaded.
     LoadUrl(TopLevelBrowsingContextId, ServoUrl),
     /// Sent when a mouse hit test is to be performed.
@@ -96,6 +98,7 @@ impl Debug for WindowEvent {
             WindowEvent::Refresh => write!(f, "Refresh"),
             WindowEvent::Resize => write!(f, "Resize"),
             WindowEvent::Keyboard(..) => write!(f, "Keyboard"),
+            WindowEvent::AllowNavigationResponse(..) => write!(f, "AllowNavigationResponse"),
             WindowEvent::LoadUrl(..) => write!(f, "LoadUrl"),
             WindowEvent::MouseWindowEventClass(..) => write!(f, "Mouse"),
             WindowEvent::MouseWindowMoveEventClass(..) => write!(f, "MouseMove"),

components/constellation/constellation.rs

@@ -156,6 +156,7 @@ use servo_rand::{random, Rng, SeedableRng, ServoRng};
 use servo_remutex::ReentrantMutex;
 use servo_url::{Host, ImmutableOrigin, ServoUrl};
 use std::borrow::ToOwned;
+use std::collections::hash_map::Entry;
 use std::collections::{HashMap, VecDeque};
 use std::marker::PhantomData;
 use std::mem::replace;
@@ -168,6 +169,8 @@ use style_traits::viewport::ViewportConstraints;
 use style_traits::CSSPixel;
 use webvr_traits::{WebVREvent, WebVRMsg};
 
+type PendingApprovalNavigations = HashMap<PipelineId, (LoadData, bool)>;
+
 /// Servo supports tabs (referred to as browsers), so `Constellation` needs to
 /// store browser specific data for bookkeeping.
 struct Browser {
@@ -366,6 +369,9 @@ pub struct Constellation<Message, LTF, STF> {
 
     /// A channel through which messages can be sent to the canvas paint thread.
     canvas_chan: IpcSender<CanvasMsg>,
+
+    /// Navigation requests from script awaiting approval from the embedder.
+    pending_approval_navigations: PendingApprovalNavigations,
 }
 
 /// State needed to construct a constellation.
@@ -698,6 +704,7 @@ where
                     webgl_threads: state.webgl_threads,
                     webvr_chan: state.webvr_chan,
                     canvas_chan: CanvasPaintThread::start(),
+                    pending_approval_navigations: HashMap::new(),
                 };
 
                 constellation.run();
@@ -1044,6 +1051,62 @@ where
             FromCompositorMsg::Keyboard(key_event) => {
                 self.handle_key_msg(key_event);
             },
+            // Perform a navigation previously requested by script, if approved by the embedder.
+            // If there is already a pending page (self.pending_changes), it will not be overridden;
+            // However, if the id is not encompassed by another change, it will be.
+            FromCompositorMsg::AllowNavigationResponse(pipeline_id, allowed) => {
+                let pending = self.pending_approval_navigations.remove(&pipeline_id);
+
+                let top_level_browsing_context_id = match self.pipelines.get(&pipeline_id) {
+                    Some(pipeline) => pipeline.top_level_browsing_context_id,
+                    None => return warn!("Attempted to navigate {} after closure.", pipeline_id),
+                };
+
+                match pending {
+                    Some((load_data, replace)) => {
+                        if allowed {
+                            self.load_url(
+                                top_level_browsing_context_id,
+                                pipeline_id,
+                                load_data,
+                                replace,
+                            );
+                        } else {
+                            let pipeline_is_top_level_pipeline = self
+                                .browsing_contexts
+                                .get(&BrowsingContextId::from(top_level_browsing_context_id))
+                                .map(|ctx| ctx.pipeline_id == pipeline_id)
+                                .unwrap_or(false);
+                            // If the navigation is refused, and this concerns an iframe,
+                            // we need to take it out of it's "delaying-load-events-mode".
+                            // https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+                            if !pipeline_is_top_level_pipeline {
+                                let msg = ConstellationControlMsg::StopDelayingLoadEventsMode(
+                                    pipeline_id,
+                                );
+                                let result = match self.pipelines.get(&pipeline_id) {
+                                    Some(pipeline) => pipeline.event_loop.send(msg),
+                                    None => {
+                                        return warn!(
+                                            "Attempted to navigate {} after closure.",
+                                            pipeline_id
+                                        )
+                                    },
+                                };
+                                if let Err(e) = result {
+                                    self.handle_send_error(pipeline_id, e);
+                                }
+                            }
+                        }
+                    },
+                    None => {
+                        return warn!(
+                            "AllowNavigationReqsponse for unknow request: {:?}",
+                            pipeline_id
+                        )
+                    },
+                };
+            },
             // Load a new page from a typed url
             // If there is already a pending page (self.pending_changes), it will not be overridden;
             // However, if the id is not encompassed by another change, it will be.
@@ -1059,12 +1122,9 @@ where
                         )
                     },
                 };
-                self.handle_load_url_msg(
-                    top_level_browsing_context_id,
-                    pipeline_id,
-                    load_data,
-                    false,
-                );
+                // Since this is a top-level load, initiated by the embedder, go straight to load_url,
+                // bypassing schedule_navigation.
+                self.load_url(top_level_browsing_context_id, pipeline_id, load_data, false);
             },
             FromCompositorMsg::IsReadyToSaveImage(pipeline_states) => {
                 let is_ready = self.handle_is_ready_to_save_image(pipeline_states);
@@ -1176,11 +1236,9 @@ where
             FromScriptMsg::ChangeRunningAnimationsState(animation_state) => {
                 self.handle_change_running_animations_state(source_pipeline_id, animation_state)
             },
-            // Load a new page from a mouse click
-            // If there is already a pending page (self.pending_changes), it will not be overridden;
-            // However, if the id is not encompassed by another change, it will be.
+            // Ask the embedder for permission to load a new page.
             FromScriptMsg::LoadUrl(load_data, replace) => {
-                self.handle_load_url_msg(source_top_ctx_id, source_pipeline_id, load_data, replace);
+                self.schedule_navigation(source_top_ctx_id, source_pipeline_id, load_data, replace);
             },
             FromScriptMsg::AbortLoadUrl => {
                 self.handle_abort_load_url_msg(source_pipeline_id);
@@ -1806,6 +1864,9 @@ where
             Some(parent_pipeline_id) => parent_pipeline_id,
             None => return warn!("Subframe {} has no parent.", pipeline_id),
         };
+        // https://html.spec.whatwg.org/multipage/#the-iframe-element:completely-loaded
+        // When a Document in an iframe is marked as completely loaded,
+        // the user agent must run the iframe load event steps.
         let msg = ConstellationControlMsg::DispatchIFrameLoadEvent {
             target: browsing_context_id,
             parent: parent_pipeline_id,
@@ -2115,14 +2176,33 @@ where
         }
     }
 
-    fn handle_load_url_msg(
+    /// Schedule a navigation(via load_url).
+    /// 1: Ask the embedder for permission.
+    /// 2: Store the details of the navigation, pending approval from the embedder.
+    fn schedule_navigation(
         &mut self,
         top_level_browsing_context_id: TopLevelBrowsingContextId,
         source_id: PipelineId,
         load_data: LoadData,
         replace: bool,
     ) {
-        self.load_url(top_level_browsing_context_id, source_id, load_data, replace);
+        match self.pending_approval_navigations.entry(source_id) {
+            Entry::Occupied(_) => {
+                return warn!(
+                    "Pipeline {:?} tried to schedule a navigation while one is already pending.",
+                    source_id
+                )
+            },
+            Entry::Vacant(entry) => {
+                let _ = entry.insert((load_data.clone(), replace));
+            },
+        };
+        // Allow the embedder to handle the url itself
+        let msg = (
+            Some(top_level_browsing_context_id),
+            EmbedderMsg::AllowNavigationRequest(source_id, load_data.url.clone()),
+        );
+        self.embedder_proxy.send(msg);
     }
 
     fn load_url(
@@ -2132,17 +2212,6 @@ where
         load_data: LoadData,
         replace: bool,
     ) -> Option<PipelineId> {
-        // Allow the embedder to handle the url itself
-        let (chan, port) = ipc::channel().expect("Failed to create IPC channel!");
-        let msg = (
-            Some(top_level_browsing_context_id),
-            EmbedderMsg::AllowNavigation(load_data.url.clone(), chan),
-        );
-        self.embedder_proxy.send(msg);
-        if let Ok(false) = port.recv() {
-            return None;
-        }
-
         debug!("Loading {} in pipeline {}.", load_data.url, source_id);
         // If this load targets an iframe, its framing element may exist
         // in a separate script thread than the framed document that initiated

components/embedder_traits/lib.rs

@@ -14,7 +14,7 @@ pub mod resources;
 use crossbeam_channel::{Receiver, Sender};
 use ipc_channel::ipc::IpcSender;
 use keyboard_types::KeyboardEvent;
-use msg::constellation_msg::{InputMethodType, TopLevelBrowsingContextId};
+use msg::constellation_msg::{InputMethodType, PipelineId, TopLevelBrowsingContextId};
 use servo_url::ServoUrl;
 use std::fmt::{Debug, Error, Formatter};
 use style_traits::cursor::CursorKind;
@@ -79,8 +79,8 @@ pub enum EmbedderMsg {
     ResizeTo(DeviceIntSize),
     // Show an alert message.
     Alert(String, IpcSender<()>),
-    /// Wether or not to follow a link
-    AllowNavigation(ServoUrl, IpcSender<bool>),
+    /// Wether or not to allow a pipeline to load a url.
+    AllowNavigationRequest(PipelineId, ServoUrl),
     /// Whether or not to allow script to open a new tab/browser
     AllowOpeningBrowser(IpcSender<bool>),
     /// A new browser was created by script
@@ -128,7 +128,7 @@ impl Debug for EmbedderMsg {
             EmbedderMsg::ResizeTo(..) => write!(f, "ResizeTo"),
             EmbedderMsg::Alert(..) => write!(f, "Alert"),
             EmbedderMsg::AllowUnload(..) => write!(f, "AllowUnload"),
-            EmbedderMsg::AllowNavigation(..) => write!(f, "AllowNavigation"),
+            EmbedderMsg::AllowNavigationRequest(..) => write!(f, "AllowNavigationRequest"),
             EmbedderMsg::Keyboard(..) => write!(f, "Keyboard"),
             EmbedderMsg::SetCursor(..) => write!(f, "SetCursor"),
             EmbedderMsg::NewFavicon(..) => write!(f, "NewFavicon"),

components/script/dom/document.rs

@@ -416,6 +416,8 @@ pub struct Document {
     /// 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>>>,
+    /// https://html.spec.whatwg.org/multipage/#completely-loaded
+    completely_loaded: Cell<bool>,
 }
 
 #[derive(JSTraceable, MallocSizeOf)]
@@ -507,6 +509,10 @@ impl Document {
         self.https_state.set(https_state);
     }
 
+    pub fn is_completely_loaded(&self) -> bool {
+        self.completely_loaded.get()
+    }
+
     pub fn is_fully_active(&self) -> bool {
         self.activity.get() == DocumentActivity::FullyActive
     }
@@ -1735,7 +1741,11 @@ impl Document {
                 self.process_deferred_scripts();
             },
             LoadType::PageSource(_) => {
-                if self.has_browsing_context {
+                if self.has_browsing_context && self.is_fully_active() {
+                    // Note: if the document is not fully active, the layout thread will have exited already.
+                    // The underlying problem might actually be that layout exits while it should be kept alive.
+                    // See https://github.com/servo/servo/issues/22507
+
                     // Disarm the reflow timer and trigger the initial reflow.
                     self.reflow_timeout.set(None);
                     self.upcast::<Node>().dirty(NodeDamage::OtherNodeDamage);
@@ -1899,7 +1909,21 @@ impl Document {
 
     // https://html.spec.whatwg.org/multipage/#the-end
     pub fn maybe_queue_document_completion(&self) {
-        if self.loader.borrow().is_blocked() {
+        // https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+        let is_in_delaying_load_events_mode = match self.window.undiscarded_window_proxy() {
+            Some(window_proxy) => window_proxy.is_delaying_load_events_mode(),
+            None => false,
+        };
+
+        // Note: if the document is not fully active, the layout thread will have exited already,
+        // and this method will panic.
+        // The underlying problem might actually be that layout exits while it should be kept alive.
+        // See https://github.com/servo/servo/issues/22507
+        let not_ready_for_load = self.loader.borrow().is_blocked() ||
+            !self.is_fully_active() ||
+            is_in_delaying_load_events_mode;
+
+        if not_ready_for_load {
             // Step 6.
             return;
         }
@@ -1952,8 +1976,6 @@ impl Document {
 
                 window.reflow(ReflowGoal::Full, ReflowReason::DocumentLoaded);
 
-                document.notify_constellation_load();
-
                 if let Some(fragment) = document.url().fragment() {
                     document.check_and_scroll_fragment(fragment);
                 }
@@ -2008,8 +2030,26 @@ impl Document {
         // Step 11.
         // TODO: ready for post-load tasks.
 
-        // Step 12.
-        // TODO: completely loaded.
+        // Step 12: completely loaded.
+        // https://html.spec.whatwg.org/multipage/#completely-loaded
+        // TODO: fully implement "completely loaded".
+        let document = Trusted::new(self);
+        if document.root().browsing_context().is_some() {
+            self.window
+                .task_manager()
+                .dom_manipulation_task_source()
+                .queue(
+                    task!(completely_loaded: move || {
+                    let document = document.root();
+                    document.completely_loaded.set(true);
+                    // Note: this will, among others, result in the "iframe-load-event-steps" being run.
+                    // https://html.spec.whatwg.org/multipage/#iframe-load-event-steps
+                    document.notify_constellation_load();
+                }),
+                    self.window.upcast(),
+                )
+                .unwrap();
+        }
     }
 
     // https://html.spec.whatwg.org/multipage/#pending-parsing-blocking-script
@@ -2701,6 +2741,7 @@ impl Document {
             fired_unload: Cell::new(false),
             responsive_images: Default::default(),
             redirect_count: Cell::new(0),
+            completely_loaded: Cell::new(false),
             script_and_layout_blockers: Cell::new(0),
             delayed_tasks: Default::default(),
         }

components/script/dom/htmliframeelement.rs

@@ -274,8 +274,13 @@ impl HTMLIFrameElement {
         );
 
         let pipeline_id = self.pipeline_id();
-        // If the initial `about:blank` page is the current page, load with replacement enabled.
-        let replace = pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get();
+        // If the initial `about:blank` page is the current page, load with replacement enabled,
+        // see https://html.spec.whatwg.org/multipage/#the-iframe-element:about:blank-3
+        let is_about_blank =
+            pipeline_id.is_some() && pipeline_id == self.about_blank_pipeline_id.get();
+        // Replacement enabled also takes into account whether the document is "completely loaded",
+        // see https://html.spec.whatwg.org/multipage/#the-iframe-element:completely-loaded
+        let replace = is_about_blank || !document.is_completely_loaded();
         self.navigate_or_reload_child_browsing_context(
             Some(load_data),
             NavigationType::Regular,

components/script/dom/servoparser/mod.rs

@@ -380,8 +380,7 @@ impl ServoParser {
         self.document.set_current_parser(None);
 
         // Step 4.
-        self.document
-            .set_ready_state(DocumentReadyState::Interactive);
+        self.document.set_ready_state(DocumentReadyState::Complete);
     }
 
     // https://html.spec.whatwg.org/multipage/#active-parser

components/script/dom/window.rs

@@ -1421,7 +1421,9 @@ impl Window {
             dom_count: self.Document().dom_count(),
         };
 
-        self.layout_chan.send(Msg::Reflow(reflow)).unwrap();
+        self.layout_chan
+            .send(Msg::Reflow(reflow))
+            .expect("Layout thread disconnected.");
 
         debug!("script: layout forked");
 
@@ -1783,8 +1785,14 @@ impl Window {
             }
         }
 
-        // Step 7
+        // Step 8
         if doc.prompt_to_unload(false) {
+            if self.window_proxy().parent().is_some() {
+                // Step 10
+                // If browsingContext is a nested browsing context,
+                // then put it in the delaying load events mode.
+                self.window_proxy().start_delaying_load_events_mode();
+            }
             self.main_thread_script_chan()
                 .send(MainThreadScriptMsg::Navigate(
                     pipeline_id,

components/script/dom/windowproxy.rs

@@ -93,6 +93,9 @@ pub struct WindowProxy {
 
     /// The parent browsing context's window proxy, if this is a nested browsing context
     parent: Option<Dom<WindowProxy>>,
+
+    /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+    delaying_load_events_mode: Cell<bool>,
 }
 
 impl WindowProxy {
@@ -117,6 +120,7 @@ impl WindowProxy {
             disowned: Cell::new(false),
             frame_element: frame_element.map(Dom::from_ref),
             parent: parent.map(Dom::from_ref),
+            delaying_load_events_mode: Cell::new(false),
             opener,
         }
     }
@@ -312,6 +316,26 @@ impl WindowProxy {
         None
     }
 
+    /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+    pub fn is_delaying_load_events_mode(&self) -> bool {
+        self.delaying_load_events_mode.get()
+    }
+
+    /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+    pub fn start_delaying_load_events_mode(&self) {
+        self.delaying_load_events_mode.set(true);
+    }
+
+    /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+    pub fn stop_delaying_load_events_mode(&self) {
+        self.delaying_load_events_mode.set(false);
+        if let Some(document) = self.document() {
+            if !document.loader().events_inhibited() {
+                ScriptThread::mark_document_with_no_blocked_loads(&document);
+            }
+        }
+    }
+
     // https://html.spec.whatwg.org/multipage/#disowned-its-opener
     pub fn disown(&self) {
         self.disowned.set(true);

components/script/script_thread.rs

@@ -1314,6 +1314,10 @@ impl ScriptThread {
         // into this loop too, but for now it's only images.
         debug!("Issuing batched reflows.");
         for (_, document) in self.documents.borrow().iter() {
+            // Step 13
+            if !document.is_fully_active() {
+                continue;
+            }
             let window = document.window();
             let pending_reflows = window.get_pending_reflow_count();
             if pending_reflows > 0 {
@@ -1396,6 +1400,7 @@ impl ScriptThread {
         match *msg {
             MixedMessage::FromConstellation(ref inner_msg) => {
                 match *inner_msg {
+                    StopDelayingLoadEventsMode(id) => Some(id),
                     NavigationResponse(id, _) => Some(id),
                     AttachLayout(ref new_layout_info) => Some(new_layout_info.new_pipeline_id),
                     Resize(id, ..) => Some(id),
@@ -1533,6 +1538,9 @@ impl ScriptThread {
 
     fn handle_msg_from_constellation(&self, msg: ConstellationControlMsg) {
         match msg {
+            ConstellationControlMsg::StopDelayingLoadEventsMode(pipeline_id) => {
+                self.handle_stop_delaying_load_events_mode(pipeline_id)
+            },
             ConstellationControlMsg::NavigationResponse(id, fetch_data) => {
                 match fetch_data {
                     FetchResponseMsg::ProcessResponse(metadata) => {
@@ -2078,6 +2086,19 @@ impl ScriptThread {
         }
     }
 
+    fn handle_stop_delaying_load_events_mode(&self, pipeline_id: PipelineId) {
+        let window = self.documents.borrow().find_window(pipeline_id);
+        if let Some(window) = window {
+            match window.undiscarded_window_proxy() {
+                Some(window_proxy) => window_proxy.stop_delaying_load_events_mode(),
+                None => warn!(
+                    "Attempted to take {} of 'delaying-load-events-mode' after having been discarded.",
+                    pipeline_id
+                ),
+            };
+        }
+    }
+
     fn handle_unload_document(&self, pipeline_id: PipelineId) {
         let document = self.documents.borrow().find_document(pipeline_id);
         if let Some(document) = document {
@@ -2164,6 +2185,20 @@ impl ScriptThread {
                         status: Some((204...205, _)),
                         ..
                     }) => {
+                        // If we have an existing window that is being navigated:
+                        if let Some(window) = self.documents.borrow().find_window(id.clone()) {
+                            let window_proxy = window.window_proxy();
+                            // https://html.spec.whatwg.org/multipage/
+                            // #navigating-across-documents:delaying-load-events-mode-2
+                            if window_proxy.parent().is_some() {
+                                // The user agent must take this nested browsing context
+                                // out of the delaying load events mode
+                                // when this navigation algorithm later matures,
+                                // or when it terminates (whether due to having run all the steps,
+                                // or being canceled, or being aborted), whichever happens first.
+                                window_proxy.stop_delaying_load_events_mode();
+                            }
+                        }
                         self.script_sender
                             .send((id.clone(), ScriptMsg::AbortLoadUrl))
                             .unwrap();
@@ -2708,6 +2743,13 @@ impl ScriptThread {
             incomplete.parent_info,
             incomplete.opener,
         );
+        if window_proxy.parent().is_some() {
+            // https://html.spec.whatwg.org/multipage/#navigating-across-documents:delaying-load-events-mode-2
+            // The user agent must take this nested browsing context
+            // out of the delaying load events mode
+            // when this navigation algorithm later matures.
+            window_proxy.stop_delaying_load_events_mode();
+        }
         window.init_window_proxy(&window_proxy);
 
         let last_modified = metadata.headers.as_ref().and_then(|headers| {

components/script_traits/lib.rs

@@ -246,6 +246,10 @@ pub enum UpdatePipelineIdReason {
 /// Messages sent from the constellation or layout to the script thread.
 #[derive(Deserialize, Serialize)]
 pub enum ConstellationControlMsg {
+    /// Takes the associated window proxy out of "delaying-load-events-mode",
+    /// used if a scheduled navigated was refused by the embedder.
+    /// https://html.spec.whatwg.org/multipage/#delaying-load-events-mode
+    StopDelayingLoadEventsMode(PipelineId),
     /// Sends the final response to script thread for fetching after all redirections
     /// have been resolved
     NavigationResponse(PipelineId, FetchResponseMsg),
@@ -338,6 +342,7 @@ impl fmt::Debug for ConstellationControlMsg {
     fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
         use self::ConstellationControlMsg::*;
         let variant = match *self {
+            StopDelayingLoadEventsMode(..) => "StopDelayingLoadsEventMode",
             NavigationResponse(..) => "NavigationResponse",
             AttachLayout(..) => "AttachLayout",
             Resize(..) => "Resize",
@@ -724,6 +729,8 @@ pub enum ConstellationMsg {
     IsReadyToSaveImage(HashMap<PipelineId, Epoch>),
     /// Inform the constellation of a key event.
     Keyboard(KeyboardEvent),
+    /// Whether to allow script to navigate.
+    AllowNavigationResponse(PipelineId, bool),
     /// Request to load a page.
     LoadUrl(TopLevelBrowsingContextId, ServoUrl),
     /// Request to traverse the joint session history of the provided browsing context.
@@ -768,6 +775,7 @@ impl fmt::Debug for ConstellationMsg {
             GetFocusTopLevelBrowsingContext(..) => "GetFocusTopLevelBrowsingContext",
             IsReadyToSaveImage(..) => "IsReadyToSaveImage",
             Keyboard(..) => "Keyboard",
+            AllowNavigationResponse(..) => "AllowNavigationResponse",
             LoadUrl(..) => "LoadUrl",
             TraverseHistory(..) => "TraverseHistory",
             WindowSize(..) => "WindowSize",

components/servo/lib.rs

@@ -279,6 +279,16 @@ where
                 self.compositor.on_resize_window_event();
             },
 
+            WindowEvent::AllowNavigationResponse(pipeline_id, allowed) => {
+                let msg = ConstellationMsg::AllowNavigationResponse(pipeline_id, allowed);
+                if let Err(e) = self.constellation_chan.send(msg) {
+                    warn!(
+                        "Sending allow navigation to constellation failed ({:?}).",
+                        e
+                    );
+                }
+            },
+
             WindowEvent::LoadUrl(top_level_browsing_context_id, url) => {
                 let msg = ConstellationMsg::LoadUrl(top_level_browsing_context_id, url);
                 if let Err(e) = self.constellation_chan.send(msg) {

ports/libmlservo/src/lib.rs

@@ -161,12 +161,15 @@ pub unsafe extern "C" fn heartbeat_servo(servo: *mut ServoInstance) {
     // Servo heartbeat goes here!
     if let Some(servo) = servo.as_mut() {
         servo.servo.handle_events(vec![]);
-        for ((_browser_id, event)) in servo.servo.get_events() {
+        for ((browser_id, event)) in servo.servo.get_events() {
             match event {
                 // Respond to any messages with a response channel
                 // to avoid deadlocking the constellation
-                EmbedderMsg::AllowNavigation(_url, sender) => {
-                    let _ = sender.send(true);
+                EmbedderMsg::AllowNavigationRequest(pipeline_id, _url) => {
+                    if let Some(_browser_id) = browser_id {
+                        let window_event = WindowEvent::AllowNavigationResponse(pipeline_id, true);
+                        servo.servo.handle_events(vec![window_event]);
+                    }
                 },
                 EmbedderMsg::GetSelectedBluetoothDevice(_, sender) => {
                     let _ = sender.send(None);

ports/libsimpleservo/src/api.rs

@@ -388,7 +388,7 @@ impl ServoGlue {
     }
 
     fn handle_servo_events(&mut self) -> Result<(), &'static str> {
-        for (_browser_id, event) in self.servo.get_events() {
+        for (browser_id, event) in self.servo.get_events() {
             match event {
                 EmbedderMsg::ChangePageTitle(title) => {
                     let fallback_title: String = if let Some(ref current_url) = self.current_url {
@@ -403,10 +403,11 @@ impl ServoGlue {
                     let title = format!("{} - Servo", title);
                     self.callbacks.host_callbacks.on_title_changed(title);
                 },
-                EmbedderMsg::AllowNavigation(_url, response_chan) => {
-                    if let Err(e) = response_chan.send(true) {
-                        warn!("Failed to send allow_navigation() response: {}", e);
-                    };
+                EmbedderMsg::AllowNavigationRequest(pipeline_id, _url) => {
+                    if let Some(_browser_id) = browser_id {
+                        let window_event = WindowEvent::AllowNavigationResponse(pipeline_id, true);
+                        let _ = self.process_event(window_event);
+                    }
                 },
                 EmbedderMsg::HistoryChanged(entries, current) => {
                     let can_go_back = current > 0;

ports/servo/browser.rs

@@ -290,9 +290,10 @@ impl Browser {
                             .push(WindowEvent::SendError(browser_id, reason));
                     }
                 },
-                EmbedderMsg::AllowNavigation(_url, sender) => {
-                    if let Err(e) = sender.send(true) {
-                        warn!("Failed to send AllowNavigation response: {}", e);
+                EmbedderMsg::AllowNavigationRequest(pipeline_id, _url) => {
+                    if let Some(_browser_id) = browser_id {
+                        self.event_queue
+                            .push(WindowEvent::AllowNavigationResponse(pipeline_id, true));
                     }
                 },
                 EmbedderMsg::AllowOpeningBrowser(response_chan) => {

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/008.html.ini

@@ -1,5 +0,0 @@
-[008.html]
-  type: testharness
-  [Link with onclick form submit to javascript url and href navigation ]
-    expected: FAIL
-

tests/wpt/metadata/html/browsers/browsing-the-web/navigating-across-documents/009.html.ini

@@ -1,5 +0,0 @@
-[009.html]
-  type: testharness
-  [Link with onclick form submit to javascript url with document.write and href navigation ]
-    expected: FAIL
-