remove sync constellation -> embedder communication

components/compositing/windowing.rs

@@ -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.
+    AllowNavigation(TopLevelBrowsingContextId, ServoUrl, 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::AllowNavigation(..) => write!(f, "AllowNavigation"),
             WindowEvent::LoadUrl(..) => write!(f, "LoadUrl"),
             WindowEvent::MouseWindowEventClass(..) => write!(f, "Mouse"),
             WindowEvent::MouseWindowMoveEventClass(..) => write!(f, "MouseMove"),

components/constellation/constellation.rs

@@ -341,6 +341,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: HashMap<(TopLevelBrowsingContextId, ServoUrl), (LoadData, bool)>,
 }
 
 /// State needed to construct a constellation.
@@ -655,6 +658,7 @@ where
                     webgl_threads: state.webgl_threads,
                     webvr_chan: state.webvr_chan,
                     canvas_chan: CanvasPaintThread::start(),
+                    pending_approval_navigations: HashMap::new(),
                 };
 
                 constellation.run();
@@ -979,6 +983,39 @@ 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::AllowNavigation(top_level_browsing_context_id, url, allowed) => {
+                let key = (top_level_browsing_context_id, url.clone());
+                let (load_data, replace) = match self.pending_approval_navigations.remove(&key) {
+                    Some((load_data, replace)) => (load_data, replace),
+                    None => return warn!(
+                                "AllowNavigation for unknow request: {:?} {:?}",
+                                top_level_browsing_context_id,
+                                url
+                            )
+                };
+                if !allowed {
+                    return;
+                }
+                let ctx_id = BrowsingContextId::from(top_level_browsing_context_id);
+                let pipeline_id = match self.browsing_contexts.get(&ctx_id) {
+                    Some(ctx) => ctx.pipeline_id,
+                    None => {
+                        return warn!(
+                            "LoadUrl for unknow browsing context: {:?}",
+                            top_level_browsing_context_id
+                        )
+                    },
+                };
+                self.handle_load_url_msg(
+                    top_level_browsing_context_id,
+                    pipeline_id,
+                    load_data,
+                    replace,
+                );
+            },
             // 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.
@@ -1111,11 +1148,20 @@ 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);
+                // Allow the embedder to handle the url itself
+                let msg = (
+                    Some(source_top_ctx_id),
+                    EmbedderMsg::AllowNavigation(load_data.url.clone()),
+                );
+                self.embedder_proxy.send(msg);
+                let key = (source_top_ctx_id, load_data.url.clone());
+                if let Some(_) = self.pending_approval_navigations.insert(key, (load_data, replace)) {
+                    warn!("Received a second LoadUrl request from script before the first could be approved from {:?}",
+                        source_top_ctx_id
+                    );
+                }
             },
             FromScriptMsg::AbortLoadUrl => {
                 self.handle_abort_load_url_msg(source_pipeline_id);
@@ -2057,17 +2103,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

@@ -87,7 +87,7 @@ pub enum EmbedderMsg {
     // Show an alert message.
     Alert(String, IpcSender<()>),
     /// Wether or not to follow a link
-    AllowNavigation(ServoUrl, IpcSender<bool>),
+    AllowNavigation(ServoUrl),
     /// Whether or not to allow script to open a new tab/browser
     AllowOpeningBrowser(IpcSender<bool>),
     /// A new browser was created by script

components/script_traits/lib.rs

@@ -740,6 +740,8 @@ pub enum ConstellationMsg {
     IsReadyToSaveImage(HashMap<PipelineId, Epoch>),
     /// Inform the constellation of a key event.
     Keyboard(KeyboardEvent),
+    /// Whether to allow script to navigate.
+    AllowNavigation(TopLevelBrowsingContextId, ServoUrl, bool),
     /// Request to load a page.
     LoadUrl(TopLevelBrowsingContextId, ServoUrl),
     /// Request to traverse the joint session history of the provided browsing context.
@@ -784,6 +786,7 @@ impl fmt::Debug for ConstellationMsg {
             GetFocusTopLevelBrowsingContext(..) => "GetFocusTopLevelBrowsingContext",
             IsReadyToSaveImage(..) => "IsReadyToSaveImage",
             Keyboard(..) => "Keyboard",
+            AllowNavigation(..) => "AllowNavigation",
             LoadUrl(..) => "LoadUrl",
             TraverseHistory(..) => "TraverseHistory",
             WindowSize(..) => "WindowSize",

components/servo/lib.rs

@@ -274,6 +274,13 @@ where
                 self.compositor.on_resize_window_event();
             },
 
+            WindowEvent::AllowNavigation(top_level_browsing_context_id, url, allowed) => {
+                let msg = ConstellationMsg::AllowNavigation(top_level_browsing_context_id, url, allowed);
+                if let Err(e) = self.constellation_chan.send(msg) {
+                    warn!("Sending load url 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

@@ -133,8 +133,9 @@ pub unsafe extern "C" fn heartbeat_servo(servo: *mut ServoInstance) {
             match event {
                 // Respond to any messages with a response channel
                 // to avoid deadlocking the constellation
-                EmbedderMsg::AllowNavigation(_url, sender) => {
-                    let _ = sender.send(true);
+                EmbedderMsg::AllowNavigation(url) => {
+                    let window_event = WindowEvent::AllowNavigation(servo.browser_id, url, true);
+                    servo.servo.handle_events(vec![window_event]);
                 },
                 EmbedderMsg::GetSelectedBluetoothDevice(_, sender) => {
                     let _ = sender.send(None);

ports/libsimpleservo/src/api.rs

@@ -350,7 +350,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 {
@@ -365,10 +365,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::AllowNavigation(url) => {
+                    if let Some(browser_id) = browser_id {
+                        let window_event = WindowEvent::AllowNavigation(browser_id, url, 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::AllowNavigation(url) => {
+                    if let Some(browser_id) = browser_id {
+                        self.event_queue
+                            .push(WindowEvent::AllowNavigation(browser_id, url, true));
                     }
                 },
                 EmbedderMsg::AllowOpeningBrowser(response_chan) => {