Prevent a crash that can occur when redirecting a file URL.

We redirect a file URL that corresponds to a directory when the file URL does
not end in a slash.  We redirect to the same URL with a slash appended to
simplify other code, which can then rely on the presence of a trailing slash
for all file URLs that correspond to a directory.

It turns out that following the redirect could result in the job being used
after free.  The code in URLRequestJob::FollowDeferredRedirect clears some
fields after calling FollowRedirect.  For all other jobs this is not a problem
since the process of following a redirect causes the job to be killed, and
URLRequestJob::Kill takes a reference to the job.  It turns out that
URLRequestFileDirJob was not calling URLRequestJob::Kill, and so this extra
reference was not being taken.

The fix is two-fold:

1-  Make URLRequestFileDirJob call URLRequestJob::Kill just like the rest of
the jobs.  This seems like a good idea for other reasons as well.

2-  Make URLRequestJob::FollowDeferredRedirect not depend on itself being alive
after the FollowRedirect call.  This just seems good for future cases where a
special URLRequestJob subclass might not call URLRequestJob::Kill for some
reason or another.

Finally, some minor changes were rqeuired to URLRequestFileDirJob to support
the call to Kill.  See changes to OnListDone.

Writing a unit test for this was a bit tricky.  It turns out that while the
URLRequestFileDirJob is waiting for the client to call FollowDeferredRedirect,
it is still traversing the directory.  It does not pause the directory listing.
The crash could only happen if the directory listing completed before the
consumer called FollowDeferredRedirect because a reference to the job was taken
on behalf of the DirectoryLister.  To test this condition, I made it possible
for a test to poll the URLRequestFileDirJob::list_completed_ flag.  Once that
is set, I then have the test call FollowDeferredRedirect.

NOTE: When running within net_unittests, NetModule::SetResourceProvider has not
been called.  So, I downgraded a NOTREACHED to a DLOG(WARNING).  NOTREACHED was
a bit excessive since it is a condition that can be reached, and I don't see
any problem with allowing unit tests to function without a resource provider.

R=wtc
BUG=18686
TEST=URLRequestTest.FileDirRedirectNoCrash

Review URL: http://codereview.chromium.org/174076

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@23944 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 1 insertions, 0 deletions

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index aa76b00..9f0f500 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -357,6 +357,7 @@ void URLRequest::ResponseStarted() {
 
 void URLRequest::FollowDeferredRedirect() {
   DCHECK(job_);
+  DCHECK(status_.is_success());
 
   job_->FollowDeferredRedirect();
 }