Test for WebSocketJob being deleted on the stack

Test for WebSocketJob being deleted on the stack

SocketStreamDispatcherHost can delete the WebSocketJob while it is still
on the stack. Add tests to ensure that WebSocketJob does not attempt to
access its own members after being deleted.

Also fix two cases where WebSocketJob attempted to access its members after
being deleted.

BUG=358038
TEST=net_unittests --gtest_filter=WebSocketJobDeleteTest*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=261707

[tyoshino (SeeGerritForStatus), 2014-04-03 05:21:55]

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
File net/websockets/websocket_job_test.cc (right):

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
net/websockets/websocket_job_test.cc:1225: // doesn't work. Detach it first to
prevent a DCHECK() failure.
Which DCHECK?

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
net/websockets/websocket_job_test.cc:1281: 
let's check that delegate_->job() is NULL at the end of each test.

[Adam Rice, 2014-04-03 06:03:03]

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
File net/websockets/websocket_job_test.cc (right):

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
net/websockets/websocket_job_test.cc:1225: // doesn't work. Detach it first to
prevent a DCHECK() failure.
On 2014/04/03 05:21:56, tyoshino wrote:
> Which DCHECK?

socket_stream.cc:328

I have clarified the comment.

https://codereview.chromium.org/221833002/diff/1/net/websockets/websocket_job...
net/websockets/websocket_job_test.cc:1281: 
On 2014/04/03 05:21:56, tyoshino wrote:
> let's check that delegate_->job() is NULL at the end of each test.

Done.

[tyoshino (SeeGerritForStatus), 2014-04-04 04:39:07]

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
File net/websockets/websocket_job_test.cc (right):

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
net/websockets/websocket_job_test.cc:1227: socket_->DetachDelegate();
is this done by some code before OnClose() is called in real world?

[Adam Rice, 2014-04-04 06:00:36]

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
File net/websockets/websocket_job_test.cc (right):

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
net/websockets/websocket_job_test.cc:1227: socket_->DetachDelegate();
On 2014/04/04 04:39:07, tyoshino wrote:
> is this done by some code before OnClose() is called in real world?

It appears OnClose() is called by real code only in SocketStream::Finish(). That
function does delegate_ = NULL after the call, which is sufficient to avoid the
DCHECK() in the destructor.

It's also called by WebSocketJob::OnCloseSpdyStream(). I have no idea what is
going on there, but since it isn't actually used I'm not going to worry about
it.

[tyoshino (SeeGerritForStatus), 2014-04-04 06:04:44]

lgtm

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
File net/websockets/websocket_job_test.cc (right):

https://codereview.chromium.org/221833002/diff/10001/net/websockets/websocket...
net/websockets/websocket_job_test.cc:1227: socket_->DetachDelegate();
On 2014/04/04 06:00:36, Adam Rice wrote:
> On 2014/04/04 04:39:07, tyoshino wrote:
> > is this done by some code before OnClose() is called in real world?
> 
> It appears OnClose() is called by real code only in SocketStream::Finish().
That
> function does delegate_ = NULL after the call, which is sufficient to avoid
the
> DCHECK() in the destructor.
> 

Ah, right! Thanks for explanation.

> It's also called by WebSocketJob::OnCloseSpdyStream(). I have no idea what is
> going on there, but since it isn't actually used I'm not going to worry about
> it.

[Adam Rice, 2014-04-04 06:06:10]

The CQ bit was checked by ricea@chromium.org

[commit-bot: I haz the power, 2014-04-04 06:06:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ricea@chromium.org/221833002/30001

[commit-bot: I haz the power, 2014-04-04 08:30:56]

Change committed as 261707