Open Bug 1157585 Opened 6 years ago Updated 5 years ago

Consider refactoring DocShell: give it control over which CookieService, AuthProvider and CacheService to give to HttpHandler


(Core :: DOM: Navigation, defect)

Not set





(Reporter: mildred-bug.mozilla, Unassigned)


(Blocks 1 open bug)


User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150420225811
Summary: Refactoring DocShell HttpHandler → Consider refactoring DocShell: give it control over which CookieService, AuthProvider and CacheService to give to HttpHandler
I was looking at the Mozilla code, and found out about the current design, which I consider could be improved. The areas I am interested with has to do with all the information that is persistent across HTTP requests (the cache, the cookie, the HTTP auth info, and perhaps more) and that is stored globally. What I would like to achieve is for each browser element to be able to have a separate version of each of those services.

This could be used by Firefox or addons to have a mode where private information is not shared between all contexts. This could be used also to have different contexts where you are logged-in as different users or not logged-in at all. I also think we could rework the HTTP Auth prompt and integrate it in a more modern way in the chrome, allow log-out from within the browser tab.

I looked at the code and I could describe the current design this way:

Basically, the <browser/> XUL element corresponds to a DocShell. When loading a new page, the DocShell will ask a nsIURILoader, which will in turn ask a nsIChannel (which I suppose is commonly implemented by nsHttpChannel).

- In nsHttpChannel::Connect(), the global cache service is located
- In HttpBaseChannel::AddCookiesToRequest(), nsHttpHandler::GetCookieService() is called to get the global cookies service
- In nsHttpChannel::BeginConnect(), the global auth provider is located

What I would like to do is to add new methods or arguments to nsIURILoader and nsIChannel so the DocShell could choose different implementation for any of the three services above. The DocShell should also expose the fact that it can have separate implementation of those.

This is my first experience with all this code, and I would like your opinion of whether you think this is a good idea. I would like starting working on it.
First thing I'd like to would be:

- a method in nsIChannel getAuthCache() that would return nsHttpAuthCache*. At first, it would be implemented as gHttpHandler->AuthCache(mIsPrivate)

- Use this method in nsHttpChannelAuthProvider::GetCredentialsForChallenge instead of gHttpHandler->AuthCache(mIsPrivate)
There is an issue with the above comment, the nsHttpAuthCache is specific to HTTP while nsIChannel is not, and nsHttpAuthCache does not implement any interface, so we would have to create one to reference it from nsIChannel.

But it seems that nsHttpAuthCache uses the appId in the cache key, which is defined in DocShell::GetAppId(). That could be a convenient way to have different DocShells not share the same auth cache.

Also, the DocShell also implements nsIAuthPromptProvider. So the DocShell can choose the UI to display when an authentication request is in progress.
A question to those familiar with that code:

Would you think that separating contexts should be done by duplicating the instance of services (multiple instances of CookieServices, HttpAuthCache ...) or should be done by adding a unique identifier to those databases (like the appId)?
Component: Networking → Document Navigation
Having multiple instances of the services do not seem to be the right way of doing things. Nearly everyone can depend on it, and many extensions are surely written to access the global services.

It seems more likely to provide a SessionId to the DocShell when loading a URI. Then, this session identifier could be used as a unique key for cookies and auth cache.
Why is a separate concept of a session id preferable to assigning separate docshells different app ids, instead?
You need to log in before you can comment on or make changes to this bug.