Commit bb2bb40a authored by jkarlin's avatar jkarlin Committed by Commit bot

This is a reland of ServiceWorkerCache::Keys.

Reland of https://codereview.chromium.org/477973002/

There was a memory leak.  EndEnumeration wasn't called on the iterator.

Fixed and tested with ASAN.

The leak was detected because SimpleCache doesn't clean up its enumerators on deletion, but it should.  See https://code.google.com/p/chromium/issues/detail?id=410276

BUG=392621

Review URL: https://codereview.chromium.org/528233003

Cr-Commit-Position: refs/heads/master@{#293326}
parent 52e5986b
......@@ -27,6 +27,9 @@ typedef scoped_ptr<disk_cache::Backend> ScopedBackendPtr;
typedef base::Callback<void(bool)> BoolCallback;
typedef base::Callback<void(disk_cache::ScopedEntryPtr, bool)>
EntryBoolCallback;
typedef base::Callback<void(scoped_ptr<ServiceWorkerRequestResponseHeaders>)>
HeadersCallback;
enum EntryIndex { INDEX_HEADERS = 0, INDEX_RESPONSE_BODY };
// The maximum size of an individual cache. Ultimately cache size is controlled
......@@ -189,8 +192,7 @@ void MatchDidReadHeaderData(
const ServiceWorkerCache::ResponseCallback& callback,
base::WeakPtr<storage::BlobStorageContext> blob_storage,
disk_cache::ScopedEntryPtr entry,
const scoped_refptr<net::IOBufferWithSize>& buffer,
int rv);
scoped_ptr<ServiceWorkerRequestResponseHeaders> headers);
void MatchDidReadResponseBodyData(
ServiceWorkerFetchRequest* request,
const ServiceWorkerCache::ResponseCallback& callback,
......@@ -211,6 +213,15 @@ void DeleteDidOpenEntry(ServiceWorkerFetchRequest* request,
scoped_ptr<disk_cache::Entry*> entryptr,
int rv);
// Copy headers out of a cache entry and into a protobuf. The callback is
// guaranteed to be run.
void ReadHeaders(disk_cache::Entry* entry, const HeadersCallback& callback);
void ReadHeadersDidReadHeaderData(
disk_cache::Entry* entry,
const HeadersCallback& callback,
const scoped_refptr<net::IOBufferWithSize>& buffer,
int rv);
// CreateBackend callbacks
void CreateBackendDidCreate(const ServiceWorkerCache::ErrorCallback& callback,
scoped_ptr<ScopedBackendPtr> backend_ptr,
......@@ -350,25 +361,16 @@ void MatchDidOpenEntry(ServiceWorkerFetchRequest* request,
DCHECK(entryptr);
disk_cache::ScopedEntryPtr entry(*entryptr);
scoped_refptr<net::IOBufferWithSize> buffer(
new net::IOBufferWithSize(entry->GetDataSize(INDEX_HEADERS)));
// Copy the entry pointer before passing it in base::Bind.
disk_cache::Entry* tmp_entry_ptr = entry.get();
net::CompletionCallback read_header_callback =
base::Bind(MatchDidReadHeaderData,
HeadersCallback headers_callback = base::Bind(MatchDidReadHeaderData,
request,
callback,
blob_storage,
base::Passed(entry.Pass()),
buffer);
int read_rv = tmp_entry_ptr->ReadData(
INDEX_HEADERS, 0, buffer.get(), buffer->size(), read_header_callback);
base::Passed(entry.Pass()));
if (read_rv != net::ERR_IO_PENDING)
read_header_callback.Run(read_rv);
ReadHeaders(tmp_entry_ptr, headers_callback);
}
void MatchDidReadHeaderData(
......@@ -376,36 +378,24 @@ void MatchDidReadHeaderData(
const ServiceWorkerCache::ResponseCallback& callback,
base::WeakPtr<storage::BlobStorageContext> blob_storage,
disk_cache::ScopedEntryPtr entry,
const scoped_refptr<net::IOBufferWithSize>& buffer,
int rv) {
if (rv != buffer->size()) {
callback.Run(ServiceWorkerCache::ErrorTypeStorage,
scoped_ptr<ServiceWorkerResponse>(),
scoped_ptr<storage::BlobDataHandle>());
return;
}
ServiceWorkerRequestResponseHeaders headers;
if (!headers.ParseFromArray(buffer->data(), buffer->size())) {
scoped_ptr<ServiceWorkerRequestResponseHeaders> headers) {
if (!headers) {
callback.Run(ServiceWorkerCache::ErrorTypeStorage,
scoped_ptr<ServiceWorkerResponse>(),
scoped_ptr<storage::BlobDataHandle>());
return;
}
scoped_ptr<ServiceWorkerResponse> response(
new ServiceWorkerResponse(request->url,
headers.status_code(),
headers.status_text(),
headers->status_code(),
headers->status_text(),
std::map<std::string, std::string>(),
""));
for (int i = 0; i < headers.response_headers_size(); ++i) {
for (int i = 0; i < headers->response_headers_size(); ++i) {
const ServiceWorkerRequestResponseHeaders::HeaderMap header =
headers.response_headers(i);
headers->response_headers(i);
response->headers.insert(std::make_pair(header.name(), header.value()));
}
......@@ -423,7 +413,6 @@ void MatchDidReadHeaderData(
callback.Run(ServiceWorkerCache::ErrorTypeStorage,
scoped_ptr<ServiceWorkerResponse>(),
scoped_ptr<storage::BlobDataHandle>());
return;
}
......@@ -549,6 +538,43 @@ void DeleteDidOpenEntry(ServiceWorkerFetchRequest* request,
callback.Run(ServiceWorkerCache::ErrorTypeOK);
}
void ReadHeaders(disk_cache::Entry* entry, const HeadersCallback& callback) {
DCHECK(entry);
scoped_refptr<net::IOBufferWithSize> buffer(
new net::IOBufferWithSize(entry->GetDataSize(INDEX_HEADERS)));
net::CompletionCallback read_header_callback =
base::Bind(ReadHeadersDidReadHeaderData, entry, callback, buffer);
int read_rv = entry->ReadData(
INDEX_HEADERS, 0, buffer.get(), buffer->size(), read_header_callback);
if (read_rv != net::ERR_IO_PENDING)
read_header_callback.Run(read_rv);
}
void ReadHeadersDidReadHeaderData(
disk_cache::Entry* entry,
const HeadersCallback& callback,
const scoped_refptr<net::IOBufferWithSize>& buffer,
int rv) {
if (rv != buffer->size()) {
callback.Run(scoped_ptr<ServiceWorkerRequestResponseHeaders>());
return;
}
scoped_ptr<ServiceWorkerRequestResponseHeaders> headers(
new ServiceWorkerRequestResponseHeaders());
if (!headers->ParseFromArray(buffer->data(), buffer->size())) {
callback.Run(scoped_ptr<ServiceWorkerRequestResponseHeaders>());
return;
}
callback.Run(headers.Pass());
}
void CreateBackendDidCreate(const ServiceWorkerCache::ErrorCallback& callback,
scoped_ptr<ScopedBackendPtr> backend_ptr,
base::WeakPtr<ServiceWorkerCache> cache,
......@@ -557,12 +583,49 @@ void CreateBackendDidCreate(const ServiceWorkerCache::ErrorCallback& callback,
callback.Run(ServiceWorkerCache::ErrorTypeStorage);
return;
}
cache->set_backend(backend_ptr->Pass());
callback.Run(ServiceWorkerCache::ErrorTypeOK);
}
} // namespace
// The state needed to pass between ServiceWorkerCache::Keys callbacks.
struct ServiceWorkerCache::KeysContext {
KeysContext(const ServiceWorkerCache::RequestsCallback& callback,
base::WeakPtr<ServiceWorkerCache> cache)
: original_callback(callback),
cache(cache),
out_keys(new ServiceWorkerCache::Requests()),
backend_iterator(NULL),
enumerated_entry(NULL) {}
~KeysContext() {
for (size_t i = 0, max = entries.size(); i < max; ++i)
entries[i]->Close();
if (enumerated_entry)
enumerated_entry->Close();
if (cache && backend_iterator)
cache->backend_->EndEnumeration(&backend_iterator);
}
// The callback passed to the Keys() function.
ServiceWorkerCache::RequestsCallback original_callback;
// The ServiceWorkerCache that Keys was called on.
base::WeakPtr<ServiceWorkerCache> cache;
// The vector of open entries in the backend.
Entries entries;
// The output of the Keys function.
scoped_ptr<ServiceWorkerCache::Requests> out_keys;
// Used for enumerating cache entries.
void* backend_iterator;
disk_cache::Entry* enumerated_entry;
};
// static
scoped_ptr<ServiceWorkerCache> ServiceWorkerCache::CreateMemoryCache(
net::URLRequestContext* request_context,
......@@ -611,7 +674,7 @@ void ServiceWorkerCache::CreateBackend(const ErrorCallback& callback) {
// debugging why the QuickStressBody unittest fails with it.
int rv = disk_cache::CreateCacheBackend(
cache_type,
net::CACHE_BACKEND_DEFAULT,
net::CACHE_BACKEND_SIMPLE,
path_,
kMaxCacheBytes,
false, /* force */
......@@ -701,6 +764,35 @@ void ServiceWorkerCache::Delete(ServiceWorkerFetchRequest* request,
open_entry_callback.Run(rv);
}
void ServiceWorkerCache::Keys(const RequestsCallback& callback) {
DCHECK(backend_);
// 1. Iterate through all of the entries, open them, and add them to a vector.
// 2. For each open entry:
// 2.1. Read the headers into a protobuf.
// 2.2. Copy the protobuf into a ServiceWorkerFetchRequest (a "key").
// 2.3. Push the response into a vector of requests to be returned.
// 3. Return the vector of requests (keys).
// The entries have to be loaded into a vector first because enumeration loops
// forever if you read data from a cache entry while enumerating.
scoped_ptr<KeysContext> keys_context(
new KeysContext(callback, weak_ptr_factory_.GetWeakPtr()));
void** backend_iterator = &keys_context->backend_iterator;
disk_cache::Entry** enumerated_entry = &keys_context->enumerated_entry;
net::CompletionCallback open_entry_callback =
base::Bind(KeysDidOpenNextEntry, base::Passed(keys_context.Pass()));
int rv = backend_->OpenNextEntry(
backend_iterator, enumerated_entry, open_entry_callback);
if (rv != net::ERR_IO_PENDING)
open_entry_callback.Run(rv);
}
bool ServiceWorkerCache::HasCreatedBackend() const {
return backend_;
}
......@@ -715,4 +807,87 @@ ServiceWorkerCache::ServiceWorkerCache(
weak_ptr_factory_(this) {
}
// static
void ServiceWorkerCache::KeysDidOpenNextEntry(
scoped_ptr<KeysContext> keys_context,
int rv) {
if (rv == net::ERR_FAILED) {
DCHECK(!keys_context->enumerated_entry);
// Enumeration is complete, extract the requests from the entries.
Entries::iterator iter = keys_context->entries.begin();
KeysProcessNextEntry(keys_context.Pass(), iter);
return;
}
base::WeakPtr<ServiceWorkerCache> cache = keys_context->cache;
if (rv < 0 || !cache) {
keys_context->original_callback.Run(ErrorTypeStorage,
scoped_ptr<Requests>());
return;
}
// Store the entry.
keys_context->entries.push_back(keys_context->enumerated_entry);
keys_context->enumerated_entry = NULL;
// Enumerate the next entry.
void** backend_iterator = &keys_context->backend_iterator;
disk_cache::Entry** enumerated_entry = &keys_context->enumerated_entry;
net::CompletionCallback open_entry_callback =
base::Bind(KeysDidOpenNextEntry, base::Passed(keys_context.Pass()));
rv = cache->backend_->OpenNextEntry(
backend_iterator, enumerated_entry, open_entry_callback);
if (rv != net::ERR_IO_PENDING)
open_entry_callback.Run(rv);
}
// static
void ServiceWorkerCache::KeysProcessNextEntry(
scoped_ptr<KeysContext> keys_context,
const Entries::iterator& iter) {
if (iter == keys_context->entries.end()) {
// All done. Return all of the keys.
keys_context->original_callback.Run(ErrorTypeOK,
keys_context->out_keys.Pass());
return;
}
ReadHeaders(
*iter,
base::Bind(KeysDidReadHeaders, base::Passed(keys_context.Pass()), iter));
}
// static
void ServiceWorkerCache::KeysDidReadHeaders(
scoped_ptr<KeysContext> keys_context,
const Entries::iterator& iter,
scoped_ptr<ServiceWorkerRequestResponseHeaders> headers) {
disk_cache::Entry* entry = *iter;
if (headers) {
keys_context->out_keys->push_back(
ServiceWorkerFetchRequest(GURL(entry->GetKey()),
headers->method(),
std::map<std::string, std::string>(),
GURL(),
false));
std::map<std::string, std::string>& req_headers =
keys_context->out_keys->back().headers;
for (int i = 0; i < headers->request_headers_size(); ++i) {
const ServiceWorkerRequestResponseHeaders::HeaderMap header =
headers->request_headers(i);
req_headers.insert(std::make_pair(header.name(), header.value()));
}
} else {
entry->Doom();
}
KeysProcessNextEntry(keys_context.Pass(), iter + 1);
}
} // namespace content
......@@ -25,6 +25,7 @@ class BlobStorageContext;
namespace content {
class ChromeBlobStorageContext;
class ServiceWorkerRequestResponseHeaders;
// TODO(jkarlin): Unload cache backend from memory once the cache object is no
// longer referenced in javascript.
......@@ -46,6 +47,10 @@ class CONTENT_EXPORT ServiceWorkerCache {
scoped_ptr<ServiceWorkerResponse>,
scoped_ptr<storage::BlobDataHandle>)>
ResponseCallback;
typedef std::vector<ServiceWorkerFetchRequest> Requests;
typedef base::Callback<void(ErrorType, scoped_ptr<Requests>)>
RequestsCallback;
static scoped_ptr<ServiceWorkerCache> CreateMemoryCache(
net::URLRequestContext* request_context,
base::WeakPtr<storage::BlobStorageContext> blob_context);
......@@ -80,6 +85,11 @@ class CONTENT_EXPORT ServiceWorkerCache {
void Delete(ServiceWorkerFetchRequest* request,
const ErrorCallback& callback);
// TODO(jkarlin): Have keys take an optional ServiceWorkerFetchRequest.
// Returns ErrorTypeOK and a vector of requests if there are no errors. The
// callback will always be called.
void Keys(const RequestsCallback& callback);
// Call to determine if CreateBackend must be called.
bool HasCreatedBackend() const;
......@@ -90,10 +100,23 @@ class CONTENT_EXPORT ServiceWorkerCache {
base::WeakPtr<ServiceWorkerCache> AsWeakPtr();
private:
struct KeysContext;
typedef std::vector<disk_cache::Entry*> Entries;
ServiceWorkerCache(const base::FilePath& path,
net::URLRequestContext* request_context,
base::WeakPtr<storage::BlobStorageContext> blob_context);
// Static callbacks for the Keys function.
static void KeysDidOpenNextEntry(scoped_ptr<KeysContext> keys_context,
int rv);
static void KeysProcessNextEntry(scoped_ptr<KeysContext> keys_context,
const Entries::iterator& iter);
static void KeysDidReadHeaders(
scoped_ptr<KeysContext> keys_context,
const Entries::iterator& iter,
scoped_ptr<ServiceWorkerRequestResponseHeaders> headers);
scoped_ptr<disk_cache::Backend> backend_;
base::FilePath path_;
net::URLRequestContext* request_context_;
......
......@@ -163,6 +163,27 @@ class ServiceWorkerCacheTest : public testing::Test {
return callback_error_ == ServiceWorkerCache::ErrorTypeOK;
}
bool Keys() {
scoped_ptr<base::RunLoop> loop(new base::RunLoop());
cache_->Keys(base::Bind(&ServiceWorkerCacheTest::RequestsCallback,
base::Unretained(this),
base::Unretained(loop.get())));
loop->Run();
return callback_error_ == ServiceWorkerCache::ErrorTypeOK;
}
void RequestsCallback(base::RunLoop* run_loop,
ServiceWorkerCache::ErrorType error,
scoped_ptr<ServiceWorkerCache::Requests> requests) {
callback_error_ = error;
callback_strings_.clear();
for (size_t i = 0u; i < requests->size(); ++i)
callback_strings_.push_back(requests->at(i).url.spec());
run_loop->Quit();
}
void ErrorTypeCallback(base::RunLoop* run_loop,
ServiceWorkerCache::ErrorType error) {
callback_error_ = error;
......@@ -192,6 +213,21 @@ class ServiceWorkerCacheTest : public testing::Test {
output->append(items[i].bytes(), items[i].length());
}
bool VerifyKeys(const std::vector<std::string>& expected_keys) {
if (expected_keys.size() != callback_strings_.size())
return false;
std::set<std::string> found_set;
for (int i = 0, max = callback_strings_.size(); i < max; ++i)
found_set.insert(callback_strings_[i]);
for (int i = 0, max = expected_keys.size(); i < max; ++i) {
if (found_set.find(expected_keys[i]) == found_set.end())
return false;
}
return true;
}
virtual bool MemoryOnly() { return false; }
protected:
......@@ -213,6 +249,7 @@ class ServiceWorkerCacheTest : public testing::Test {
ServiceWorkerCache::ErrorType callback_error_;
scoped_ptr<ServiceWorkerResponse> callback_response_;
scoped_ptr<storage::BlobDataHandle> callback_response_data_;
std::vector<std::string> callback_strings_;
};
class ServiceWorkerCacheTestP : public ServiceWorkerCacheTest,
......@@ -243,6 +280,65 @@ TEST_P(ServiceWorkerCacheTestP, PutBodyDropBlobRef) {
EXPECT_EQ(ServiceWorkerCache::ErrorTypeOK, callback_error_);
}
TEST_P(ServiceWorkerCacheTestP, MatchNoBody) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Match(no_body_request_.get()));
EXPECT_EQ(200, callback_response_->status_code);
EXPECT_STREQ("OK", callback_response_->status_text.c_str());
EXPECT_STREQ("http://example.com/no_body.html",
callback_response_->url.spec().c_str());
}
TEST_P(ServiceWorkerCacheTestP, MatchBody) {
EXPECT_TRUE(Put(body_request_.get(), body_response_.get()));
EXPECT_TRUE(Match(body_request_.get()));
EXPECT_EQ(200, callback_response_->status_code);
EXPECT_STREQ("OK", callback_response_->status_text.c_str());
EXPECT_STREQ("http://example.com/body.html",
callback_response_->url.spec().c_str());
std::string response_body;
CopyBody(callback_response_data_.get(), &response_body);
EXPECT_STREQ(expected_blob_data_.c_str(), response_body.c_str());
}
TEST_P(ServiceWorkerCacheTestP, EmptyKeys) {
EXPECT_TRUE(Keys());
EXPECT_EQ(0u, callback_strings_.size());
}
TEST_P(ServiceWorkerCacheTestP, TwoKeys) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Put(body_request_.get(), body_response_.get()));
EXPECT_TRUE(Keys());
EXPECT_EQ(2u, callback_strings_.size());
std::vector<std::string> expected_keys;
expected_keys.push_back(no_body_request_->url.spec());
expected_keys.push_back(body_request_->url.spec());
EXPECT_TRUE(VerifyKeys(expected_keys));
}
TEST_P(ServiceWorkerCacheTestP, TwoKeysThenOne) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Put(body_request_.get(), body_response_.get()));
EXPECT_TRUE(Keys());
EXPECT_EQ(2u, callback_strings_.size());
std::vector<std::string> expected_keys;
expected_keys.push_back(no_body_request_->url.spec());
expected_keys.push_back(body_request_->url.spec());
EXPECT_TRUE(VerifyKeys(expected_keys));
EXPECT_TRUE(Delete(body_request_.get()));
EXPECT_TRUE(Keys());
EXPECT_EQ(1u, callback_strings_.size());
std::vector<std::string> expected_key;
expected_key.push_back(no_body_request_->url.spec());
EXPECT_TRUE(VerifyKeys(expected_key));
}
// TODO(jkarlin): Once SimpleCache is working bug-free on Windows reenable these
// tests. In the meanwhile we know that Windows operations will be a little
// flaky (though not crashy). See https://crbug.com/409109
#ifndef OS_WIN
TEST_P(ServiceWorkerCacheTestP, DeleteNoBody) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Match(no_body_request_.get()));
......@@ -265,27 +361,6 @@ TEST_P(ServiceWorkerCacheTestP, DeleteBody) {
EXPECT_TRUE(Delete(body_request_.get()));
}
TEST_P(ServiceWorkerCacheTestP, MatchNoBody) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Match(no_body_request_.get()));
EXPECT_EQ(200, callback_response_->status_code);
EXPECT_STREQ("OK", callback_response_->status_text.c_str());
EXPECT_STREQ("http://example.com/no_body.html",
callback_response_->url.spec().c_str());
}
TEST_P(ServiceWorkerCacheTestP, MatchBody) {
EXPECT_TRUE(Put(body_request_.get(), body_response_.get()));
EXPECT_TRUE(Match(body_request_.get()));
EXPECT_EQ(200, callback_response_->status_code);
EXPECT_STREQ("OK", callback_response_->status_text.c_str());
EXPECT_STREQ("http://example.com/body.html",
callback_response_->url.spec().c_str());
std::string response_body;
CopyBody(callback_response_data_.get(), &response_body);
EXPECT_STREQ(expected_blob_data_.c_str(), response_body.c_str());
}
TEST_P(ServiceWorkerCacheTestP, QuickStressNoBody) {
for (int i = 0; i < 100; ++i) {
EXPECT_FALSE(Match(no_body_request_.get()));
......@@ -303,6 +378,7 @@ TEST_P(ServiceWorkerCacheTestP, QuickStressBody) {
ASSERT_TRUE(Delete(body_request_.get()));
}
}
#endif // OS_WIN
INSTANTIATE_TEST_CASE_P(ServiceWorkerCacheTest,
ServiceWorkerCacheTestP,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment