docshell/browsingcontext/webnavigation loadURI methods now require nsIURI

[Gijs Kruitbosch]

TL;DR: the `loadURI` method on nsIWebNavigation (often accessed on docshell objects) and CanonicalBrowsingContext now take an nsIURI as their first parameter. Despite their name, they used to take strings and run "fixup" on them to try to get an nsIURI.

If you land a patch that passes a string instead, that won't compile (for C++ callers) or it will throw a runtime exception (JS callers: NS_ERROR_XPC_BAD_CONVERT_JS: Could not convert JavaScript argument arg 0 [nsIWebNavigation.loadURI]). I apologize for any patch bitrot this may cause.

You can either pass an nsIURI instead (recommended), or if you are getting arbitrary input as strings, can call `fixupAndLoadURIString` instead (which takes a string and will use nsIURIFixup to parse/guess at what the URI should be).

(Note: for browser mochitests, you can use BrowserTestUtils.loadURIString which still takes a string.)

Note that Emilio also recently landed an API addition that allows you to get an nsIURI directly if you have a DOM URL, and vice versa, without copying and reparsing the underlying string, which might help depending on your context.

These same changes have also been made to the identically-named APIs exposed on the `browser` custom element and on the `gBrowser`/tabbrowser JS wrappers.

I intend to file follow-ups to convert more callsites to `loadURI` (instead of `fixupAndLoadURIString`) and directly pass nsIURIs. This will avoid repeatedly copying and reparsing the same URL (which can be expensive, especially for large data URIs) and makes the behaviour more predictable. Quite often consumers already have/need an nsIURI instance (to start a speculative connection, or to pass to places APIs, or to decide what process the browsing context needs to switch to, or...) and so making sure that that URI corresponds to what is being loaded should help both performance and reliability going forward.

Thanks,
Gijs

[Daniel Veditz]

Thanks for the effort you put into this!

In addition to the performance gains, this will help prevent future security bugs. You know this, of course (and hinted at it with the mention of more predictable behaviour), but I want everyone else to appreciate that benefit of your work. String URLs are a temptation to evil (ad hoc parsing): any difference in how one section of code interprets a URL compared to another could easily be a security bug. URLs may not be as bad as names, but they do have edge cases that aren't obvious from the way most web URLs look. Always use the common parsing routines and operate on nsIURIs (or Principals) rather than strings.

-Dan Veditz