Auto merge of #16477 - sadmansk:url_param_browser, r=paulrouget

Pass URL to Browser::new(), delegate url checking logic to 3rd party

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16477)
<!-- Reviewable:end -->

4 files changed, 29 insertions, 23 deletions

diff --git a/components/config/opts.rs b/components/config/opts.rs
index 2a51577b14a..d01a5dae480 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 22279a4189b..899ccec9986 100644
--- a/components/servo/lib.rs
+++ b/components/servo/lib.rs
@@ -122,7 +122,7 @@ pub struct Browser<Window: WindowMethods + 'static> {
 }
 
 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();
 
@@ -203,7 +203,7 @@ impl<Window> Browser<Window> 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<compositor_thread::EventLoopW
 
 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,
@@ -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 22affc6f76a..678ff6b3478 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 7d390988be0..6f21fa7dd83 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();