Closed Bug 922464 (OMT-nsIURI) Opened 11 years ago Closed 5 years ago

[meta] Centralize URI parsing and make it threadsafe

Categories

(Core :: Networking, defect, P5)

defect

Tracking

()

RESOLVED FIXED
Performance Impact none

People

(Reporter: sicking, Unassigned)

References

(Depends on 4 open bugs, Blocks 3 open bugs)

Details

(Keywords: meta, perf, Whiteboard: [necko-would-take])

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.
Whiteboard: [necko-would-take]
Alias: OMT-nsIURI
No longer depends on: post-57-api-changes
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-]
Priority: -- → P5
Keywords: perf
Depends on: 1425889
Depends on: 1432613
Depends on: 1459861
Keywords: meta
Summary: Centralize URI parsing and make it threadsafe → [meta] Centralize URI parsing and make it threadsafe

With bug 1536744 closed I think we can officially call this project complete.
The remaining bugs are just usability/performance improvements.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

If you're not already planning to, would it be possible to send a mail to dev-platform about this and any limitations people should still be aware of? I think it would be great to both recognize this fantastic achievement and also help perhaps surface any further plans about allowing principal creation from any thread/etc. in a way that's visible to everyone interested in the platform. Thanks!

Flags: needinfo?(valentin.gosu)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #13)

If you're not already planning to, would it be possible to send a mail to dev-platform

https://groups.google.com/forum/#!topic/mozilla.dev.platform/ILkf8vTRZsI

Flags: needinfo?(valentin.gosu)
Performance Impact: --- → -
Whiteboard: [necko-would-take][qf-] → [necko-would-take]
You need to log in before you can comment on or make changes to this bug.