Fix webkit URLRequestJob subtypes to handle Kill() correctly.

Kill() should prevent calling back into the delegate.  So we cancel pending callbacks.

BUG=63692
TEST=existing

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

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

4 files changed, 37 insertions, 21 deletions

diff --git a/webkit/blob/blob_url_request_job.cc b/webkit/blob/blob_url_request_job.cc
index 6b62605..bbe26be 100644
--- a/webkit/blob/blob_url_request_job.cc
+++ b/webkit/blob/blob_url_request_job.cc
@@ -4,6 +4,7 @@
 
 #include "webkit/blob/blob_url_request_job.h"
 
+#include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/file_util_proxy.h"
@@ -30,14 +31,14 @@ static const int kHTTPMethodNotAllow = 405;
 static const int kHTTPRequestedRangeNotSatisfiable = 416;
 static const int kHTTPInternalError = 500;
 
-static const char* kHTTPOKText = "OK";
-static const char* kHTTPPartialContentText = "Partial Content";
-static const char* kHTTPNotAllowedText = "Not Allowed";
-static const char* kHTTPNotFoundText = "Not Found";
-static const char* kHTTPMethodNotAllowText = "Method Not Allowed";
-static const char* kHTTPRequestedRangeNotSatisfiableText =
+static const char kHTTPOKText[] = "OK";
+static const char kHTTPPartialContentText[] = "Partial Content";
+static const char kHTTPNotAllowedText[] = "Not Allowed";
+static const char kHTTPNotFoundText[] = "Not Found";
+static const char kHTTPMethodNotAllowText[] = "Method Not Allowed";
+static const char kHTTPRequestedRangeNotSatisfiableText[] =
     "Requested Range Not Satisfiable";
