[CacheStorage] Pad and bin opaque resource sizes.

content/browser/cache_storage/cache_storage_cache_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/cache_storage/cache_storage_cache.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <set>
 #include <utility>
 
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "base/strings/string_split.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/blob_storage/chrome_blob_storage_context.h"
 #include "content/browser/cache_storage/cache_storage_cache_handle.h"
@@ skipping 32 matching lines @@
   // The FileSystemContext and thread task runner are not actually used but a
   // task runner is needed to avoid a DCHECK in BlobURLRequestJob ctor.
   return base::WrapUnique(new storage::BlobProtocolHandler(
       blob_storage_context, NULL, base::ThreadTaskRunnerHandle::Get().get()));
 }
 
 // A disk_cache::Backend wrapper that can delay operations.
 class DelayableBackend : public disk_cache::Backend {
  public:
   explicit DelayableBackend(std::unique_ptr<disk_cache::Backend> backend)
-      : backend_(std::move(backend)), delay_doom_(false) {}
+      : backend_(std::move(backend)), delay_open_entry_(false) {}
 
   // disk_cache::Backend overrides
   net::CacheType GetCacheType() const override {
     return backend_->GetCacheType();
   }
   int32_t GetEntryCount() const override { return backend_->GetEntryCount(); }
   int OpenEntry(const std::string& key,
                 disk_cache::Entry** entry,
                 const CompletionCallback& callback) override {
+    if (delay_open_entry_ && open_entry_callback_.is_null()) {
+      open_entry_callback_ = base::Bind(&DelayableBackend::OpenEntryDelayedImpl,
+                                        base::Unretained(this), key,
+                                        base::Unretained(entry), callback);
+      return net::ERR_IO_PENDING;
+    }
     return backend_->OpenEntry(key, entry, callback);
   }
 
   int CreateEntry(const std::string& key,
                   disk_cache::Entry** entry,
                   const CompletionCallback& callback) override {
     return backend_->CreateEntry(key, entry, callback);
   }
   int DoomEntry(const std::string& key,
                 const CompletionCallback& callback) override {
-    if (delay_doom_) {
-      doom_entry_callback_ = base::Bind(&DelayableBackend::DoomEntryDelayedImpl,
-                                        base::Unretained(this), key, callback);
-      return net::ERR_IO_PENDING;
-    }
-
     return backend_->DoomEntry(key, callback);
   }
   int DoomAllEntries(const CompletionCallback& callback) override {
     return backend_->DoomAllEntries(callback);
   }
   int DoomEntriesBetween(base::Time initial_time,
                          base::Time end_time,
                          const CompletionCallback& callback) override {
     return backend_->DoomEntriesBetween(initial_time, end_time, callback);
   }
@@ skipping 15 matching lines @@
     return backend_->OnExternalCacheHit(key);
   }
 
   size_t DumpMemoryStats(
       base::trace_event::ProcessMemoryDump* pmd,
       const std::string& parent_absolute_name) const override {
     NOTREACHED();
     return 0u;
   }
 
