Pass the initial URL as an argument to Browser::new, not as a opts property

[paulrouget]

This issue is part of an effort to improve Servo's embedding story. See https://github.com/paulrouget/servoshell/projects/2

Starting Servo from a third party application requires setting the initial URL as an option:

let mut opts = opts::default_opts();
opts.url = ServoUrl::parse(url).ok();
opts::set_defaults(opts);

I would expect to be able to do that instead:

servo::Browser::new(window, url);

[yysung1123]

I would like to work on this!

[paulrouget]

That would be great, but I'd like first to get the confirmation from someone else that this is a valid proposal.

@Ms2ger or @jdm any opinion on this?

Ideally, Servo could run with its default options.

[Ms2ger]

I think the whole setup with InitLoadUrl is kinda weird, but I'd say the proposed change won't make anything worse, so let's go ahead and do it.

[paulrouget]

@deror1869107 It's yours :)

[yysung1123]

@paulrouget

Could I replace Servo::Browser::new with pub fn new(window: Rc<Window>, url: Option<ServoUrl>)?

[paulrouget]

Could I replace Servo::Browser::new with pub fn new(window: Rc, url: Option)?

I think so.

Does the ServoUrl need to be an Option? Is there any reason to support url:None?

[yysung1123]

If url is None, it would use default url from opts::get().

[paulrouget]

Any reason to keep url in opts?

[cbrewster]

@paulrouget we used the url in opts for browserhtml https://github.com/servo/servo/blob/master/resources/package-prefs.json#L5. This allows us to not need a script like the old runservo to provide the bhtml url to servo.

[paulrouget]

@cbrewster 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.

So basically, I would suggest that we do:

1. remove Opts::url
2. move the logic of computing the initial url from opts.rs to /ports/servo/
3. add a ServoUrl argument to Browser::new (not an Option, just ServoUrl)

Does it make sense?

[cbrewster]

@paulrouget sounds good!

[jdm]

#15699 was an attempt at fixing this; the work there had some review comments that needed to be addressed.

[sadmansk]

@jdm I could take over this issue if no one else is working on it.

[jdm]

Please do!

[sadmansk]

@paulrouget do you want there to be a check if the ServoUrlparameter is None before calling InitLoadUrl? Currently I'm expecting a valid url to be passed into Browser::new(). I'll open the PR and add comments to the segments I have questions about.