Do not set io_loop_ to NULL until we have deleted request_ and set...

Do not set io_loop_ to NULL until we have deleted request_ and set
request_ to NULL, otherwise Cancel() may fail to post a CancelURLRequest
task because io_loop_ is NULL but request_ is not NULL.  (That task adds
a reference to the OCSPRequestSession object to ensure that it lives
longer than the URLRequest object.)

R=ukai
BUG=28526
TEST=none

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

[wtc, 2009-12-03 05:25:10]

pmarks, could you revert ukai's CL in
http://codereview.chromium.org/449009 and test
this CL?  Thanks.

I believe this CL fixes the regression of r32580 that pmarks
tracked down using bisection (see http://crbug.com/28913).

In
http://src.chromium.org/viewvc/chrome/trunk/src/net/ocsp/nss_ocsp.cc?r1=32580...

note the change around line 214 / line 218.  That change
reversed the order in which we set request_ and io_loop_
to NULL.

This CL reverts that change, and then puts the
  io_loop_ = NULL;
statement in a locked block.

[wtc, 2009-12-03 05:49:29]

I just uploaded Patch Set 2, which is a better fix.
It enables Cancel() to not have to post a CancelURLRequest
task in the normal case (because request_ has been deleted).

pmarks, if you don't have much time, please just test
Patch Set 2.  Thanks.

[ukai, 2009-12-03 05:58:15]

LGTM.
Thanks!

[pmarks, 2009-12-03 07:50:39]

My steps:
1. sync & compile
2. test -> crash!
3. apply patch
4. compile
5. test -> does not crash

LGTM from an "it appears to work" perspective, though I haven't tried to study
the control flow.

[wtc, 2009-12-03 19:08:02]

pmarks: thanks for testing the patch.  You don't need to
review it.

I removed a debugging DCHECK (because it accesses request_
on a worker thread -- request_ should only be used by the
IO thread) and committed the CL.