servo: Merge #15795 - Let the embedder decide if servo should follow a link or not (from paulrouget:follow-link); r=asajeffrey
authorPaul Rouget <me@paulrouget.com>
Tue, 14 Mar 2017 07:22:23 -0700
changeset 347528 fca8d4a51babdeb5550ffaa3b5e44f75bc309ae0
parent 347527 9555f7ced9de7b1750cbdff82df01931b9ef412b
child 347529 e4b3b7377c7f09e8180f734f1f1bef4506cb54dc
push id31499
push userkwierso@gmail.com
push dateTue, 14 Mar 2017 23:41:39 +0000
treeherdermozilla-central@cef93bf5a0a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasajeffrey
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #15795 - Let the embedder decide if servo should follow a link or not (from paulrouget:follow-link); r=asajeffrey We want to give a chance to the embedder to handle a link itself. Is it a problem that this will add a round trip to the main thread every time `load_url` is called? --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #15655 <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because I'm not sure how to test that <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 808ffffd1e8dab9ca6340a981b797434b34e2e36
servo/components/compositing/compositor.rs
servo/components/compositing/compositor_thread.rs
servo/components/compositing/windowing.rs
servo/components/constellation/constellation.rs
servo/ports/cef/window.rs
servo/ports/glutin/window.rs
--- a/servo/components/compositing/compositor.rs
+++ b/servo/components/compositing/compositor.rs
@@ -539,16 +539,23 @@ impl<Window: WindowMethods> IOCompositor
                 }
 
                 // Inform the embedder that the load has finished.
                 //
                 // TODO(pcwalton): Specify which frame's load completed.
                 self.window.load_end(back, forward, root);
             }
 
