1485652 - Make nsNSSSocketInfo::IsAcceptableForHost work asynchronously on the socket process

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement, P2)

Description

[Honza Bambas (:mayhemer)]

This is used for h2 conn joining an performs the cert chain verification, synchronously on the socket thread.

This method is going to be called on the socket process.  Certificate validation can only be done on the parent process, hence we need to IPC it, ideally asynchronously, in the worst case synchronously.

This will have two parts, if done async:
- PSM, should be fairly easy
- Necko, must wait for the result asynchronously

The Necko async part is going to be quite complicated.  Usage:

nsHttpConnectionMgr::TryDispatchTransaction,
nsHttpConnectionMgr::ProcessSpdyPendingQ,
nsHttpConnectionMgr::GetOrCreateConnectionEntry,
nsHttpConnectionMgr::nsConnectionEntry::AvailableForDispatchNow

all call it via:

nsHttpConnectionMgr::GetSpdyActiveConn -> nsHttpConnectionMgr::FindCoalescableConnection -> nsHttpConnectionMgr::FindCoalescableConnectionByHashKey -> nsHttpConnection -> ASpdySession -> nsISSLSocketControl.  


Patrick, Nick, do you happen to know about some way around it?  Note that right now we are already doing the verification on the socket process in a blocking way.  If we do a sync call to the parent and do it to a dedicated thread that can't be blocked on any sync calls to the socket process, we are probably safe.

Comment 1

[u408661]

Can you elaborate what you mean by "some way around it"? Do you mean some way of getting around the call into nsISSLSocketControl? Unfortunately, I don't believe so - all that info is required to know if we can merge the connections. We already coalesce more than other browsers do (though within the bounds of what's allowed by the spec), which leads to periodic confusion on the part of server implementors (and bug reports that I have to close). If we try to get clever, I'm afraid it will be far too easy to fall into a situation where we are improperly coalescing, and then we'll have real bugs to fix.

But, perhaps I'm misunderstanding your question.

(FTR, I think what you propose in your last sentence will work fine.)

Comment 2

[Honza Bambas (:mayhemer)]

What I would prefer is to turn the synchronous bool getter "IsAcceptableForHost" to be async.  But returning the same result as we do now.  I.e. to change the API, not the decision.

This is probably going to be very hard as the stack is deep and the code complicated.

Hence, doing this in a sync way is probably the way to go.  If there are issues, we will deal with them later.

I'm moving this to necko since the hard work would happen there.

Comment 3

[Honza Bambas (:mayhemer)]

(In reply to Honza Bambas (:mayhemer) from comment #2)
> Hence, doing this in a sync way is probably the way to go.  If there are

I mean through *sync IPC*

Comment 4

[Dragana Damjanovic [:dragana]]

Attachment: WIP Implement remote IsAcceptableForHost

Comment 5

[Kershaw Chang [:kershaw]]

Attachment: Bug 1485652 - Reimplement IsAcceptableForHost

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0774925b94a9
Reimplement IsAcceptableForHost r=keeler

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/0774925b94a9