[CacheStorage] Pad and bin opaque resource sizes.

content/browser/cache_storage/cache_storage_cache_unittest.cc

Index: content/browser/cache_storage/cache_storage_cache_unittest.cc
diff --git a/content/browser/cache_storage/cache_storage_cache_unittest.cc b/content/browser/cache_storage/cache_storage_cache_unittest.cc
index 30dfe7754cf779f2d409aed6111a0592b9ccaaa4..e6821e738984bf9510b142a08b4dc9c22c5505b2 100644
--- a/content/browser/cache_storage/cache_storage_cache_unittest.cc
+++ b/content/browser/cache_storage/cache_storage_cache_unittest.cc
@@ -29,6 +29,7 @@
 #include "content/public/common/referrer.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "crypto/symmetric_key.h"
 #include "net/base/test_completion_callback.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -62,7 +63,7 @@ std::unique_ptr<storage::BlobProtocolHandler> CreateMockBlobProtocolHandler(
 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 {
@@ -72,6 +73,12 @@ class DelayableBackend : public disk_cache::Backend {
   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::BindOnce(
+          &DelayableBackend::OpenEntryDelayedImpl, base::Unretained(this), key,
+          base::Unretained(entry), callback);
+      return net::ERR_IO_PENDING;
+    }
     return backend_->OpenEntry(key, entry, callback);
   }
 
@@ -82,13 +89,6 @@ class DelayableBackend : public disk_cache::Backend {
   }
   int DoomEntry(const std::string& key,
                 const CompletionCallback& callback) override {
-    if (delay_doom_) {
-      doom_entry_callback_ =
-          base::BindOnce(&DelayableBackend::DoomEntryDelayedImpl,
-                         base::Unretained(this), key, callback);
-      return net::ERR_IO_PENDING;
-    }
-
     return backend_->DoomEntry(key, callback);
   }
   int DoomAllEntries(const CompletionCallback& callback) override {
@@ -124,25 +124,28 @@ class DelayableBackend : public disk_cache::Backend {
     return 0u;
   }
 
-  // Call to continue a delayed doom.
-  void DoomEntryContinue() {
-    EXPECT_FALSE(doom_entry_callback_.is_null());
-    std::move(doom_entry_callback_).Run();
+  // Call to continue a delayed call to OpenEntry.
+  bool OpenEntryContinue() {
+    if (open_entry_callback_.is_null())
+      return false;
+    std::move(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::OnceClosure doom_entry_callback_;
+  bool delay_open_entry_;
+  base::OnceClosure open_entry_callback_;
 };
 
 void CopyBody(const storage::BlobDataHandle& blob_handle, std::string* output) {
@@ -269,6 +272,11 @@ ServiceWorkerResponse SetCacheName(const ServiceWorkerResponse& original) {
   return result;
 }
 
+std::unique_ptr<crypto::SymmetricKey> CreateTestPaddingKey() {
+  return crypto::SymmetricKey::Import(crypto::SymmetricKey::HMAC_SHA1,
+                                      "abc123");
+}
+
 }  // namespace
 
 // A CacheStorageCache that can optionally delay during backend creation.
@@ -289,7 +297,9 @@ class TestCacheStorageCache : public CacheStorageCache {
                           request_context_getter,
                           quota_manager_proxy,
                           blob_context,
-                          0 /* cache_size */),
+                          0 /* cache_size */,
+                          0 /* cache_padding */,
+                          CreateTestPaddingKey()),
         delay_backend_creation_(false) {}
 
   void CreateBackend(ErrorCallback callback) override {
@@ -1593,6 +1603,53 @@ TEST_P(CacheStorageCacheTestP, Size) {
   EXPECT_EQ(0, Size());
 }
 
+TEST_F(CacheStorageCacheTest, VerifyOpaqueSizePadding) {
+  ServiceWorkerFetchRequest non_opaque_request(body_request_);
+  non_opaque_request.url = GURL("http://example.com/no-pad.html");
+  ServiceWorkerResponse non_opaque_response(body_response_);
+  const int side_data_size = 0;
+  EXPECT_EQ(0, CacheStorageCache::CalculateResponsePadding(
+                   non_opaque_response, CreateTestPaddingKey().get(),
+                   side_data_size));
+  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;
+  base::Time response_time(base::Time::Now());
+  opaque_response.response_time = response_time;
+
+  EXPECT_TRUE(Put(opaque_request, opaque_response));
+  // Padding can be zero bytes, so if the opaque URL + padding key changes in

[jkarlin, 2017/07/21 21:18:09]

Suggest:

// This test is fragile. Right now it deterministically adds non-zero padding.
But if the url, padding key, or padding algorithm change it might become zero.

[cmumford, 2017/07/27 23:33:24]

Done.

+  // the future check for this.
+  int64_t opaque_cache_size = Size();
+  EXPECT_GT(opaque_cache_size, 2 * non_padded_cache_size);
+
+  // Now write side data and expect to see the padding change.
+  const std::string expected_side_data1 = "OpaqueSideData";
+  scoped_refptr<net::IOBuffer> buffer1(
+      new net::StringIOBuffer(expected_side_data1));
+  EXPECT_TRUE(WriteSideData(opaque_request.url, response_time, buffer1,
+                            expected_side_data1.length()));
+  int64_t opaque_cache_size_with_side_data = Size();
+  EXPECT_GT(opaque_cache_size_with_side_data, 2 * non_padded_cache_size);
+  EXPECT_NE(opaque_cache_size_with_side_data, opaque_cache_size);

[jkarlin, 2017/07/21 21:18:09]

How is this testing that the padding changed? It's just showing that the size
grew after writing side data, which you would expect with or without padding.

[cmumford, 2017/07/27 23:33:24]

Agreed. I've changed a fair amount of this function to hopefully be a better
test and more clear.

+
+  // Now reset side data back to zero.
+  const std::string expected_side_data2 = "";
+  scoped_refptr<net::IOBuffer> buffer2(
+      new net::StringIOBuffer(expected_side_data2));
+  EXPECT_TRUE(WriteSideData(opaque_request.url, response_time, buffer2,
+                            expected_side_data2.length()));
+  int64_t size_after_delete = Size();
+  EXPECT_EQ(size_after_delete, opaque_cache_size);

[jkarlin, 2017/07/21 21:18:09]

Also delete the opaque data and verify that the size returns to
non_padded_cache_size.

[cmumford, 2017/07/27 23:33:24]

Done.

+}
+

[jkarlin, 2017/07/21 21:18:09]

We need some cache_storage_manager_unittest.cc tests that open/close the caches,
and change the CacheStorageCache static key to ensure that the key is properly
stored on disk.

[cmumford, 2017/07/27 23:33:24]

I added CacheStorageManagerTest.CacheSizePaddedAfterReopen and
CacheStorageManagerTest.PersistedCacheKeyUsed.

 TEST_P(CacheStorageCacheTestP, GetSizeThenClose) {
   EXPECT_TRUE(Put(body_request_, body_response_));
   int64_t cache_size = Size();
@@ -1612,7 +1669,7 @@ TEST_P(CacheStorageCacheTestP, VerifySerialScheduling) {
   // 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;
 
@@ -1636,7 +1693,7 @@ TEST_P(CacheStorageCacheTestP, VerifySerialScheduling) {
   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),
@@ -1648,7 +1705,7 @@ TEST_P(CacheStorageCacheTestP, VerifySerialScheduling) {
   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();