Refactor embedder NewBrowser flow

components/compositing/windowing.rs

@@ -8,7 +8,6 @@ use embedder_traits::EventLoopWaker;
 use euclid::TypedScale;
 #[cfg(feature = "gleam")]
 use gleam::gl;
-use ipc_channel::ipc::IpcSender;
 use msg::constellation_msg::{Key, KeyModifiers, KeyState, TopLevelBrowsingContextId, TraversalDirection};
 use script_traits::{MouseButton, TouchEventType, TouchId};
 use servo_geometry::{DeviceIndependentPixel, DeviceUintLength};
@@ -75,7 +74,7 @@ pub enum WindowEvent {
     /// Sent when Ctr+R/Apple+R is called to reload the current page.
     Reload(TopLevelBrowsingContextId),
     /// Create a new top level browsing context
-    NewBrowser(ServoUrl, IpcSender<TopLevelBrowsingContextId>),
+    NewBrowser(ServoUrl, TopLevelBrowsingContextId),
     /// Close a top level browsing context
     CloseBrowser(TopLevelBrowsingContextId),
     /// Panic a top level browsing context.

components/constellation/constellation.rs

@@ -578,7 +578,8 @@ where
                 let swmanager_receiver =
                     route_ipc_receiver_to_new_mpsc_receiver_preserving_errors(swmanager_receiver);
 
-                PipelineNamespace::install(PipelineNamespaceId(0));
+                // Zero is reserved for the embedder.
+                PipelineNamespace::install(PipelineNamespaceId(1));
 
                 let mut constellation: Constellation<Message, LTF, STF> = Constellation {
                     script_sender: ipc_script_sender,
@@ -605,8 +606,10 @@ where
                     pipelines: HashMap::new(),
                     browsing_contexts: HashMap::new(),
                     pending_changes: vec![],
-                    // We initialize the namespace at 1, since we reserved namespace 0 for the constellation
+                    // We initialize the namespace at 2,
-                    next_pipeline_namespace_id: PipelineNamespaceId(1),
+                    // since we reserved namespace 0 for the embedder,
+                    // and 0 for the constellation
+                    next_pipeline_namespace_id: PipelineNamespaceId(2),
                     focus_pipeline_id: None,
                     time_profiler_chan: state.time_profiler_chan,
                     mem_profiler_chan: state.mem_profiler_chan,
@@ -1026,8 +1029,8 @@ where
             },
             // Create a new top level browsing context. Will use response_chan to return
             // the browsing context id.
-            FromCompositorMsg::NewBrowser(url, response_chan) => {
+            FromCompositorMsg::NewBrowser(url, top_level_browsing_context_id) => {
-                self.handle_new_top_level_browsing_context(url, response_chan);
+                self.handle_new_top_level_browsing_context(url, top_level_browsing_context_id);
             },
             // Close a top level browsing context.
             FromCompositorMsg::CloseBrowser(top_level_browsing_context_id) => {
@@ -1651,17 +1654,12 @@ where
     fn handle_new_top_level_browsing_context(
         &mut self,
         url: ServoUrl,
-        reply: IpcSender<TopLevelBrowsingContextId>,
+        top_level_browsing_context_id: TopLevelBrowsingContextId
     ) {
         let window_size = self.window_size.initial_viewport;
         let pipeline_id = PipelineId::new();
-        let top_level_browsing_context_id = TopLevelBrowsingContextId::new();
+        let msg = (Some(top_level_browsing_context_id), EmbedderMsg::BrowserCreated(top_level_browsing_context_id));
-        if let Err(e) = reply.send(top_level_browsing_context_id) {
+        self.embedder_proxy.send(msg);
-            warn!(
-                "Failed to send newly created top level browsing context ({}).",
-                e
-            );
-        }
         let browsing_context_id = BrowsingContextId::from(top_level_browsing_context_id);
         let load_data = LoadData::new(url.clone(), None, None, None);
         let sandbox = IFrameSandboxState::IFrameUnsandboxed;

components/script_traits/lib.rs

@@ -717,7 +717,7 @@ pub enum ConstellationMsg {
     /// Dispatch WebVR events to the subscribed script threads.
     WebVREvents(Vec<PipelineId>, Vec<WebVREvent>),
     /// Create a new top level browsing context.
-    NewBrowser(ServoUrl, IpcSender<TopLevelBrowsingContextId>),
+    NewBrowser(ServoUrl, TopLevelBrowsingContextId),
     /// Close a top level browsing context.
     CloseBrowser(TopLevelBrowsingContextId),
     /// Panic a top level browsing context.

components/servo/lib.rs

@@ -87,6 +87,7 @@ use gaol::sandbox::{ChildSandbox, ChildSandboxMethods};
 use gfx::font_cache_thread::FontCacheThread;
 use ipc_channel::ipc::{self, IpcSender};
 use log::{Log, Metadata, Record};
+use msg::constellation_msg::{PipelineNamespace, PipelineNamespaceId};
 use net::resource_thread::new_resource_threads;
 use net_traits::IpcSend;
 use profile::mem as profile_mem;
@@ -135,6 +136,9 @@ impl<Window> Servo<Window> where Window: WindowMethods + 'static {
         // Make sure the gl context is made current.
         window.prepare_for_composite(Length::new(0), Length::new(0));
 
+        // Reserving a namespace to create TopLevelBrowserContextId.
+        PipelineNamespace::install(PipelineNamespaceId(0));
+
         // Get both endpoints of a special channel for communication between
         // the client window and the compositor. This channel is unique because
         // messages to client may need to pump a platform-specific event loop
@@ -326,8 +330,8 @@ impl<Window> Servo<Window> where Window: WindowMethods + 'static {
                 self.compositor.capture_webrender();
             }
 
-            WindowEvent::NewBrowser(url, response_chan) => {
+            WindowEvent::NewBrowser(url, browser_id) => {
-                let msg = ConstellationMsg::NewBrowser(url, response_chan);
+                let msg = ConstellationMsg::NewBrowser(url, browser_id);
                 if let Err(e) = self.constellation_chan.send(msg) {
                     warn!("Sending NewBrowser message to constellation failed ({}).", e);
                 }

ports/libsimpleservo/src/api.rs

@@ -8,7 +8,6 @@ use servo::compositing::windowing::{AnimationState, EmbedderCoordinates, MouseWi
 use servo::embedder_traits::EmbedderMsg;
 use servo::embedder_traits::resources::{self, Resource};
 use servo::euclid::{Length, TypedPoint2D, TypedScale, TypedSize2D, TypedVector2D};
-use servo::ipc_channel::ipc;
 use servo::msg::constellation_msg::TraversalDirection;
 use servo::script_traits::{MouseButton, TouchEventType};
 use servo::servo_config::opts;
@@ -75,7 +74,14 @@ pub struct ServoGlue {
     servo: Servo<ServoCallbacks>,
     batch_mode: bool,
     callbacks: Rc<ServoCallbacks>,
-    browser_id: BrowserId,
+    /// id of the top level browsing context. It is unique as tabs
+    /// are not supported yet. None until created.
+    browser_id: Option<BrowserId>,
+    // A rudimentary stack of "tabs".
+    // EmbedderMsg::BrowserCreated will push onto it.
+    // EmbedderMsg::CloseBrowser will pop from it,
+    // and exit if it is empty afterwards.
+    browsers: Vec<BrowserId>,
     events: Vec<WindowEvent>,
     current_url: Option<ServoUrl>,
 }
@@ -133,28 +139,34 @@ pub fn init(
         waker,
     });
 
-    let mut servo = Servo::new(callbacks.clone());
+    let servo = Servo::new(callbacks.clone());
-
-    let (sender, receiver) = ipc::channel().map_err(|_| "Can't create ipc::channel")?;
-    servo.handle_events(vec![WindowEvent::NewBrowser(url.clone(), sender)]);
-    let browser_id = receiver.recv().map_err(|_| "Can't receive browser_id")?;
-    servo.handle_events(vec![WindowEvent::SelectBrowser(browser_id)]);
 
     SERVO.with(|s| {
-        *s.borrow_mut() = Some(ServoGlue {
+        let mut servo_glue = ServoGlue {
             servo,
             batch_mode: false,
             callbacks,
-            browser_id,
+            browser_id: None,
+            browsers: vec![],
             events: vec![],
-            current_url: Some(url),
+            current_url: Some(url.clone()),
-        });
+        };
+        let browser_id = BrowserId::new();
+        let _ = servo_glue.process_event(WindowEvent::NewBrowser(url, browser_id));
+        *s.borrow_mut() = Some(servo_glue);
     });
 
     Ok(())
 }
 
 impl ServoGlue {
+    fn get_browser_id(&self) -> Result<BrowserId, &'static str> {
+        let browser_id = match self.browser_id {
+            Some(id) => id,
+            None => return Err("No BrowserId set yet.")
+        };
+        Ok(browser_id)
+    }
     /// This is the Servo heartbeat. This needs to be called
     /// everytime wakeup is called or when embedder wants Servo
     /// to act on its pending events.
@@ -182,7 +194,8 @@ impl ServoGlue {
         ServoUrl::parse(url)
             .map_err(|_| "Can't parse URL")
             .and_then(|url| {
-                let event = WindowEvent::LoadUrl(self.browser_id, url);
+                let browser_id = self.get_browser_id()?;
+                let event = WindowEvent::LoadUrl(browser_id, url);
                 self.process_event(event)
             })
     }
@@ -190,7 +203,8 @@ impl ServoGlue {
     /// Reload the page.
     pub fn reload(&mut self) -> Result<(), &'static str> {
         debug!("reload");
-        let event = WindowEvent::Reload(self.browser_id);
+        let browser_id = self.get_browser_id()?;
+        let event = WindowEvent::Reload(browser_id);
         self.process_event(event)
     }
 
@@ -203,14 +217,16 @@ impl ServoGlue {
     /// Go back in history.
     pub fn go_back(&mut self) -> Result<(), &'static str> {
         debug!("go_back");
-        let event = WindowEvent::Navigation(self.browser_id, TraversalDirection::Back(1));
+        let browser_id = self.get_browser_id()?;
+        let event = WindowEvent::Navigation(browser_id, TraversalDirection::Back(1));
         self.process_event(event)
     }
 
     /// Go forward in history.
     pub fn go_forward(&mut self) -> Result<(), &'static str> {
         debug!("go_forward");
-        let event = WindowEvent::Navigation(self.browser_id, TraversalDirection::Forward(1));
+        let browser_id = self.get_browser_id()?;
+        let event = WindowEvent::Navigation(browser_id, TraversalDirection::Forward(1));
         self.process_event(event)
     }
 
@@ -329,7 +345,31 @@ impl ServoGlue {
                     info!("Alert: {}", message);
                     let _ = sender.send(());
                 },
-                EmbedderMsg::CloseBrowser |
+                EmbedderMsg::AllowOpeningBrowser(response_chan) => {
+                    // Note: would be a place to handle pop-ups config.
+                    // see Step 7 of #the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name
+                    if let Err(e) = response_chan.send(true) {
+                        warn!("Failed to send AllowOpeningBrowser response: {}", e);
+                    };
+                },
+                EmbedderMsg::BrowserCreated(new_browser_id) => {
+                    // TODO: properly handle a new "tab"
+                    self.browsers.push(new_browser_id);
+                    if self.browser_id.is_none() {
+                        self.browser_id = Some(new_browser_id);
+                    }
+                    self.events.push(WindowEvent::SelectBrowser(new_browser_id));
+                },
+                EmbedderMsg::CloseBrowser => {
+                    // TODO: close the appropriate "tab".
+                    let _ = self.browsers.pop();
+                    if let Some(prev_browser_id) = self.browsers.last() {
+                        self.browser_id = Some(*prev_browser_id);
+                        self.events.push(WindowEvent::SelectBrowser(*prev_browser_id));
+                    } else {
+                        self.events.push(WindowEvent::Quit);
+                    }
+                },
                 EmbedderMsg::Status(..) |
                 EmbedderMsg::SelectFiles(..) |
                 EmbedderMsg::MoveTo(..) |

ports/servo/browser.rs

@@ -67,11 +67,6 @@ impl Browser {
         mem::replace(&mut self.event_queue, Vec::new())
     }
 
-    pub fn set_browser_id(&mut self, browser_id: BrowserId) {
-        self.browser_id = Some(browser_id);
-        self.browsers.push(browser_id);
-    }
-
     pub fn handle_window_events(&mut self, events: Vec<WindowEvent>) {
         for event in events {
             match event {
@@ -302,6 +297,9 @@ impl Browser {
                 EmbedderMsg::BrowserCreated(new_browser_id) => {
                     // TODO: properly handle a new "tab"
                     self.browsers.push(new_browser_id);
+                    if self.browser_id.is_none() {
+                        self.browser_id = Some(new_browser_id);
+                    }
                     self.event_queue.push(WindowEvent::SelectBrowser(new_browser_id));
                 }
                 EmbedderMsg::KeyEvent(ch, key, state, modified) => {
@@ -332,6 +330,7 @@ impl Browser {
                     // TODO: close the appropriate "tab".
                     let _ = self.browsers.pop();
                     if let Some(prev_browser_id) = self.browsers.last() {
+                        self.browser_id = Some(*prev_browser_id);
                         self.event_queue.push(WindowEvent::SelectBrowser(*prev_browser_id));
                     } else {
                         self.event_queue.push(WindowEvent::Quit);

ports/servo/non_android_main.rs

@@ -27,11 +27,10 @@ mod resources;
 mod browser;
 
 use backtrace::Backtrace;
-use servo::Servo;
+use servo::{Servo, BrowserId};
 use servo::compositing::windowing::WindowEvent;
 use servo::config::opts::{self, ArgumentParsingResult, parse_url_or_filename};
 use servo::config::servo_version;
-use servo::ipc_channel::ipc;
 use servo::servo_config::prefs::PREFS;
 use servo::servo_url::ServoUrl;
 use std::env;
@@ -150,12 +149,8 @@ pub fn main() {
     let target_url = cmdline_url.or(pref_url).or(blank_url).unwrap();
 
     let mut servo = Servo::new(window.clone());
-
+    let browser_id = BrowserId::new();
-    let (sender, receiver) = ipc::channel().unwrap();
+    servo.handle_events(vec![WindowEvent::NewBrowser(target_url, browser_id)]);
-    servo.handle_events(vec![WindowEvent::NewBrowser(target_url, sender)]);
-    let browser_id = receiver.recv().unwrap();
-    browser.set_browser_id(browser_id);
-    servo.handle_events(vec![WindowEvent::SelectBrowser(browser_id)]);
 
     servo.setup_logging();