Commit 01bc0b71 authored by scottmg's avatar scottmg Committed by Commit bot

Revert of Add Keys() function to ServiceWorkerCache. (patchset #13 id:280001...

Revert of Add Keys() function to ServiceWorkerCache. (patchset #13 id:280001 of https://codereview.chromium.org/477973002/)

Reason for revert:
Failing on content_unittests

EmptyKeys_1
TwoKeys_1
TwoKeysThenOne_1

on Linux ASAN LSAN

Original issue's description:
> Add Keys() function to ServiceWorkerCache.
>
> Other changes:
> * Introduced a ReadHeaders helper function which reads headers from a cache entry into a protobuf.  This is used by Match() and Keys().
>
>
> BUG=392621
>
> Committed: https://chromium.googlesource.com/chromium/src/+/02812f0a3edcacca1f7e05ea0a47cb3d8c6025e1

TBR=michaeln@chromium.org,gavinp@chromium.org,jkarlin@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=392621

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

Cr-Commit-Position: refs/heads/master@{#292970}
parent b3e8766e
...@@ -25,7 +25,6 @@ class BlobStorageContext; ...@@ -25,7 +25,6 @@ class BlobStorageContext;
namespace content { namespace content {
class ChromeBlobStorageContext; class ChromeBlobStorageContext;
class ServiceWorkerRequestResponseHeaders;
// TODO(jkarlin): Unload cache backend from memory once the cache object is no // TODO(jkarlin): Unload cache backend from memory once the cache object is no
// longer referenced in javascript. // longer referenced in javascript.
...@@ -47,10 +46,6 @@ class CONTENT_EXPORT ServiceWorkerCache { ...@@ -47,10 +46,6 @@ class CONTENT_EXPORT ServiceWorkerCache {
scoped_ptr<ServiceWorkerResponse>, scoped_ptr<ServiceWorkerResponse>,
scoped_ptr<storage::BlobDataHandle>)> scoped_ptr<storage::BlobDataHandle>)>
ResponseCallback; ResponseCallback;
typedef std::vector<ServiceWorkerFetchRequest> Requests;
typedef base::Callback<void(ErrorType, scoped_ptr<Requests>)>
RequestsCallback;
static scoped_ptr<ServiceWorkerCache> CreateMemoryCache( static scoped_ptr<ServiceWorkerCache> CreateMemoryCache(
net::URLRequestContext* request_context, net::URLRequestContext* request_context,
base::WeakPtr<storage::BlobStorageContext> blob_context); base::WeakPtr<storage::BlobStorageContext> blob_context);
...@@ -85,11 +80,6 @@ class CONTENT_EXPORT ServiceWorkerCache { ...@@ -85,11 +80,6 @@ class CONTENT_EXPORT ServiceWorkerCache {
void Delete(ServiceWorkerFetchRequest* request, void Delete(ServiceWorkerFetchRequest* request,
const ErrorCallback& callback); 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. // Call to determine if CreateBackend must be called.
bool HasCreatedBackend() const; bool HasCreatedBackend() const;
...@@ -100,23 +90,10 @@ class CONTENT_EXPORT ServiceWorkerCache { ...@@ -100,23 +90,10 @@ class CONTENT_EXPORT ServiceWorkerCache {
base::WeakPtr<ServiceWorkerCache> AsWeakPtr(); base::WeakPtr<ServiceWorkerCache> AsWeakPtr();
private: private:
struct KeysContext;
typedef std::vector<disk_cache::Entry*> Entries;
ServiceWorkerCache(const base::FilePath& path, ServiceWorkerCache(const base::FilePath& path,
net::URLRequestContext* request_context, net::URLRequestContext* request_context,
base::WeakPtr<storage::BlobStorageContext> blob_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_; scoped_ptr<disk_cache::Backend> backend_;
base::FilePath path_; base::FilePath path_;
net::URLRequestContext* request_context_; net::URLRequestContext* request_context_;
......
...@@ -163,27 +163,6 @@ class ServiceWorkerCacheTest : public testing::Test { ...@@ -163,27 +163,6 @@ class ServiceWorkerCacheTest : public testing::Test {
return callback_error_ == ServiceWorkerCache::ErrorTypeOK; 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, void ErrorTypeCallback(base::RunLoop* run_loop,
ServiceWorkerCache::ErrorType error) { ServiceWorkerCache::ErrorType error) {
callback_error_ = error; callback_error_ = error;
...@@ -213,21 +192,6 @@ class ServiceWorkerCacheTest : public testing::Test { ...@@ -213,21 +192,6 @@ class ServiceWorkerCacheTest : public testing::Test {
output->append(items[i].bytes(), items[i].length()); 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; } virtual bool MemoryOnly() { return false; }
protected: protected:
...@@ -249,7 +213,6 @@ class ServiceWorkerCacheTest : public testing::Test { ...@@ -249,7 +213,6 @@ class ServiceWorkerCacheTest : public testing::Test {
ServiceWorkerCache::ErrorType callback_error_; ServiceWorkerCache::ErrorType callback_error_;
scoped_ptr<ServiceWorkerResponse> callback_response_; scoped_ptr<ServiceWorkerResponse> callback_response_;
scoped_ptr<storage::BlobDataHandle> callback_response_data_; scoped_ptr<storage::BlobDataHandle> callback_response_data_;
std::vector<std::string> callback_strings_;
}; };
class ServiceWorkerCacheTestP : public ServiceWorkerCacheTest, class ServiceWorkerCacheTestP : public ServiceWorkerCacheTest,
...@@ -280,65 +243,6 @@ TEST_P(ServiceWorkerCacheTestP, PutBodyDropBlobRef) { ...@@ -280,65 +243,6 @@ TEST_P(ServiceWorkerCacheTestP, PutBodyDropBlobRef) {
EXPECT_EQ(ServiceWorkerCache::ErrorTypeOK, callback_error_); 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) { TEST_P(ServiceWorkerCacheTestP, DeleteNoBody) {
EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get())); EXPECT_TRUE(Put(no_body_request_.get(), no_body_response_.get()));
EXPECT_TRUE(Match(no_body_request_.get())); EXPECT_TRUE(Match(no_body_request_.get()));
...@@ -361,6 +265,27 @@ TEST_P(ServiceWorkerCacheTestP, DeleteBody) { ...@@ -361,6 +265,27 @@ TEST_P(ServiceWorkerCacheTestP, DeleteBody) {
EXPECT_TRUE(Delete(body_request_.get())); 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) { TEST_P(ServiceWorkerCacheTestP, QuickStressNoBody) {
for (int i = 0; i < 100; ++i) { for (int i = 0; i < 100; ++i) {
EXPECT_FALSE(Match(no_body_request_.get())); EXPECT_FALSE(Match(no_body_request_.get()));
...@@ -378,7 +303,6 @@ TEST_P(ServiceWorkerCacheTestP, QuickStressBody) { ...@@ -378,7 +303,6 @@ TEST_P(ServiceWorkerCacheTestP, QuickStressBody) {
ASSERT_TRUE(Delete(body_request_.get())); ASSERT_TRUE(Delete(body_request_.get()));
} }
} }
#endif // OS_WIN
INSTANTIATE_TEST_CASE_P(ServiceWorkerCacheTest, INSTANTIATE_TEST_CASE_P(ServiceWorkerCacheTest,
ServiceWorkerCacheTestP, 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