+            (Msg::AllowNavigation(url, response_chan), ShutdownState::NotShuttingDown) => {
+                let allow = self.window.allow_navigation(url);
+                if let Err(e) = response_chan.send(allow) {
+                    warn!("Failed to send allow_navigation result ({}).", e);
+                }
+            }
+
             (Msg::DelayedCompositionTimeout(timestamp), ShutdownState::NotShuttingDown) => {
                 if let CompositionRequest::DelayedComposite(this_timestamp) =
                     self.composition_request {
                     if timestamp == this_timestamp {
                         self.composition_request = CompositionRequest::CompositeNow(
                             CompositingReason::DelayedCompositeTimeout)
                     }
                 }
--- a/servo/components/compositing/compositor_thread.rs
+++ b/servo/components/compositing/compositor_thread.rs
@@ -81,16 +81,18 @@ pub enum Msg {
     /// Alerts the compositor that the given pipeline has changed whether it is running animations.
     ChangeRunningAnimationsState(PipelineId, AnimationState),
     /// Replaces the current frame tree, typically called during main frame navigation.
     SetFrameTree(SendableFrameTree, IpcSender<()>),
     /// The load of a page has begun: (can go back, can go forward).
     LoadStart(bool, bool),
     /// The load of a page has completed: (can go back, can go forward, is root frame).
     LoadComplete(bool, bool, bool),
+    /// Wether or not to follow a link
+    AllowNavigation(ServoUrl, IpcSender<bool>),
     /// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
     DelayedCompositionTimeout(u64),
     /// Composite.
     Recomposite(CompositingReason),
     /// Sends an unconsumed key event back to the compositor.
     KeyEvent(Option<char>, Key, KeyState, KeyModifiers),
     /// Script has handled a touch event, and either prevented or allowed default actions.
     TouchEventProcessed(EventResult),
@@ -139,16 +141,17 @@ impl Debug for Msg {
             Msg::Exit => write!(f, "Exit"),
             Msg::ShutdownComplete => write!(f, "ShutdownComplete"),
             Msg::ScrollFragmentPoint(..) => write!(f, "ScrollFragmentPoint"),
             Msg::ChangeRunningAnimationsState(..) => write!(f, "ChangeRunningAnimationsState"),
             Msg::ChangePageTitle(..) => write!(f, "ChangePageTitle"),
             Msg::ChangePageUrl(..) => write!(f, "ChangePageUrl"),
             Msg::SetFrameTree(..) => write!(f, "SetFrameTree"),
             Msg::LoadComplete(..) => write!(f, "LoadComplete"),
+            Msg::AllowNavigation(..) => write!(f, "AllowNavigation"),
             Msg::LoadStart(..) => write!(f, "LoadStart"),
             Msg::DelayedCompositionTimeout(..) => write!(f, "DelayedCompositionTimeout"),
             Msg::Recomposite(..) => write!(f, "Recomposite"),
             Msg::KeyEvent(..) => write!(f, "KeyEvent"),
             Msg::TouchEventProcessed(..) => write!(f, "TouchEventProcessed"),
             Msg::SetCursor(..) => write!(f, "SetCursor"),
             Msg::CreatePng(..) => write!(f, "CreatePng"),
             Msg::ViewportConstrained(..) => write!(f, "ViewportConstrained"),
--- a/servo/components/compositing/windowing.rs
+++ b/servo/components/compositing/windowing.rs
@@ -132,16 +132,18 @@ pub trait WindowMethods {
     /// Called when the browser chrome should display a status message.
     fn status(&self, Option<String>);
     /// Called when the browser has started loading a frame.
     fn load_start(&self, back: bool, forward: bool);
     /// Called when the browser is done loading a frame.
     fn load_end(&self, back: bool, forward: bool, root: bool);
     /// Called when the browser encounters an error while loading a URL
     fn load_error(&self, code: NetError, url: String);
+    /// Wether or not to follow a link
+    fn allow_navigation(&self, url: ServoUrl) -> bool;
     /// Called when the <head> tag has finished parsing
     fn head_parsed(&self);
 
     /// Returns the scale factor of the system (device pixels / device independent pixels).
     fn hidpi_factor(&self) -> ScaleFactor<f32, DeviceIndependentPixel, DevicePixel>;
 
     /// Creates a channel to the compositor. The dummy parameter is needed because we don't have
     /// UFCS in Rust yet.
--- a/servo/components/constellation/constellation.rs
+++ b/servo/components/constellation/constellation.rs
@@ -1570,16 +1570,23 @@ impl<Message, LTF, STF> Constellation<Me
         }
     }
 
     fn handle_load_url_msg(&mut self, source_id: PipelineId, load_data: LoadData, replace: bool) {
         self.load_url(source_id, load_data, replace);
     }
 
     fn load_url(&mut self, source_id: PipelineId, 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!");
+        self.compositor_proxy.send(ToCompositorMsg::AllowNavigation(load_data.url.clone(), chan));
+        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
         // the new load. The framing element must be notified about the
         // requested change so it can update its internal state.
         //
         // If replace is true, the current entry is replaced instead of a new entry being added.
         let (frame_id, parent_info) = match self.pipelines.get(&source_id) {
--- a/servo/ports/cef/window.rs
+++ b/servo/ports/cef/window.rs
@@ -479,16 +479,20 @@ impl WindowMethods for Window {
                        .get_client()
                        .get_render_handler()
                        .on_cursor_change(browser.clone(), cursor_handle,
                                          self.cursor_type_for_cursor(cursor), &info);
             }
         }
     }
 
+    fn allow_navigation(&self, _: ServoUrl) -> bool {
+        true
+    }
+
     fn supports_clipboard(&self) -> bool {
         false
     }
 }
 
 struct CefCompositorProxy {
     sender: Sender<compositor_thread::Msg>,
 }
--- a/servo/ports/glutin/window.rs
+++ b/servo/ports/glutin/window.rs
@@ -1102,16 +1102,20 @@ impl WindowMethods for Window {
             }
 
             _ => {
                 self.platform_handle_key(key, mods);
             }
         }
     }
 
+    fn allow_navigation(&self, _: ServoUrl) -> bool {
+        true
+    }
+
     fn supports_clipboard(&self) -> bool {
         false
     }
 }
 
 struct GlutinCompositorProxy {
     sender: Sender<compositor_thread::Msg>,
     window_proxy: Option<glutin::WindowProxy>,