-static const char* kHTTPInternalErrorText = "Internal Server Error";
+static const char kHTTPInternalErrorText[] = "Internal Server Error";
 
 BlobURLRequestJob::BlobURLRequestJob(
     net::URLRequest* request,
@@ -58,7 +59,8 @@ BlobURLRequestJob::BlobURLRequestJob(
       read_buf_remaining_bytes_(0),
       error_(false),
       headers_set_(false),
-      byte_range_set_(false) {
+      byte_range_set_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
 }
 
 BlobURLRequestJob::~BlobURLRequestJob() {
@@ -66,8 +68,9 @@ BlobURLRequestJob::~BlobURLRequestJob() {
 
 void BlobURLRequestJob::Start() {
   // Continue asynchronously.
-  MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &BlobURLRequestJob::DidStart));
+  MessageLoop::current()->PostTask(
+      FROM_HERE,
+      method_factory_.NewRunnableMethod(&BlobURLRequestJob::DidStart));
 }
 
 void BlobURLRequestJob::DidStart() {
@@ -90,6 +93,8 @@ void BlobURLRequestJob::Kill() {
   stream_.Close();
 
   URLRequestJob::Kill();
+  callback_factory_.RevokeAll();
+  method_factory_.RevokeAll();
 }
 
 void BlobURLRequestJob::ResolveFile(const FilePath& file_path) {
@@ -109,18 +114,16 @@ void BlobURLRequestJob::ResolveFile(const FilePath& file_path) {
   bool exists = file_util::GetFileInfo(file_path, &file_info);
 
   // Continue asynchronously.
-  MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &BlobURLRequestJob::DidResolve,
-      (exists ? base::PLATFORM_FILE_OK : base::PLATFORM_FILE_ERROR_NOT_FOUND),
-      file_info));
+  MessageLoop::current()->PostTask(
+      FROM_HERE,
+      method_factory_.NewRunnableMethod(
+          &BlobURLRequestJob::DidResolve,
+          exists ? base::PLATFORM_FILE_OK : base::PLATFORM_FILE_ERROR_NOT_FOUND,
+          file_info));
 }
 
 void BlobURLRequestJob::DidResolve(base::PlatformFileError rv,
                                    const base::PlatformFileInfo& file_info) {
-  // We may have been orphaned...
-  if (!request_)
-    return;
-
   // If an error occured, bail out.
   if (rv == base::PLATFORM_FILE_ERROR_NOT_FOUND) {
     NotifyFailure(net::ERR_FILE_NOT_FOUND);
diff --git a/webkit/blob/blob_url_request_job.h b/webkit/blob/blob_url_request_job.h
index 53414cd..c82ebfc 100644
--- a/webkit/blob/blob_url_request_job.h
+++ b/webkit/blob/blob_url_request_job.h
@@ -9,6 +9,7 @@
 #include "base/ref_counted.h"
 #include "base/scoped_callback_factory.h"
 #include "base/scoped_ptr.h"
+#include "base/task.h"
 #include "net/base/completion_callback.h"
 #include "net/base/file_stream.h"
 #include "net/http/http_byte_range.h"
@@ -78,6 +79,7 @@ class BlobURLRequestJob : public net::URLRequestJob {
   bool byte_range_set_;
   net::HttpByteRange byte_range_;
   scoped_ptr<net::HttpResponseInfo> response_info_;
+  ScopedRunnableMethodFactory<BlobURLRequestJob> method_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BlobURLRequestJob);
 };
diff --git a/webkit/blob/view_blob_internals_job.cc b/webkit/blob/view_blob_internals_job.cc
index 6d6955b..49798b4 100644
--- a/webkit/blob/view_blob_internals_job.cc
+++ b/webkit/blob/view_blob_internals_job.cc
@@ -4,6 +4,7 @@
 
 #include "webkit/blob/view_blob_internals_job.h"
 
+#include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/format_macros.h"
 #include "base/i18n/number_formatting.h"
@@ -102,15 +103,17 @@ namespace webkit_blob {
 ViewBlobInternalsJob::ViewBlobInternalsJob(
     net::URLRequest* request, BlobStorageController* blob_storage_controller)
     : URLRequestSimpleJob(request),
-      blob_storage_controller_(blob_storage_controller) {
+      blob_storage_controller_(blob_storage_controller),
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
 }
 
 ViewBlobInternalsJob::~ViewBlobInternalsJob() {
 }
 
 void ViewBlobInternalsJob::Start() {
-  MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
-      this, &ViewBlobInternalsJob::DoWorkAsync));
+  MessageLoop::current()->PostTask(
+      FROM_HERE,
+      method_factory_.NewRunnableMethod(&ViewBlobInternalsJob::DoWorkAsync));
 }
 
 bool ViewBlobInternalsJob::IsRedirectResponse(GURL* location,
@@ -126,6 +129,11 @@ bool ViewBlobInternalsJob::IsRedirectResponse(GURL* location,
   return false;
 }
 
+void ViewBlobInternalsJob::Kill() {
+  URLRequestSimpleJob::Kill();
+  method_factory_.RevokeAll();
+}
+
 void ViewBlobInternalsJob::DoWorkAsync() {
   if (request_->url().has_query() &&
       StartsWithASCII(request_->url().query(), "remove=", true)) {
diff --git a/webkit/blob/view_blob_internals_job.h b/webkit/blob/view_blob_internals_job.h
index 913531c..2f557ae 100644
--- a/webkit/blob/view_blob_internals_job.h
+++ b/webkit/blob/view_blob_internals_job.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/task.h"
 #include "net/url_request/url_request_simple_job.h"
 
 namespace net {
@@ -30,6 +31,7 @@ class ViewBlobInternalsJob : public URLRequestSimpleJob {
                        std::string* charset,
                        std::string* data) const;
   virtual bool IsRedirectResponse(GURL* location, int* http_status_code);
+  virtual void Kill();
 
  private:
   ~ViewBlobInternalsJob();
@@ -40,6 +42,7 @@ class ViewBlobInternalsJob : public URLRequestSimpleJob {
                                       std::string* out);
 
   BlobStorageController* blob_storage_controller_;
+  ScopedRunnableMethodFactory<ViewBlobInternalsJob> method_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ViewBlobInternalsJob);
 };