Closed Bug 922464 (OMT-nsIURI) Opened 6 years ago Closed 6 months ago

[meta] Centralize URI parsing and make it threadsafe

Categories

(Core :: Networking, defect, P5)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: sicking, Unassigned)

References

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

Details

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

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]
Duplicate of this bug: 1332355
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-]
Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P5
Keywords: perf
Depends on: 1425889
Depends on: 1432613
Depends on: 1456088
Depends on: 1459861
Duplicate of this bug: 407758
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: 6 months 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)
You need to log in before you can comment on or make changes to this bug.