From 708c561ae698788c39d7d0ef7d796f349d7f218b Mon Sep 17 00:00:00 2001 From: Sadman Kazi Date: Sun, 23 Apr 2017 20:17:36 -0400 Subject: [PATCH] Pass URL to Browser::new(), delegate url checking logic to third party Use Option instead of ServoUrl for url in opts Cleaner mapping from parse_url to url_opt Add comment about url parsing error Print reason for parsing error Remove comment about warning Add home url when openning new browser window in CEF Fix merge error --- components/config/opts.rs | 22 ++++++++-------------- components/servo/lib.rs | 10 ++++------ ports/cef/browser.rs | 10 ++++++++-- ports/servo/main.rs | 10 +++++++++- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/components/config/opts.rs b/components/config/opts.rs index 2a51577b14a0..d01a5dae4803 100644 --- a/components/config/opts.rs +++ b/components/config/opts.rs @@ -480,7 +480,7 @@ 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, @@ -631,11 +631,10 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult { } 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 @@ -645,16 +644,11 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult { 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() @@ -775,7 +769,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult { 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, diff --git a/components/servo/lib.rs b/components/servo/lib.rs index 22279a4189bd..899ccec99863 100644 --- a/components/servo/lib.rs +++ b/components/servo/lib.rs @@ -122,7 +122,7 @@ pub struct Browser { } impl Browser where Window: WindowMethods + 'static { - pub fn new(window: Rc) -> Browser { + pub fn new(window: Rc, target_url: ServoUrl) -> Browser { // Global configuration options, parsed from the command line. let opts = opts::get(); @@ -203,7 +203,7 @@ impl Browser where Window: WindowMethods + 'static { // 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(), @@ -287,7 +287,7 @@ fn create_compositor_channel(event_loop_waker: Box, config_dir: Option, - url: Option, + url: ServoUrl, compositor_proxy: CompositorProxy, time_profiler_chan: time::ProfilerChan, mem_profiler_chan: mem::ProfilerChan, @@ -337,9 +337,7 @@ fn create_constellation(user_agent: Cow<'static, str>, 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 { diff --git a/ports/cef/browser.rs b/ports/cef/browser.rs index 22affc6f76a5..678ff6b3478a 100644 --- a/ports/cef/browser.rs +++ b/ports/cef/browser.rs @@ -9,6 +9,8 @@ use interfaces::{CefBrowser, CefBrowserHost, CefClient, CefFrame, CefRequestCont 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; @@ -130,7 +132,9 @@ impl ServoCefBrowser { 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 { @@ -171,7 +175,9 @@ impl ServoCefBrowserExtensions for CefBrowser { 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); } diff --git a/ports/servo/main.rs b/ports/servo/main.rs index 7d390988be0a..6f21fa7dd838 100644 --- a/ports/servo/main.rs +++ b/ports/servo/main.rs @@ -37,6 +37,8 @@ use servo::compositing::windowing::WindowEvent; 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; @@ -143,10 +145,16 @@ fn main() { 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();