Closed Bug 922464 (OMT-nsIURI) Opened 6 years ago Closed 6 months ago
[meta] Centralize URI parsing and make it threadsafe
We'd really like to make more code runnable on worker threads. One of the things that we basically always need anytime we're interacting with necko is dealing with nsIURIs. However since nsIURIs currently can be implemented by addons, and created through addon-provided factories, we can neither create nsIURIs or interact with them off the main thread. However the whole idea of having addons provide nsIURI parser, just means that we're encouraging more different ways of parsing URIs. More unity would be good. So what we could do is something like this: * Remove nsIProtocolHandler.newURI * Add some way for a protocol handlers to declare if they want to use a nsIURI or nsIURL. I.e. if they want to be parsed using a plain URI parser or one that supports hierarchical directories. Possibly other metadata would need to be signaled too. * Make nsIURI and nsIURL immutable. * Write a single URI parser which is used to parse all URIs. Initially we probably need that parser to carry over all quirky differences between parsing of chrome:// and resource:// etc. * Make said parser available off the main thread by caching metadata about custom schemes in a threadsafe hash. Once we have done that we should also do * Change our parsers to match the URL spec that's currently in the works. * Remove the differences between our various hierarchical parsers as to increase code reuse. Another nice-to-have is: * Make our error handling more consistent so that URI handling either fails at nsIURI-parse time, or that it fails after nsIChannel::AsyncOpen has been called. I.e. ideally creating an nsIChannel from a URI, and calling AsyncOpen on it should always succeed.
https://mxr.mozilla.org/addons/search?string=contract%2B%40mozilla.org/network/protocol%3B1%3Fname lists 59 addons that would likely need to be updated. An alternative approach here would be to keep nsIProtocolHandler.newURI, but only use it when URIs are created from the main thread. For off-main-thread creations of URIs we'd only support internal schemes as well as schemes that were added through the declarative form in bullet two above. For such off-main-thread URIs we'd make the nsIURI immutable using nsIMutable. We could also add warnings whenever a URI was created using nsIProtocolHandler.newURI but that there was no declarative equivalent. That way the above list can hopefully be shortened. We could likewise add warnings to all nsIURI mutators in order to phase out mutable URIs.
> * Add some way for a protocol handlers to declare if they want to use a nsIURI > or nsIURL. I.e. if they want to be parsed using a plain URI parser or one that > supports hierarchical directories. Possibly other metadata would need to be > signaled too. Is protocolFlags insufficient?
URI_NORELATIVE might indeed be enough to tell if we should use hierarchical url parsing or "plain" parsing.
> Is protocolFlags insufficient? Yes. What's needed is a _declarative_ way that doesn't involve calling into protocol-handler-provided code at URI parse time. Precaching all the protocolFlags for all protocol handlers in the system would work, but we have no way to enumerate them right now.
What would a declarative way look like if it's not some form of function/getter on nsIProtocolHandler? We can always proxy to the main thread whenever get a request for a new scheme for the first time. Though that does suck complexity and performance wise.
> What would a declarative way look like For example, something registered with an XPCOM category whose contents we can just read at startup. This can include calling code, of course, as long as we only do that on the main thread... Given category observers, we can even allow dynamic addition/removal.
This may affect comm-central as we probably also define some URI protocols.
Whiteboard: [necko-would-take] → [necko-would-take][qf]
[qf-]. Sadly this isn't something we can do for 57 because of add-on compatibility issues.
Whiteboard: [necko-would-take][qf] → [necko-would-take][qf-]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P5
Summary: Centralize URI parsing and make it threadsafe → [meta] Centralize URI parsing and make it threadsafe
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.