Revert of ReReland ServiceWorkerCacheStorage::Keys function (patchset #3...

Revert of ReReland ServiceWorkerCacheStorage::Keys function (patchset #3 id:140001 of https://codereview.chromium.org/543093003/)

Reason for revert:
Linux Valgrind now passes but ChromeOS Valgrind doesn't.  Looks like I need to do more on TearDown.

Original issue's description:
> ReReland ServiceWorkerCacheStorage::Keys function
>
> Suppression (error hash=#E226F7559AD360E7#):
> For more info on using suppressions see http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/memory-sheriff#TOC-Suppressing-memory-reports
> {
> <insert_a_suppression_name_here>
> Memcheck:Leak
> fun:_Znw*
> fun:_ZN4base8internal20PostTaskAndReplyImpl16PostTaskAndReplyERKN15tracked_objects8LocationERKNS_8CallbackIFvvEEESA_
> fun:_ZN4base10TaskRunner16PostTaskAndReplyERKN15tracked_objects8LocationERKNS_8CallbackIFvvEEES9_
> fun:_ZN10disk_cache15SimpleEntryImpl13CloseInternalEv
> fun:_ZN10disk_cache15SimpleEntryImpl24RunNextOperationIfNeededEv
> fun:_ZN10disk_cache15SimpleEntryImpl21DoomOperationCompleteERKN4base8CallbackIFviEEENS0_5StateEi
> }
> Original CL: https://codereview.chromium.org/477973002/
> Reland: https://codereview.chromium.org/528233003/
>
> The SimpleCache backend wasn't given a chance to finish up a close operation before the IO messageloop closed at the end of functions.  The tests now run the loop in TearDown.
>
> The original patch is PatchSet1 and the fix is PatchSet3.
>
> BUG=392621
>
> Committed: https://crrev.com/bbfe6ce49b8e99b190085dffbf446b474c5f07ff
> Cr-Commit-Position: refs/heads/master@{#294090}

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

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

Cr-Commit-Position: refs/heads/master@{#294150}

content/browser/service_worker/service_worker_cache.h

@@ -25,7 +25,6 @@ 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.
@@ -47,10 +46,6 @@ 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);
@@ -85,11 +80,6 @@ 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;
 
@@ -100,23 +90,10 @@ 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_;

content/browser/service_worker/service_worker_cache_storage_manager_unittest.cc

@@ -50,12 +50,6 @@ class ServiceWorkerCacheStorageManagerTest : public testing::Test {
         url_request_context, blob_storage_context->context()->AsWeakPtr());
   }
 
-  virtual void TearDown() OVERRIDE {
-    base::RunLoop().RunUntilIdle();
-    cache_manager_.reset();
-    base::RunLoop().RunUntilIdle();
-  }
-
   virtual bool MemoryOnly() { return false; }
 
   void BoolAndErrorCallback(

content/browser/service_worker/service_worker_cache_unittest.cc

@@ -78,12 +78,6 @@ class ServiceWorkerCacheTest : public testing::Test {
     CreateBackend();
   }
 
-  virtual void TearDown() OVERRIDE {
-    base::RunLoop().RunUntilIdle();
-    cache_.reset();
-    base::RunLoop().RunUntilIdle();
-  }
-
   void CreateRequests(ChromeBlobStorageContext* blob_storage_context) {
     std::map<std::string, std::string> headers;
     headers.insert(std::make_pair("a", "a"));
@@ -169,27 +163,6 @@ 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;
@@ -219,21 +192,6 @@ 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:
@@ -255,7 +213,6 @@ 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,
@@ -286,65 +243,6 @@ 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()));
@@ -367,6 +265,27 @@ 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()));
@@ -384,7 +303,6 @@ TEST_P(ServiceWorkerCacheTestP, QuickStressBody) {
     ASSERT_TRUE(Delete(body_request_.get()));
   }
 }
-#endif  // OS_WIN
 
 INSTANTIATE_TEST_CASE_P(ServiceWorkerCacheTest,
                         ServiceWorkerCacheTestP,