-  // Call to continue a delayed doom.
-  void DoomEntryContinue() {
-    EXPECT_FALSE(doom_entry_callback_.is_null());
-    doom_entry_callback_.Run();
+  // Call to continue a delayed call to OpenEntry.
+  bool OpenEntryContinue() {
+    if (open_entry_callback_.is_null())
+      return false;
+    open_entry_callback_.Run();
+    return true;
   }
 
-  void set_delay_doom(bool value) { delay_doom_ = value; }
+  void set_delay_open_entry(bool value) { delay_open_entry_ = value; }
 
  private:
-  void DoomEntryDelayedImpl(const std::string& key,
+  void OpenEntryDelayedImpl(const std::string& key,
+                            disk_cache::Entry** entry,
                             const CompletionCallback& callback) {
-    int rv = backend_->DoomEntry(key, callback);
+    int rv = backend_->OpenEntry(key, entry, callback);
     if (rv != net::ERR_IO_PENDING)
       callback.Run(rv);
   }
 
   std::unique_ptr<disk_cache::Backend> backend_;
-  bool delay_doom_;
-  base::Closure doom_entry_callback_;
+  bool delay_open_entry_;
+  base::Closure open_entry_callback_;
 };
 
 void CopyBody(const storage::BlobDataHandle& blob_handle, std::string* output) {
   *output = std::string();
   std::unique_ptr<storage::BlobDataSnapshot> data =
       blob_handle.CreateSnapshot();
   const auto& items = data->items();
   for (const auto& item : items) {
     switch (item->type()) {
       case storage::DataElement::TYPE_BYTES: {
@@ skipping 126 matching lines @@
       const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
       const scoped_refptr<storage::QuotaManagerProxy>& quota_manager_proxy,
       base::WeakPtr<storage::BlobStorageContext> blob_context)
       : CacheStorageCache(origin,
                           cache_name,
                           path,
                           cache_storage,
                           request_context_getter,
                           quota_manager_proxy,
                           blob_context,
-                          0 /* cache_size */),
+                          0 /* cache_size */,
+                          0 /* cache_padding */,
+                          "123abc" /* padding_key */),
         delay_backend_creation_(false) {}
 
   void CreateBackend(const ErrorCallback& callback) override {
     backend_creation_callback_ = callback;
     if (delay_backend_creation_)
       return;
     ContinueCreateBackend();
   }
 
   void ContinueCreateBackend() {
@@ skipping 378 matching lines @@
 
   void VerifyAllOpsFail() {
     EXPECT_FALSE(Put(no_body_request_, no_body_response_));
     EXPECT_FALSE(Match(no_body_request_));
     EXPECT_FALSE(Delete(body_request_));
     EXPECT_FALSE(Keys());
   }
 
   virtual bool MemoryOnly() { return false; }
 
+  virtual bool PadResources() const { return false; }
+
   void SetMaxQuerySizeBytes(size_t max_bytes) {
     cache_->max_query_size_bytes_ = max_bytes;
   }
 
  protected:
   base::ScopedTempDir temp_dir_;
   TestBrowserThreadBundle browser_thread_bundle_;
   TestBrowserContext browser_context_;
   std::unique_ptr<net::URLRequestJobFactoryImpl> url_request_job_factory_;
   scoped_refptr<MockSpecialStoragePolicy> quota_policy_;
@@ skipping 14 matching lines @@
   std::string expected_blob_data_;
 
   CacheStorageError callback_error_ = CACHE_STORAGE_OK;
   std::unique_ptr<ServiceWorkerResponse> callback_response_;
   std::unique_ptr<storage::BlobDataHandle> callback_response_data_;
   std::vector<std::string> callback_strings_;
   bool callback_closed_ = false;
   int64_t callback_size_ = 0;
 };
 
+enum class CacheLocation { IN_MEMORY, ON_DISK };
+
+enum class PadType { ADD_PADDING, NO_PADDING };
+
 class CacheStorageCacheTestP : public CacheStorageCacheTest,
-                               public testing::WithParamInterface<bool> {
-  bool MemoryOnly() override { return !GetParam(); }
+                               public testing::WithParamInterface<
+                                   testing::tuple<CacheLocation, PadType>> {
+ public:
+  bool MemoryOnly() override {
+    return testing::get<0>(GetParam()) == CacheLocation::IN_MEMORY;
+  }
+
+  bool PadResources() const override {

[jkarlin, 2017/06/08 18:58:48]

It's unclear to me what PadResources is being used for? I don't see it affecting
any of the test behavior.

[cmumford, 2017/06/12 18:09:31]

It was, but unnecessarily, so I removed it.

+    return testing::get<1>(GetParam()) == PadType::ADD_PADDING;
+  }
 };
 
 TEST_P(CacheStorageCacheTestP, PutNoBody) {
   EXPECT_TRUE(Put(no_body_request_, no_body_response_));
 }
 
 TEST_P(CacheStorageCacheTestP, PutBody) {
   EXPECT_TRUE(Put(body_request_, body_response_));
 }
 
@@ skipping 848 matching lines @@
   EXPECT_TRUE(Delete(no_body_request_));
   EXPECT_EQ(0, Size());
 
   EXPECT_TRUE(Put(body_request_, body_response_));
   EXPECT_LT(no_body_size, Size());
 
   EXPECT_TRUE(Delete(body_request_));
   EXPECT_EQ(0, Size());
 }
 
+TEST_P(CacheStorageCacheTestP, VerifyOpaqueSizePadding) {
+  // Test InMemory/OnDisk configs
+  if (PadResources())
+    return;
+
+  const std::string padding_key = "123abc";
+
+  ServiceWorkerFetchRequest non_opaque_request(body_request_);
+  non_opaque_request.url = GURL("http://example.com/no-pad.html");
+  ServiceWorkerResponse non_opaque_response(body_response_);
+  EXPECT_EQ(CacheStorageCache::CalculateResponsePadding(non_opaque_response,

[jkarlin, 2017/06/08 18:58:48]

EXPECT_EQ(0, ...)

[cmumford, 2017/06/12 18:09:31]

Done.

+                                                        padding_key),
+            0);
+  EXPECT_TRUE(Put(non_opaque_request, non_opaque_response));
+  int64_t non_padded_cache_size = Size();
+
+  ServiceWorkerFetchRequest opaque_request(non_opaque_request);
+  opaque_request.url = GURL("http://example.com/opaque.html");
+  // Same URL length means same cache sizes (ignoring padding).
+  EXPECT_EQ(opaque_request.url.spec().length(),
+            non_opaque_request.url.spec().length());
+  ServiceWorkerResponse opaque_response(non_opaque_response);
+  opaque_response.response_type = blink::kWebServiceWorkerResponseTypeOpaque;
+
+  const int64_t padding_size =
+      CacheStorageCache::CalculateResponsePadding(opaque_response, padding_key);

[jkarlin, 2017/06/08 18:58:48]

I'd rather that we not call this private function and just use the test below to
verify that we more than doubled in size using the public function.

[cmumford, 2017/06/12 18:09:31]

Will do. Technically padding can be zero, so the test is relying on the
hard-coded padding_key + URL returning > 0. I'll add a comment to make this
clear.

+  EXPECT_GT(padding_size, 0);
+  EXPECT_TRUE(Put(opaque_request, opaque_response));
+  EXPECT_EQ(2 * non_padded_cache_size + padding_size, Size());
+}
+
 TEST_P(CacheStorageCacheTestP, GetSizeThenClose) {
   EXPECT_TRUE(Put(body_request_, body_response_));
   int64_t cache_size = Size();
   EXPECT_EQ(cache_size, GetSizeThenClose());
   VerifyAllOpsFail();
 }
 
 TEST_P(CacheStorageCacheTestP, OpsFailOnClosedBackend) {
   // Create the backend and put something in it.
   EXPECT_TRUE(Put(body_request_, body_response_));
   EXPECT_TRUE(Close());
   VerifyAllOpsFail();
 }
 
 TEST_P(CacheStorageCacheTestP, VerifySerialScheduling) {
   // Start two operations, the first one is delayed but the second isn't. The
   // second should wait for the first.
   EXPECT_TRUE(Keys());  // Opens the backend.
   DelayableBackend* delayable_backend = cache_->UseDelayableBackend();
-  delayable_backend->set_delay_doom(true);
+  delayable_backend->set_delay_open_entry(true);
 
   int sequence_out = -1;
 
   CacheStorageBatchOperation operation1;
   operation1.operation_type = CACHE_STORAGE_CACHE_OPERATION_TYPE_PUT;
   operation1.request = body_request_;
   operation1.response = body_response_;
 
   std::unique_ptr<base::RunLoop> close_loop1(new base::RunLoop());
   cache_->BatchOperation(
       std::vector<CacheStorageBatchOperation>(1, operation1),
       base::Bind(&CacheStorageCacheTest::SequenceCallback,
                  base::Unretained(this), 1, &sequence_out, close_loop1.get()));
 
   // Blocks on creating the cache entry.
   base::RunLoop().RunUntilIdle();
 
   CacheStorageBatchOperation operation2;
   operation2.operation_type = CACHE_STORAGE_CACHE_OPERATION_TYPE_PUT;
   operation2.request = body_request_;
   operation2.response = body_response_;
 
-  delayable_backend->set_delay_doom(false);
+  delayable_backend->set_delay_open_entry(false);
   std::unique_ptr<base::RunLoop> close_loop2(new base::RunLoop());
   cache_->BatchOperation(
       std::vector<CacheStorageBatchOperation>(1, operation2),
       base::Bind(&CacheStorageCacheTest::SequenceCallback,
                  base::Unretained(this), 2, &sequence_out, close_loop2.get()));
 
   // The second put operation should wait for the first to complete.
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(callback_response_);
 
-  delayable_backend->DoomEntryContinue();
+  EXPECT_TRUE(delayable_backend->OpenEntryContinue());
   close_loop1->Run();
   EXPECT_EQ(1, sequence_out);
   close_loop2->Run();
   EXPECT_EQ(2, sequence_out);
 }
 
-INSTANTIATE_TEST_CASE_P(CacheStorageCacheTest,
-                        CacheStorageCacheTestP,
-                        ::testing::Values(false, true));
+INSTANTIATE_TEST_CASE_P(
+    CacheStorageCacheTest,
+    CacheStorageCacheTestP,
+    ::testing::Combine(
+        ::testing::Values(CacheLocation::IN_MEMORY, CacheLocation::ON_DISK),
+        ::testing::Values(PadType::ADD_PADDING, PadType::NO_PADDING)));
 
 }  // namespace content