Skip to content

Commit

Permalink
DiskBasedCertCache method name change + readability fixes.
Browse files Browse the repository at this point in the history 
Review URL: https://codereview.chromium.org/432053002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287464 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
brandonsalmon@chromium.org committed Aug 5, 2014
1 parent 4a7ce7d commit 6d3bb89
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 115 deletions.
79 changes: 39 additions & 40 deletions net/http/disk_based_cert_cache.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ enum CacheResult {
DISK_CACHE_HIT,
DISK_CACHE_ENTRY_CORRUPT,
DISK_CACHE_ERROR,
CACHE_MISS,
CACHE_RESULT_MAX
};

Expand All @@ -50,9 +49,9 @@ void RecordCacheResult(CacheResult result) {

} // namespace

// WriteWorkers represent pending Set jobs in the DiskBasedCertCache. Each
// certificate requested to be cached is assigned a Writeworker on a one-to-one
// basis. The same certificate should not have multiple WriteWorkers at the same
// WriteWorkers represent pending SetCertificate jobs in the DiskBasedCertCache.
// Each certificate requested to be stored is assigned a WriteWorker.
// The same certificate should not have multiple WriteWorkers at the same
// time; instead, add a user callback to the existing WriteWorker.
class DiskBasedCertCache::WriteWorker {
public:
Expand Down Expand Up @@ -112,7 +111,7 @@ class DiskBasedCertCache::WriteWorker {
bool canceled_;

disk_cache::Entry* entry_;
State state_;
State next_state_;
scoped_refptr<IOBuffer> buffer_;
int io_buf_len_;

Expand All @@ -131,7 +130,7 @@ DiskBasedCertCache::WriteWorker::WriteWorker(
key_(key),
canceled_(false),
entry_(NULL),
state_(STATE_NONE),
next_state_(STATE_NONE),
io_buf_len_(0),
cleanup_callback_(cleanup_callback),
io_callback_(
Expand All @@ -146,8 +145,8 @@ DiskBasedCertCache::WriteWorker::~WriteWorker() {
}

void DiskBasedCertCache::WriteWorker::Start() {
DCHECK_EQ(STATE_NONE, state_);
state_ = STATE_CREATE;
DCHECK_EQ(STATE_NONE, next_state_);
next_state_ = STATE_CREATE;
int rv = DoLoop(OK);

if (rv == ERR_IO_PENDING)
Expand Down Expand Up @@ -181,9 +180,9 @@ void DiskBasedCertCache::WriteWorker::OnIOComplete(int rv) {

int DiskBasedCertCache::WriteWorker::DoLoop(int rv) {
do {
State next_state = state_;
state_ = STATE_NONE;
switch (next_state) {
State state = next_state_;
next_state_ = STATE_NONE;
switch (state) {
case STATE_CREATE:
rv = DoCreate();
break;
Expand All @@ -206,13 +205,13 @@ int DiskBasedCertCache::WriteWorker::DoLoop(int rv) {
NOTREACHED();
break;
}
} while (rv != ERR_IO_PENDING && state_ != STATE_NONE);
} while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);

return rv;
}

int DiskBasedCertCache::WriteWorker::DoCreate() {
state_ = STATE_CREATE_COMPLETE;
next_state_ = STATE_CREATE_COMPLETE;

return backend_->CreateEntry(key_, &entry_, io_callback_);
}
Expand All @@ -222,24 +221,24 @@ int DiskBasedCertCache::WriteWorker::DoCreateComplete(int rv) {
// If this occurs, it is necessary to instead open the previously
// existing entry.
if (rv < 0) {
state_ = STATE_OPEN;
next_state_ = STATE_OPEN;
return OK;
}

state_ = STATE_WRITE;
next_state_ = STATE_WRITE;
return OK;
}

int DiskBasedCertCache::WriteWorker::DoOpen() {
state_ = STATE_OPEN_COMPLETE;
next_state_ = STATE_OPEN_COMPLETE;
return backend_->OpenEntry(key_, &entry_, io_callback_);
}

int DiskBasedCertCache::WriteWorker::DoOpenComplete(int rv) {
if (rv < 0)
return rv;

state_ = STATE_WRITE;
next_state_ = STATE_WRITE;
return OK;
}

Expand All @@ -254,7 +253,7 @@ int DiskBasedCertCache::WriteWorker::DoWrite() {
io_buf_len_ = write_data.size();
memcpy(buffer_->data(), write_data.data(), io_buf_len_);

state_ = STATE_WRITE_COMPLETE;
next_state_ = STATE_WRITE_COMPLETE;

return entry_->WriteData(0 /* index */,
0 /* offset */,
Expand Down Expand Up @@ -291,11 +290,11 @@ void DiskBasedCertCache::WriteWorker::RunCallbacks(int rv) {
user_callbacks_.clear();
}

// ReadWorkers represent pending Get jobs in the DiskBasedCertCache. Each
// certificate requested to be retrieved from the cache is assigned a ReadWorker
// on a one-to-one basis. The same |key| should not have multiple ReadWorkers
// at the same time; instead, call AddCallback to add a user_callback_ to
// the the existing ReadWorker.
// ReadWorkers represent pending GetCertificate jobs in the DiskBasedCertCache.
// Each certificate requested to be retrieved from the cache is assigned a
// ReadWorker. The same |key| should not have multiple ReadWorkers at the
// same time; instead, call AddCallback to add a user callback to the
// existing ReadWorker.
class DiskBasedCertCache::ReadWorker {
public:
// |backend| is the backend to read |certificate| from, using
Expand Down Expand Up @@ -348,7 +347,7 @@ class DiskBasedCertCache::ReadWorker {

disk_cache::Entry* entry_;

State state_;
State next_state_;
scoped_refptr<IOBuffer> buffer_;
int io_buf_len_;

Expand All @@ -365,7 +364,7 @@ DiskBasedCertCache::ReadWorker::ReadWorker(disk_cache::Backend* backend,
key_(key),
canceled_(false),
entry_(NULL),
state_(STATE_NONE),
next_state_(STATE_NONE),
io_buf_len_(0),
cleanup_callback_(cleanup_callback),
io_callback_(
Expand All @@ -380,8 +379,8 @@ DiskBasedCertCache::ReadWorker::~ReadWorker() {
}

void DiskBasedCertCache::ReadWorker::Start() {
DCHECK_EQ(STATE_NONE, state_);
state_ = STATE_OPEN;
DCHECK_EQ(STATE_NONE, next_state_);
next_state_ = STATE_OPEN;
int rv = DoLoop(OK);

if (rv == ERR_IO_PENDING)
Expand Down Expand Up @@ -415,9 +414,9 @@ void DiskBasedCertCache::ReadWorker::OnIOComplete(int rv) {

int DiskBasedCertCache::ReadWorker::DoLoop(int rv) {
do {
State next_state = state_;
state_ = STATE_NONE;
switch (next_state) {
State state = next_state_;
next_state_ = STATE_NONE;
switch (state) {
case STATE_OPEN:
rv = DoOpen();
break;
Expand All @@ -434,30 +433,28 @@ int DiskBasedCertCache::ReadWorker::DoLoop(int rv) {
NOTREACHED();
break;
}
} while (rv != ERR_IO_PENDING && state_ != STATE_NONE);
} while (rv != ERR_IO_PENDING && next_state_ != STATE_NONE);

return rv;
}

int DiskBasedCertCache::ReadWorker::DoOpen() {
state_ = STATE_OPEN_COMPLETE;
next_state_ = STATE_OPEN_COMPLETE;
return backend_->OpenEntry(key_, &entry_, io_callback_);
}

int DiskBasedCertCache::ReadWorker::DoOpenComplete(int rv) {
if (rv < 0) {
// Errors other than ERR_CACHE_MISS are not recorded as either a hit
// or a miss.
RecordCacheResult(rv == ERR_CACHE_MISS ? CACHE_MISS : DISK_CACHE_ERROR);
RecordCacheResult(DISK_CACHE_ERROR);
return rv;
}

state_ = STATE_READ;
next_state_ = STATE_READ;
return OK;
}

int DiskBasedCertCache::ReadWorker::DoRead() {
state_ = STATE_READ_COMPLETE;
next_state_ = STATE_READ_COMPLETE;
io_buf_len_ = entry_->GetDataSize(0 /* index */);
buffer_ = new IOBuffer(io_buf_len_);
return entry_->ReadData(
Expand Down Expand Up @@ -525,7 +522,8 @@ DiskBasedCertCache::~DiskBasedCertCache() {
}
}

void DiskBasedCertCache::Get(const std::string& key, const GetCallback& cb) {
void DiskBasedCertCache::GetCertificate(const std::string& key,
const GetCallback& cb) {
DCHECK(!key.empty());

// If the handle is already in the MRU cache, just return that (via callback).
Expand Down Expand Up @@ -557,8 +555,9 @@ void DiskBasedCertCache::Get(const std::string& key, const GetCallback& cb) {
}
}

void DiskBasedCertCache::Set(const X509Certificate::OSCertHandle cert_handle,
const SetCallback& cb) {
void DiskBasedCertCache::SetCertificate(
const X509Certificate::OSCertHandle cert_handle,
const SetCallback& cb) {
DCHECK(!cb.is_null());
DCHECK(cert_handle);
std::string key = GetCacheKeyForCert(cert_handle);
Expand Down
26 changes: 14 additions & 12 deletions net/http/disk_based_cert_cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class NET_EXPORT_PRIVATE DiskBasedCertCache {
GetCallback;
typedef base::Callback<void(const std::string&)> SetCallback;

// Initializes a new DiskBasedCertCache that will use |backend|, which has
// previously been initialized, to store the certificate in the cache.
// Initializes a new DiskBasedCertCache that will access the disk cache via
// |backend|.
explicit DiskBasedCertCache(disk_cache::Backend* backend);
~DiskBasedCertCache();

Expand All @@ -39,20 +39,22 @@ class NET_EXPORT_PRIVATE DiskBasedCertCache {
// Otherwise, |cb| will be called with NULL. Callers that wish to store
// a reference to the certificate need to use X509Certificate::DupOSCertHandle
// inside |cb|.
void Get(const std::string& key, const GetCallback& cb);
void GetCertificate(const std::string& key, const GetCallback& cb);

// Stores |cert_handle| in the cache. If |cert_handle| is successfully stored,
// |cb| will be called with the key. If |cb| is called with an empty
// string, then |cert_handle| was not stored.
void Set(const X509Certificate::OSCertHandle cert_handle,
const SetCallback& cb);
void SetCertificate(const X509Certificate::OSCertHandle cert_handle,
const SetCallback& cb);

// Returns the number of in-memory MRU cache hits that have occurred
// on Set and Get operations. Intended for test purposes only.
// on SetCertificate and GetCertificate operations. Intended for test purposes
// only.
size_t mem_cache_hits_for_testing() const { return mem_cache_hits_; }

// Returns the number of in-memory MRU cache misses that have occurred
// on Set and Get operations. Intended for test purposes only.
// on SetCertificate and GetCertificate operations. Intended for test purposes
// only.
size_t mem_cache_misses_for_testing() const { return mem_cache_misses_; }

private:
Expand All @@ -64,8 +66,8 @@ class NET_EXPORT_PRIVATE DiskBasedCertCache {
void operator()(X509Certificate::OSCertHandle cert_handle);
};

// An in-memory cache that is used to prevent redundant reads and writes
// to and from the disk cache.
// An in-memory cache that is used to prevent redundantly reading
// from disk.
typedef base::MRUCacheBase<std::string,
X509Certificate::OSCertHandle,
CertFree> MRUCertCache;
Expand All @@ -75,9 +77,9 @@ class NET_EXPORT_PRIVATE DiskBasedCertCache {
typedef base::hash_map<std::string, ReadWorker*> ReadWorkerMap;
typedef base::hash_map<std::string, WriteWorker*> WriteWorkerMap;

// FinishedReadOperation and FinishedWriteOperation are used by callbacks
// given to the workers to signal the DiskBasedCertCache they have completed
// their work.
// FinishedReadOperation and FinishedWriteOperation are used to remove
// workers from their respective worker maps, and perform other necessary
// cleanup. They are called from the workers via callback.
void FinishedReadOperation(const std::string& key,
X509Certificate::OSCertHandle cert_handle);
void FinishedWriteOperation(const std::string& key,
Expand Down
Loading

0 comments on commit 6d3bb89

Please sign in to comment.