8 files changed, 65 insertions, 10 deletions
diff --git a/net/base/net_util.cc b/net/base/net_util.cc
index 11941a7..d679e14 100644
--- a/net/base/net_util.cc
+++ b/net/base/net_util.cc
@@ -912,10 +912,13 @@ std::string CanonicalizeHost(const std::wstring& host,
 
 std::string GetDirectoryListingHeader(const string16& title) {
   static const StringPiece header(NetModule::GetResource(IDR_DIR_HEADER_HTML));
-  if (header.empty()) {
-    NOTREACHED() << "expected resource not found";
-  }
-  std::string result(header.data(), header.size());
+  // This can be null in unit tests.
+  DLOG_IF(WARNING, header.empty()) <<
+      "Missing resource: directory listing header";
+
+  std::string result;
+  if (!header.empty())
+    result.assign(header.data(), header.size());
 
   result.append("<script>start(");
   string_escape::JsonDoubleQuote(title, true, &result);
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();
 }
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index 00ff344..bdd76e7 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -499,6 +499,10 @@ class URLRequest {
     priority_ = priority;
   }
 
+#ifdef UNIT_TEST
+  URLRequestJob* job() { return job_; }
+#endif
+
  protected:
   // Allow the URLRequestJob class to control the is_pending() flag.
   void set_is_pending(bool value) { is_pending_ = value; }
diff --git a/net/url_request/url_request_file_dir_job.cc b/net/url_request/url_request_file_dir_job.cc
index ecdf014..a933274 100644
--- a/net/url_request/url_request_file_dir_job.cc
+++ b/net/url_request/url_request_file_dir_job.cc
@@ -67,6 +67,8 @@ void URLRequestFileDirJob::Kill() {
   // OnListDone will notify the URLRequest at this time.
   if (lister_)
     lister_->Cancel();
+
+  URLRequestJob::Kill();
 }
 
 bool URLRequestFileDirJob::ReadRawData(net::IOBuffer* buf, int buf_size,
@@ -148,12 +150,12 @@ void URLRequestFileDirJob::OnListFile(
 void URLRequestFileDirJob::OnListDone(int error) {
   CloseLister();
 
-  if (error) {
+  if (canceled_) {
     read_pending_ = false;
-    NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, error));
-  } else if (canceled_) {
+    // No need for NotifyCanceled() since canceled_ is set inside Kill().
+  } else if (error) {
     read_pending_ = false;
-    NotifyCanceled();
+    NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, error));
   } else {
     list_complete_ = true;
     CompleteRead();
diff --git a/net/url_request/url_request_file_dir_job.h b/net/url_request/url_request_file_dir_job.h
index 606e3b5..52a6d61 100644
--- a/net/url_request/url_request_file_dir_job.h
+++ b/net/url_request/url_request_file_dir_job.h
@@ -32,6 +32,8 @@ class URLRequestFileDirJob
   virtual void OnListFile(const file_util::FileEnumerator::FindInfo& data);
   virtual void OnListDone(int error);
 
+  bool list_complete() const { return list_complete_; }
+
  private:
   void CloseLister();
   // When we have data and a read has been pending, this function
diff --git a/net/url_request/url_request_job.cc b/net/url_request/url_request_job.cc
index 4518d8a..b84fdd5 100644
--- a/net/url_request/url_request_job.cc
+++ b/net/url_request/url_request_job.cc
@@ -122,13 +122,21 @@ void URLRequestJob::ContinueDespiteLastError() {
 
 void URLRequestJob::FollowDeferredRedirect() {
   DCHECK(deferred_redirect_status_code_ != -1);
+
   // NOTE: deferred_redirect_url_ may be invalid, and attempting to redirect to
   // such an URL will fail inside FollowRedirect.  The DCHECK above asserts
   // that we called OnReceivedRedirect.
 
-  FollowRedirect(deferred_redirect_url_, deferred_redirect_status_code_);
+  // It is also possible that FollowRedirect will drop the last reference to
+  // this job, so we need to reset our members before calling it.
+
+  GURL redirect_url = deferred_redirect_url_;
+  int redirect_status_code = deferred_redirect_status_code_;
+
   deferred_redirect_url_ = GURL();
   deferred_redirect_status_code_ = -1;
+
+  FollowRedirect(redirect_url, redirect_status_code);
 }
 
 int64 URLRequestJob::GetByteReadCount() const {
diff --git a/net/url_request/url_request_job.h b/net/url_request/url_request_job.h
index 60554ed..9a34f4a 100644
--- a/net/url_request/url_request_job.h
+++ b/net/url_request/url_request_job.h
@@ -64,7 +64,9 @@ class URLRequestJob : public base::RefCountedThreadSafe<URLRequestJob>,
 
   // This function MUST somehow call NotifyDone/NotifyCanceled or some requests
   // will get leaked. Certain callers use that message to know when they can
-  // delete their URLRequest object, even when doing a cancel.
+  // delete their URLRequest object, even when doing a cancel. The default Kill
+  // implementation calls NotifyCanceled, so it is recommended that subclasses
+  // call URLRequestJob::Kill() after doing any additional work.
   //
   // The job should endeavor to stop working as soon as is convenient, but must
   // not send and complete notifications from inside this function. Instead,
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index d68175b..afa2c6c 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -36,6 +36,7 @@
 #include "net/proxy/proxy_service.h"
 #include "net/socket/ssl_test_util.h"
 #include "net/url_request/url_request.h"
+#include "net/url_request/url_request_file_dir_job.h"
 #include "net/url_request/url_request_test_job.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -1011,6 +1012,38 @@ TEST_F(URLRequestTest, FileDirCancelTest) {
   net::NetModule::SetResourceProvider(NULL);
 }
 
+TEST_F(URLRequestTest, FileDirRedirectNoCrash) {
+  // There is an implicit redirect when loading a file path that matches a
+  // directory and does not end with a slash.  Ensure that following such
+  // redirects does not crash.  See http://crbug.com/18686.
+
+  FilePath path;
+  PathService::Get(base::DIR_SOURCE_ROOT, &path);
+  path = path.Append(FILE_PATH_LITERAL("net"));
+  path = path.Append(FILE_PATH_LITERAL("data"));
+  path = path.Append(FILE_PATH_LITERAL("url_request_unittest"));
+
+  TestDelegate d;
+  d.set_quit_on_redirect(true);
+  TestURLRequest req(net::FilePathToFileURL(path), &d);
+  req.Start();
+  MessageLoop::current()->Run();
+
+  // Let the directory lister have time to finish its work, which will
+  // cause the URLRequestFileDirJob's ref count to drop to 1.
+  URLRequestFileDirJob* job = static_cast<URLRequestFileDirJob*>(req.job());
+  while (!job->list_complete()) {
+    PlatformThread::Sleep(10);
+    MessageLoop::current()->RunAllPending();
+  }
+
+  // Should not crash during this call!
+  req.FollowDeferredRedirect();
+
+  // Flush event queue.
+  MessageLoop::current()->RunAllPending();
+}
+
 TEST_F(URLRequestTestHTTP, RestrictRedirects) {
   ASSERT_TRUE(NULL != server_.get());