servo: Merge #16477 - Pass URL to Browser::new(), delegate url checking logic to 3rd party (from sadmansk:url_param_browser); r=paulrouget
authorSadman Kazi <sadman@sadmansk.com>
Sun, 11 Jun 2017 21:16:06 -0700
changeset 411551 f1ecd59649e4253c42aec200da396fc4591f27cd
parent 411550 70bfa8c83b16b1e3dd4477a6f9aa7c8d03ea6072
child 411552 9305945e748c7196d5927ba08da9185d1a52b597
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaulrouget
bugs16477, 15636
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 #16477 - Pass URL to Browser::new(), delegate url checking logic to 3rd party (from sadmansk:url_param_browser); r=paulrouget <!-- Please describe your changes on the following line: --> 1. Move the logic of computing the initial url from `opts.rs` to `/ports/servo/main.rs` 2. Add a `ServoUrl` argument to `Browser::new` Based on the requested changes by @paulrouget: >We can read the pref in main() instead. shell.homepage would be used if the url is not passed as an argument. I'm trying to decouple the "app" logic and the "web engine" logic. I think it's up to the app to set the initial URL, and I'm not sure the initial url should be part of opts. --- <!-- 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 #15636 <!-- Either: --> - [ ] There are tests for these changes <!-- 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. --> Source-Repo: https://github.com/servo/servo Source-Revision: 87140641a4f8636b431db41777d01302f8f6ad3d
servo/components/config/opts.rs
servo/components/servo/lib.rs
servo/ports/cef/browser.rs
servo/ports/servo/main.rs
--- a/servo/components/config/opts.rs
+++ b/servo/components/config/opts.rs
@@ -475,17 +475,17 @@ fn default_user_agent_string(agent: User
 const DEFAULT_USER_AGENT: UserAgent = UserAgent::Android;
 
 #[cfg(not(target_os = "android"))]
 const DEFAULT_USER_AGENT: UserAgent = UserAgent::Desktop;
 
 pub fn default_opts() -> Opts {
     Opts {
         is_running_problem_test: false,
-        url: Some(ServoUrl::parse("about:blank").unwrap()),
+        url: None,
         tile_size: 512,
         device_pixels_per_px: None,
         time_profiling: None,
         time_profiler_trace_path: None,
         mem_profiler_period: None,
         nonincremental_layout: false,
         userscripts: None,
         user_stylesheets: Vec::new(),
@@ -626,40 +626,34 @@ pub fn from_cmdline_args(args: &[String]
         }
     }
 
     if debug_options.help {
         print_debug_usage(app_name)
     }
 
     let cwd = env::current_dir().unwrap();
-    let homepage_pref = PREFS.get("shell.homepage");
     let url_opt = if !opt_match.free.is_empty() {
         Some(&opt_match.free[0][..])
     } else {
-        homepage_pref.as_string()
+        None
     };
     let is_running_problem_test =
         url_opt
         .as_ref()
         .map_or(false, |url|
              url.starts_with("http://web-platform.test:8000/2dcontext/drawing-images-to-the-canvas/") ||
              url.starts_with("http://web-platform.test:8000/_mozilla/mozilla/canvas/") ||
              url.starts_with("http://web-platform.test:8000/_mozilla/css/canvas_over_area.html"));
 
-    let url = match url_opt {
-        Some(url_string) => {
-            parse_url_or_filename(&cwd, url_string)
-                .unwrap_or_else(|()| args_fail("URL parsing failed"))
-        },
-        None => {
-            print_usage(app_name, &opts);
-            args_fail("servo asks that you provide a URL")
-        }
-    };
+    let url_opt = url_opt.and_then(|url_string| parse_url_or_filename(&cwd, url_string)
+                                   .or_else(|error| {
+                                       warn!("URL parsing failed ({:?}).", error);
+                                       Err(error)
+                                   }).ok());
 
     let tile_size: usize = match opt_match.opt_str("s") {
         Some(tile_size_str) => tile_size_str.parse()
             .unwrap_or_else(|err| args_fail(&format!("Error parsing option: -s ({})", err))),
         None => 512,
     };
 
     let device_pixels_per_px = opt_match.opt_str("device-pixel-ratio").map(|dppx_str|
@@ -770,17 +764,17 @@ pub fn from_cmdline_args(args: &[String]
     let do_not_use_native_titlebar =
         opt_match.opt_present("b") ||
         !PREFS.get("shell.native-titlebar.enabled").as_boolean().unwrap();
 
     let is_printing_version = opt_match.opt_present("v") || opt_match.opt_present("version");
 
     let opts = Opts {
         is_running_problem_test: is_running_problem_test,
-        url: Some(url),
+        url: url_opt,
         tile_size: tile_size,
         device_pixels_per_px: device_pixels_per_px,
         time_profiling: time_profiling,
         time_profiler_trace_path: opt_match.opt_str("profiler-trace-path"),
         mem_profiler_period: mem_profiler_period,
         nonincremental_layout: nonincremental_layout,
         userscripts: opt_match.opt_default("userscripts", ""),
         user_stylesheets: user_stylesheets,
--- a/servo/components/servo/lib.rs
+++ b/servo/components/servo/lib.rs
@@ -117,17 +117,17 @@ pub use servo_url as url;
 /// loop to pump messages between the embedding application and
 /// various browser components.
 pub struct Browser<Window: WindowMethods + 'static> {
     compositor: IOCompositor<Window>,
     constellation_chan: Sender<ConstellationMsg>,
 }
 
 impl<Window> Browser<Window> where Window: WindowMethods + 'static {
-    pub fn new(window: Rc<Window>) -> Browser<Window> {
+    pub fn new(window: Rc<Window>, target_url: ServoUrl) -> Browser<Window> {
         // Global configuration options, parsed from the command line.
         let opts = opts::get();
 
         // Make sure the gl context is made current.
         window.prepare_for_composite(0, 0);
 
         // Get both endpoints of a special channel for communication between
         // the client window and the compositor. This channel is unique because
@@ -198,17 +198,17 @@ impl<Window> Browser<Window> where Windo
         // can't defer it after `create_constellation` has started.
         script::init();
 
         // Create the constellation, which maintains the engine
         // pipelines, including the script and layout threads, as well
         // as the navigation context.
         let (constellation_chan, sw_senders) = create_constellation(opts.user_agent.clone(),
                                                                     opts.config_dir.clone(),
-                                                                    opts.url.clone(),
+                                                                    target_url,
                                                                     compositor_proxy.clone_compositor_proxy(),
                                                                     time_profiler_chan.clone(),
                                                                     mem_profiler_chan.clone(),
                                                                     debugger_chan,
                                                                     devtools_chan,
                                                                     supports_clipboard,
                                                                     &webrender,
                                                                     webrender_api_sender.clone());
@@ -282,17 +282,17 @@ fn create_compositor_channel(event_loop_
         },
      CompositorReceiver {
          receiver: receiver
      })
 }
 
 fn create_constellation(user_agent: Cow<'static, str>,
                         config_dir: Option<PathBuf>,
-                        url: Option<ServoUrl>,
+                        url: ServoUrl,
                         compositor_proxy: CompositorProxy,
                         time_profiler_chan: time::ProfilerChan,
                         mem_profiler_chan: mem::ProfilerChan,
                         debugger_chan: Option<debugger::Sender>,
                         devtools_chan: Option<Sender<devtools_traits::DevtoolsControlMsg>>,
                         supports_clipboard: bool,
                         webrender: &webrender::Renderer,
                         webrender_api_sender: webrender_traits::RenderApiSender)
@@ -332,19 +332,17 @@ fn create_constellation(user_agent: Cow<
         let (mut handler, sender) = WebVRCompositorHandler::new();
         let webvr_thread = WebVRThread::spawn(constellation_chan.clone(), sender);
         handler.set_webvr_thread_sender(webvr_thread.clone());
 
         webrender.set_vr_compositor_handler(handler);
         constellation_chan.send(ConstellationMsg::SetWebVRThread(webvr_thread)).unwrap();
     }
 
-    if let Some(url) = url {
-        constellation_chan.send(ConstellationMsg::InitLoadUrl(url)).unwrap();
-    };
+    constellation_chan.send(ConstellationMsg::InitLoadUrl(url)).unwrap();
 
     // channels to communicate with Service Worker Manager
     let sw_senders = SWManagerSenders {
         swmanager_sender: from_swmanager_sender,
         resource_sender: resource_sender
     };
 
     (constellation_chan, sw_senders)
--- a/servo/ports/cef/browser.rs
+++ b/servo/ports/cef/browser.rs
@@ -4,16 +4,18 @@
 
 use browser_host::{ServoCefBrowserHost, ServoCefBrowserHostExtensions};
 use eutil::Downcast;
 use frame::{ServoCefFrame, ServoCefFrameExtensions};
 use interfaces::{CefBrowser, CefBrowserHost, CefClient, CefFrame, CefRequestContext};
 use interfaces::{cef_browser_t, cef_browser_host_t, cef_client_t, cef_frame_t};
 use interfaces::{cef_request_context_t};
 use servo::Browser;
+use servo::servo_config::prefs::PREFS;
+use servo::servo_url::ServoUrl;
 use types::{cef_browser_settings_t, cef_string_t, cef_window_info_t, cef_window_handle_t};
 use window;
 use wrappers::CefWrap;
 
 use compositing::windowing::{WindowNavigateMsg, WindowEvent};
 use glutin_app;
 use libc::c_int;
 use std::cell::{Cell, RefCell};
@@ -125,17 +127,19 @@ impl ServoCefBrowser {
     pub fn new(window_info: &cef_window_info_t, client: CefClient) -> ServoCefBrowser {
         let frame = ServoCefFrame::new().as_cef_interface();
         let host = ServoCefBrowserHost::new(client.clone()).as_cef_interface();
         let mut window_handle: cef_window_handle_t = get_null_window_handle();
 
         let (glutin_window, servo_browser) = if window_info.windowless_rendering_enabled == 0 {
             let parent_window = glutin_app::WindowID::new(window_info.parent_window as *mut _);
             let glutin_window = glutin_app::create_window(Some(parent_window));
-            let servo_browser = Browser::new(glutin_window.clone());
+            let home_url = ServoUrl::parse(PREFS.get("shell.homepage").as_string()
+                    .unwrap_or("about:blank")).unwrap();
+            let servo_browser = Browser::new(glutin_window.clone(), home_url);
             window_handle = glutin_window.platform_window().window as cef_window_handle_t;
             (Some(glutin_window), ServoBrowser::OnScreen(servo_browser))
         } else {
             (None, ServoBrowser::Invalid)
         };
 
         let id = ID_COUNTER.with(|counter| {
             counter.fetch_add(1, Ordering::SeqCst)
@@ -166,17 +170,19 @@ pub trait ServoCefBrowserExtensions {
     fn pinch_zoom_level(&self) -> f32;
 }
 
 impl ServoCefBrowserExtensions for CefBrowser {
     fn init(&self, window_info: &cef_window_info_t) {
         if window_info.windowless_rendering_enabled != 0 {
             let window = window::Window::new(window_info.width, window_info.height);
             window.set_browser(self.clone());
-            let servo_browser = Browser::new(window.clone());
+            let home_url = ServoUrl::parse(PREFS.get("shell.homepage").as_string()
+                    .unwrap_or("about:blank")).unwrap();
+            let servo_browser = Browser::new(window.clone(), home_url);
             *self.downcast().servo_browser.borrow_mut() = ServoBrowser::OffScreen(servo_browser);
         }
 
         self.downcast().host.set_browser((*self).clone());
         self.downcast().frame.set_browser((*self).clone());
         if window_info.windowless_rendering_enabled == 0 {
             self.downcast().host.initialize_compositing();
         }
--- a/servo/ports/servo/main.rs
+++ b/servo/ports/servo/main.rs
@@ -32,16 +32,18 @@ extern crate sig;
 
 use backtrace::Backtrace;
 use servo::Browser;
 use servo::compositing::windowing::WindowEvent;
 #[cfg(target_os = "android")]
 use servo::config;
 use servo::config::opts::{self, ArgumentParsingResult};
 use servo::config::servo_version;
+use servo::servo_config::prefs::PREFS;
+use servo::servo_url::ServoUrl;
 use std::env;
 use std::panic;
 use std::process;
 use std::rc::Rc;
 use std::thread;
 
 pub mod platform {
     #[cfg(target_os = "macos")]
@@ -138,20 +140,26 @@ fn main() {
 
     if opts::get().is_printing_version {
         println!("{}", servo_version());
         process::exit(0);
     }
 
     let window = app::create_window(None);
 
+    // If the url is not provided, we fallback to the homepage in PREFS,
+    // or a blank page in case the homepage is not set either.
+    let target_url = opts::get().url.clone()
+                .unwrap_or(ServoUrl::parse(PREFS.get("shell.homepage").as_string()
+                    .unwrap_or("about:blank")).unwrap());
+
     // Our wrapper around `Browser` that also implements some
     // callbacks required by the glutin window implementation.
     let mut browser = BrowserWrapper {
-        browser: Browser::new(window.clone()),
+        browser: Browser::new(window.clone(), target_url)
     };
 
     browser.browser.setup_logging();
 
     register_glutin_resize_handler(&window, &mut browser);
 
     browser.browser.handle_events(vec![WindowEvent::InitializeCompositing]);