Add HttpStreamFactory Job orphaning semantics.

Add HttpStreamFactory Job orphaning semantics.

BUG=54371, 69688
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=76389

[willchan no longer on Chromium, 2011-02-27 23:42:32]

This builds on top of my previous changelist that I haven't landed yet (hoping
to get mmenke or eroman to review that soon).

This changelist isn't done. Other than assertions to make sure I didn't break
existing behavior, I haven't really made sure orphaning works properly. I'm
going to fix that by handling SPDY late binding (not SPDY ip connection pooling)
and adding a test for that to verify the orphaning logic works properly.

I'm sending this out now so you can see the slight change to the APIs, so you
can see how the SPDY ip connection pooling change will integrate. I'll update
this for reals later tonight or tomorrow.

[willchan no longer on Chromium, 2011-03-01 00:22:42]

Ok, code is all updated. There are probably a lot of tests I should write. I'm
going to focus on that now. But this is ready for review and hopefully I won't
uncover any bugs when I write the tests. The existing tests make sure my late
binding changes haven't broken anything, but there probably aren't any tests to
make sure late binding is giving us the speedup we intend.

http://codereview.chromium.org/6591030/diff/4001/net/http/http_stream_factory...
File net/http/http_stream_factory_impl.cc (right):

http://codereview.chromium.org/6591030/diff/4001/net/http/http_stream_factory...
net/http/http_stream_factory_impl.cc:88: // TODO(willchan): If it's important,
switch RequestSet out for a FIFO
Please comment here if you think this is important.

http://codereview.chromium.org/6591030/diff/4001/net/http/http_stream_factory...
net/http/http_stream_factory_impl.cc:102: // TODO(mbelshe): Alert other valid
requests.
You will need to update this to search other valid hostnames (where ips match
up).

http://codereview.chromium.org/6591030/diff/3015/net/http/http_stream_factory...
File net/http/http_stream_factory_impl_request.cc (right):

http://codereview.chromium.org/6591030/diff/3015/net/http/http_stream_factory...
net/http/http_stream_factory_impl_request.cc:97: // TODO(mbelshe): Revisit this
when we implement ip connection pooling of
Please chime in on this.

[willchan no longer on Chromium, 2011-03-01 03:36:59]

Wow, turns out my code worked out on the first try! I've added two tests to show
that late binding works. They basically force the other sockets to hang.

[Mike Belshe, 2011-03-01 17:09:19]

lgtm

http://codereview.chromium.org/6591030/diff/7001/net/http/http_stream_factory...
File net/http/http_stream_factory_impl.h (right):

http://codereview.chromium.org/6591030/diff/7001/net/http/http_stream_factory...
net/http/http_stream_factory_impl.h:90: // destroyed.
"...deleted when the factory is destroyed." sounds like when the browser dies?

From looking at the code, its not nearly that bad.  The orphaned jobs simply
park here until the IO completes, at which point we clean them up and remove
them from this set?

[willchan no longer on Chromium, 2011-03-01 17:48:27]

http://codereview.chromium.org/6591030/diff/7001/net/http/http_stream_factory...
File net/http/http_stream_factory_impl.h (right):

http://codereview.chromium.org/6591030/diff/7001/net/http/http_stream_factory...
net/http/http_stream_factory_impl.h:90: // destroyed.
On 2011/03/01 17:09:19, Mike Belshe wrote:
> "...deleted when the factory is destroyed." sounds like when the browser dies?
> 
> From looking at the code, its not nearly that bad.  The orphaned jobs simply
> park here until the IO completes, at which point we clean them up and remove
> them from this set?

Oh, I'll clarify this comment. What I mean is that for |request_map_|, we're
guaranteed that it's empty when we hit ~HttpStreamFactoryImpl, because all
URLRequests must be canceled by then. Orphaned jobs aren't tied to a Request, so
they won't be canceled when the URLRequest (and its corresponding Request) are
canceled, so there may be some leftover in this set when the destructor runs. We
delete all the leftovers.