net: fix callbacks in DiskCacheBasedSSLHostInfo

Previously, DiskCacheBasedSSLHostInfo had a couple of problems:
 * I had assumed that the disk cache was taking references to the
   callback. It wasn't.
 * Even if it were, I was passing pointers into
   DiskCacheBasedSSLHostInfo which needed to be live for the duration
   of the callback.

This change switches to a custom callback class which doesn't need
reference counting and which contains pointer members that remain live
for the duration of the callback.

BUG=63867
TEST=Navigate to an HTTPS page with Snap Start running and no network connection.

http://codereview.chromium.org/5826001/

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

2 files changed, 79 insertions, 28 deletions

diff --git a/net/http/disk_cache_based_ssl_host_info.cc b/net/http/disk_cache_based_ssl_host_info.cc
index cd1cac8..2b83f56 100644
--- a/net/http/disk_cache_based_ssl_host_info.cc
+++ b/net/http/disk_cache_based_ssl_host_info.cc
@@ -17,9 +17,9 @@ DiskCacheBasedSSLHostInfo::DiskCacheBasedSSLHostInfo(
     const SSLConfig& ssl_config,
     HttpCache* http_cache)
     : SSLHostInfo(hostname, ssl_config),
-      callback_(new CancelableCompletionCallback<DiskCacheBasedSSLHostInfo>(
-                        ALLOW_THIS_IN_INITIALIZER_LIST(this),
-                        &DiskCacheBasedSSLHostInfo::DoLoop)),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      callback_(new CallbackImpl(weak_ptr_factory_.GetWeakPtr(),
+                                 &DiskCacheBasedSSLHostInfo::DoLoop)),
       state_(GET_BACKEND),
       ready_(false),
       hostname_(hostname),
@@ -39,7 +39,8 @@ DiskCacheBasedSSLHostInfo::~DiskCacheBasedSSLHostInfo() {
   DCHECK(!user_callback_);
   if (entry_)
     entry_->Close();
-  callback_->Cancel();
+  if (!IsCallbackPending())
+    delete callback_;
 }
 
 std::string DiskCacheBasedSSLHostInfo::key() const {
@@ -92,13 +93,27 @@ void DiskCacheBasedSSLHostInfo::DoLoop(int rv) {
   } while (rv != ERR_IO_PENDING && state_ != NONE);
 }
 
