Closed Bug 1062005 Opened 10 years ago Closed 10 years ago

provide thread-safe basic url parsing

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: bkelly, Assigned: bkelly)

Details

Attachments

(1 file)

As I understand it, we currently provide URI/URL parsing via nsIURI and nsIURL which can only be used from the main thread. I've been told this is largely due to the fact that nsIURI can be js implemented (bug 888604). Also, the IO service uses plugins to provide different protocol handlers which cannot be accessed off main thread. My main question here is, is there any reason we could not implement this basic url parsing algorithm in a thread-safe way? http://url.spec.whatwg.org/#concept-basic-url-parser As far as I can tell, its protocol agnostic. The reason I want this is that the ServiceWorker Cache object has the concept of "ignore the URL search field" on many operations. With the current nsIURI implementation this means I have to bounce to the main thread for nearly every operation or make the lookup extremely inefficient in the ignoreSearch case. I'm willing to implement this algorithm, but I'd like to hear if people have objections or other ideas. Thanks. Putting this in Netwerk since thats where NS_NewURI() and friends live.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
> As far as I can tell, its protocol agnostic. It's not. It has all sorts of places where it's scheme-dependent. See the scheme checks in "relative slash state", the "relative scheme" stuff in the scheme state and the things that depend on the "relative flag", probably others. Now what we _could_ do is implement this spec in all its non-extensible glory. Assuming that's actually backwards-compatible... Note that Jonas has a plan for making nsIURI available on background threads, so you may want to talk to him about timeframes.
Jonas, can you point me towards your plans for fixing off-main-thread URL manipulation? Thanks!
Flags: needinfo?(jonas)
I'm inclined to implement the Cache `ignoreSearch` feature only for http/https schemes initially. If/when we fix our off-main-thread URI story we can expand it to include other schemes. I spoke with Jake on IRC and he seemed amenable to this plan. He had me open this issue: https://github.com/slightlyoff/ServiceWorker/issues/440
Assignee: bkelly → nobody
Status: ASSIGNED → NEW
After looking at the code, I think that for the SW ignoreSearch feature for HTTP(S) URLs, all we need is to use nsStdURLParser. Its implementation doesn't do anything not thread safe that I can see as long as we don't pass references to it across threads (its AddRef/Release are not thread safe, but even that is easy to fix if we need to.) The only complication is that this is an XPCOM service so we can't just do_CreateInstance() it on the background thread. However, I suggest exporting nsURLParsers.h and creating the C++ object directly.
Note that this stuff obviously won't work for add-on provided URL scheme parsers, but I don't think we need to block on that for SW (I am assuming that we don't support add-ons which provide their own way of parsing HTTP(S) URLs!)
I've verified that Ehsan's suggestion works for my particular use case. This patch exports the nsURLParsers.h header from necko. Patrick, for context, I want to use nsStdURLParser to extract the query from a URL off the main thread. At this point I know the URL is http/https and so does not need extended scheme support. I would like to avoid having to bounce to the main thread just to do this parse with NS_NewURI().
Attachment #8484455 - Flags: review?(mcmanus)
Comment on attachment 8484455 [details] [diff] [review] Export nsURLParsers.h to allow use of nsStdURLParser. Review of attachment 8484455 [details] [diff] [review]: ----------------------------------------------------------------- I really want thread safe uri functions too - thanks for working on this - it would be great to see a full interface available. The socket thread does some things with uris in strings for a couple of corner cases that it should really do with real nsIURIs for this same reason.
Attachment #8484455 - Flags: review?(mcmanus) → review+
The short of my plan is: 1. Add a way for addons to register a URL parser for a particular scheme. However rather than by registering a callback which converts a string to an nsIURI instance, the API registers a set of flags which indicate how the URI should be parsed. I.e. should it be treated like a URL with '/' separated folders, or as a non-heirarchical URI (like data:), or like an origin with some data, (like blob:). Or any other parsing methods that we think might make sense. 2. Deprecate nsIProtocolHandler.newURI and evangelize to addons to stop using it. 3. Deprecate all mutators on nsIURI/nsIURL, like the nsIURL filePath setter. If needed add functions on nsIURI/nsIURL like "give me a new nsIURL but with filePath set to X". 4. Remove nsIProtocolHandler.newURI and nsIURI/nsIURL mutators after a release or two. This way only our own parsers will be generating nsIURI instances. And those instances will be readonly. That should make it easy to make the nsIURI instances threadsafe, as well as make the uri parser threadsafe. However obviously this is not a short-term plan.
Assignee: nobody → bkelly
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: