Fix a SPDY crash bug where a CHECK() is hit because a cancelled stream is considered active by the …

Fix a SPDY crash bug where a CHECK() is hit because a cancelled stream is considered active by the spdy session.

BUG=139375
TEST=SpdySessionSpdy2Test.CancelStream


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

[willchan no longer on Chromium, 2012-07-27 21:41:42]

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session.h (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session.h:227: // Delete a stream that has been created, but not
yet activated.
The difference between deleting and closing a created stream is not obvious. See
the CloseStream() and CloseCreatedStream() functions above. You should add
comments explaining the difference.

[ramant (doing other things), 2012-07-27 21:43:58]

lgtm

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session.cc (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session.cc:744: }
nit: consider calling DeleteCreatedStream. It is your call.

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session_spdy2_unittest.cc (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session_spdy2_unittest.cc:1311: 
overly nit: consider deleting extra blank line. :-).

[Ryan Hamilton, 2012-07-27 21:51:35]

Thanks for the quick review!

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session.cc (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session.cc:744: }
On 2012/07/27 21:43:58, ramant wrote:
> nit: consider calling DeleteCreatedStream. It is your call.

I'll simply delete the newly created method DeleteCreatedStream.  That was a
mistake on my part.  Thanks for catching.

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session.h (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session.h:227: // Delete a stream that has been created, but not
yet activated.
On 2012/07/27 21:41:42, willchan wrote:
> The difference between deleting and closing a created stream is not obvious.
See
> the CloseStream() and CloseCreatedStream() functions above. You should add
> comments explaining the difference.

This was an oversight on my part.  (I can't believe that I missed
CloseCreatedStream().)  I will delete the newly added method and call that
instead.

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
File net/spdy/spdy_session_spdy2_unittest.cc (right):

https://chromiumcodereview.appspot.com/10829066/diff/2001/net/spdy/spdy_sessi...
net/spdy/spdy_session_spdy2_unittest.cc:1311: 
On 2012/07/27 21:43:58, ramant wrote:
> overly nit: consider deleting extra blank line. :-).

Done.

[commit-bot: I haz the power, 2012-07-27 21:51:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/10829066/7002

[commit-bot: I haz the power, 2012-07-27 23:16:27]

Try job failure for 10829066-7002 (retry) (retry) (retry) on linux_rel for steps
"nacl_integration, net_unittests".
It's a second try, previously, steps "nacl_integration, net_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-07-27 23:26:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/10829066/14001

[commit-bot: I haz the power, 2012-07-28 02:13:21]

Change committed as 148865