+bool DiskCacheBasedSSLHostInfo::IsCallbackPending() const {
+  switch (state_) {
+    case GET_BACKEND_COMPLETE:
+    case OPEN_COMPLETE:
+    case READ_COMPLETE:
+    case CREATE_COMPLETE:
+    case WRITE_COMPLETE:
+      return true;
+    default:
+      return false;
+  }
+}
+
 int DiskCacheBasedSSLHostInfo::DoGetBackend() {
   state_ = GET_BACKEND_COMPLETE;
-  return http_cache_->GetBackend(&backend_, callback_.get());
+  return http_cache_->GetBackend(callback_->backend_pointer(), callback_);
 }
 
 int DiskCacheBasedSSLHostInfo::DoGetBackendComplete(int rv) {
   if (rv == OK) {
+    backend_ = callback_->backend();
     state_ = OPEN;
   } else {
     state_ = WAIT_FOR_DATA_READY_DONE;
@@ -108,11 +123,12 @@ int DiskCacheBasedSSLHostInfo::DoGetBackendComplete(int rv) {
 
 int DiskCacheBasedSSLHostInfo::DoOpen() {
   state_ = OPEN_COMPLETE;
-  return backend_->OpenEntry(key(), &entry_, callback_.get());
+  return backend_->OpenEntry(key(), callback_->entry_pointer(), callback_);
 }
 
 int DiskCacheBasedSSLHostInfo::DoOpenComplete(int rv) {
   if (rv == OK) {
+    entry_ = callback_->entry();
     state_ = READ;
   } else {
     state_ = WAIT_FOR_DATA_READY_DONE;
@@ -131,7 +147,7 @@ int DiskCacheBasedSSLHostInfo::DoRead() {
   read_buffer_ = new IOBuffer(size);
   state_ = READ_COMPLETE;
   return entry_->ReadData(0 /* index */, 0 /* offset */, read_buffer_,
-                          size, callback_.get());
+                          size, callback_);
 }
 
 int DiskCacheBasedSSLHostInfo::DoReadComplete(int rv) {
@@ -195,13 +211,14 @@ void DiskCacheBasedSSLHostInfo::Persist() {
 int DiskCacheBasedSSLHostInfo::DoCreate() {
   DCHECK(entry_ == NULL);
   state_ = CREATE_COMPLETE;
-  return backend_->CreateEntry(key(), &entry_, callback_.get());
+  return backend_->CreateEntry(key(), callback_->entry_pointer(), callback_);
 }
 
 int DiskCacheBasedSSLHostInfo::DoCreateComplete(int rv) {
   if (rv != OK) {
     state_ = SET_DONE;
   } else {
+    entry_ = callback_->entry();
     state_ = WRITE;
   }
   return OK;
@@ -211,9 +228,9 @@ int DiskCacheBasedSSLHostInfo::DoWrite() {
   write_buffer_ = new IOBuffer(new_data_.size());
   memcpy(write_buffer_->data(), new_data_.data(), new_data_.size());
   state_ = WRITE_COMPLETE;
+
   return entry_->WriteData(0 /* index */, 0 /* offset */, write_buffer_,
-                           new_data_.size(), callback_.get(),
-                           true /* truncate */);
+                           new_data_.size(), callback_, true /* truncate */);
 }
 
 int DiskCacheBasedSSLHostInfo::DoWriteComplete(int rv) {
diff --git a/net/http/disk_cache_based_ssl_host_info.h b/net/http/disk_cache_based_ssl_host_info.h
index 1d53b90..430931a 100644
--- a/net/http/disk_cache_based_ssl_host_info.h
+++ b/net/http/disk_cache_based_ssl_host_info.h
@@ -10,6 +10,7 @@
 #include "base/lock.h"
 #include "base/non_thread_safe.h"
 #include "base/scoped_ptr.h"
+#include "base/weak_ptr.h"
 #include "net/base/completion_callback.h"
 #include "net/disk_cache/disk_cache.h"
 #include "net/socket/ssl_host_info.h"
@@ -36,7 +37,53 @@ class DiskCacheBasedSSLHostInfo : public SSLHostInfo,
   virtual void Persist();
 
  private:
+  enum State {
+    GET_BACKEND,
+    GET_BACKEND_COMPLETE,
+    OPEN,
+    OPEN_COMPLETE,
+    READ,
+    READ_COMPLETE,
+    WAIT_FOR_DATA_READY_DONE,
+    CREATE,
+    CREATE_COMPLETE,
+    WRITE,
+    WRITE_COMPLETE,
+    SET_DONE,
+    NONE,
+  };
+
   ~DiskCacheBasedSSLHostInfo();
+
+  class CallbackImpl : public CallbackRunner<Tuple1<int> > {
+   public:
+    CallbackImpl(const base::WeakPtr<DiskCacheBasedSSLHostInfo>& obj,
+                 void (DiskCacheBasedSSLHostInfo::*meth) (int))
+        : obj_(obj),
+          meth_(meth) {
+    }
+
+    virtual void RunWithParams(const Tuple1<int>& params) {
+      if (!obj_) {
+        delete this;
+      } else {
+        DispatchToMethod(obj_.get(), meth_, params);
+      }
+    }
+
+    disk_cache::Backend** backend_pointer() { return &backend_; }
+    disk_cache::Entry** entry_pointer() { return &entry_; }
+    disk_cache::Backend* backend() const { return backend_; }
+    disk_cache::Entry* entry() const { return entry_; }
+
+   protected:
+    base::WeakPtr<DiskCacheBasedSSLHostInfo> obj_;
+    void (DiskCacheBasedSSLHostInfo::*meth_) (int);
+
+    disk_cache::Backend* backend_;
+    disk_cache::Entry* entry_;
+  };
+
   std::string key() const;
 
   void DoLoop(int rv);
@@ -58,31 +105,18 @@ class DiskCacheBasedSSLHostInfo : public SSLHostInfo,
   // SetDone is the terminal state of the write operation.
   int SetDone();
 
-  enum State {
-    GET_BACKEND,
-    GET_BACKEND_COMPLETE,
-    OPEN,
-    OPEN_COMPLETE,
-    READ,
-    READ_COMPLETE,
-    WAIT_FOR_DATA_READY_DONE,
-    CREATE,
-    CREATE_COMPLETE,
-    WRITE,
-    WRITE_COMPLETE,
-    SET_DONE,
-    NONE,
-  };
+  // IsCallbackPending returns true if we have a pending callback.
+  bool IsCallbackPending() const;
 
-  scoped_refptr<CancelableCompletionCallback<DiskCacheBasedSSLHostInfo> >
-      callback_;
+  base::WeakPtrFactory<DiskCacheBasedSSLHostInfo> weak_ptr_factory_;
+  CallbackImpl* callback_;
   State state_;
   bool ready_;
   std::string new_data_;
   const std::string hostname_;
   HttpCache* const http_cache_;
   disk_cache::Backend* backend_;
-  disk_cache::Entry *entry_;
+  disk_cache::Entry* entry_;
   CompletionCallback* user_callback_;
   scoped_refptr<net::IOBuffer> read_buffer_;
   scoped_refptr<net::IOBuffer> write_buffer_;