From f16532a7c0277e1876d4b8c75992faeef7e5867e Mon Sep 17 00:00:00 2001
From: "agl@chromium.org"
<agl@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed, 15 Dec 2010 20:07:39 +0000
Subject: [PATCH] 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
---
net/http/disk_cache_based_ssl_host_info.cc | 37 ++++++++----
net/http/disk_cache_based_ssl_host_info.h | 70 ++++++++++++++++------
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 cd1cac8ca0747..2b83f5618c074 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 1d53b909dffbc..430931aec70a1 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_;
--
GitLab