Simplify cached_metadata_handler.h

The main purpose is to replace the default CacheType argument with a
normal one. The callers are already passing this parameter explicitly in
the vast majority of cases, as it is really helpful to know whether the
operation will be persisted across platform instances.

For ClearCachedMetadata(): all callers are passing the parameter
explicitly. We will continue to need both modes (clearing on disk for
privacy reasons, clearing from memory to free up some memory). Making it
non-default is trivial.

For SetCachedMetadata(): the documentation says that is should always
persist, hence it seems that this was the intention initially. The
kCacheLocally is used only in tests. Making a test-only carveout with
DisableSendToPlatformForTesting(). The reason for not persisting in
those tests is not clear to me, perhaps this makes the tests run faster?

Increasing the size of the class is slightly unfortunate, I could not
find an elegant solution to overcome this. Though the size of these
objects is small compared to all of the serialized data, so it feels OK.

Bug: 1045052
Change-Id: Ifec42fbd66498c6860b32d86a25d3e857a97b4c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2019246Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Auto-Submit: Egor Pasko <pasko@chromium.org>
Commit-Queue: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736869}

third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc

@@ -341,9 +341,9 @@ TEST_F(ScriptStreamingTest, DISABLED_SuppressingStreaming) {
 
   SingleCachedMetadataHandler* cache_handler = resource_->CacheHandler();
   EXPECT_TRUE(cache_handler);
+  cache_handler->DisableSendToPlatformForTesting();
   cache_handler->SetCachedMetadata(V8CodeCache::TagForCodeCache(cache_handler),
-                                   reinterpret_cast<const uint8_t*>("X"), 1,
-                                   CachedMetadataHandler::kCacheLocally);
+                                   reinterpret_cast<const uint8_t*>("X"), 1);
 
   AppendData("function foo() {");
   AppendPadding();

third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc

@@ -237,8 +237,7 @@ static void ProduceCacheInternal(
         cache_handler->ClearCachedMetadata(
             CachedMetadataHandler::kCacheLocally);
         cache_handler->SetCachedMetadata(
-            V8CodeCache::TagForCodeCache(cache_handler), data, length,
-            CachedMetadataHandler::kSendToPlatform);
+            V8CodeCache::TagForCodeCache(cache_handler), data, length);
       }
 
       TRACE_EVENT_END1(
@@ -297,9 +296,9 @@ void V8CodeCache::SetCacheTimeStamp(
     SingleCachedMetadataHandler* cache_handler) {
   uint64_t now_ms = base::TimeTicks::Now().since_origin().InMilliseconds();
   cache_handler->ClearCachedMetadata(CachedMetadataHandler::kCacheLocally);
-  cache_handler->SetCachedMetadata(
-      TagForTimeStamp(cache_handler), reinterpret_cast<uint8_t*>(&now_ms),
-      sizeof(now_ms), CachedMetadataHandler::kSendToPlatform);
+  cache_handler->SetCachedMetadata(TagForTimeStamp(cache_handler),
+                                   reinterpret_cast<uint8_t*>(&now_ms),
+                                   sizeof(now_ms));
 }
 
 // static

third_party/blink/renderer/modules/service_worker/service_worker_script_cached_metadata_handler.cc

@@ -39,10 +39,7 @@ void ServiceWorkerScriptCachedMetadataHandler::Trace(blink::Visitor* visitor) {
 void ServiceWorkerScriptCachedMetadataHandler::SetCachedMetadata(
     uint32_t data_type_id,
     const uint8_t* data,
-    size_t size,
-    CacheType type) {
-  if (type != kSendToPlatform)
-    return;
+    size_t size) {
   cached_metadata_ = CachedMetadata::Create(data_type_id, data, size);
   base::span<const uint8_t> serialized_data =
       cached_metadata_->SerializedData();

third_party/blink/renderer/modules/service_worker/service_worker_script_cached_metadata_handler.h

@@ -27,8 +27,7 @@ class ServiceWorkerScriptCachedMetadataHandler
   void Trace(blink::Visitor*) override;
   void SetCachedMetadata(uint32_t data_type_id,
                          const uint8_t*,
-                         size_t,
-                         CacheType) override;
+                         size_t) override;
   void ClearCachedMetadata(CacheType) override;
   scoped_refptr<CachedMetadata> GetCachedMetadata(
       uint32_t data_type_id) const override;

third_party/blink/renderer/platform/loader/fetch/cached_metadata_handler.h

@@ -47,8 +47,8 @@ PLATFORM_EXPORT bool ShouldUseIsolatedCodeCache(mojom::RequestContextType,
 class CachedMetadataHandler : public GarbageCollected<CachedMetadataHandler> {
  public:
   enum CacheType {
-    kSendToPlatform,  // send cache data to blink::Platform::cacheMetadata
-    kCacheLocally     // cache only in Resource's member variables
+    kSendToPlatform,  // send cache data to blink::Platform::CacheMetadata
+    kCacheLocally     // cache only in objects of this class
   };
 
   // Enum for marking serialized cached metadatas so that the deserializers
@@ -63,7 +63,7 @@ class CachedMetadataHandler : public GarbageCollected<CachedMetadataHandler> {
   virtual void Trace(blink::Visitor* visitor) {}
 
   // Reset existing metadata, to allow setting new data.
-  virtual void ClearCachedMetadata(CacheType = kCacheLocally) = 0;
+  virtual void ClearCachedMetadata(CacheType) = 0;
 
   // Returns the encoding to which the cache is specific.
   virtual String Encoding() const = 0;
@@ -88,8 +88,13 @@ class SingleCachedMetadataHandler : public CachedMetadataHandler {
   // identifier that is used to distinguish data generated by the caller.
   virtual void SetCachedMetadata(uint32_t data_type_id,
                                  const uint8_t*,
-                                 size_t,
-                                 CacheType = kSendToPlatform) = 0;
+                                 size_t) = 0;
+
+  // Permanently disable persisting CachedMetadata in the platform only when it
+  // is set.
+  void DisableSendToPlatformForTesting() {
+    disable_send_to_platform_for_testing_ = true;
+  }
 
   // Returns cached metadata of the given type associated with this resource.
   // This cached metadata can be pruned at any time.
@@ -98,6 +103,7 @@ class SingleCachedMetadataHandler : public CachedMetadataHandler {
 
  protected:
   SingleCachedMetadataHandler() = default;
+  bool disable_send_to_platform_for_testing_ = false;
 };
 
 }  // namespace blink

third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.cc

@@ -18,22 +18,19 @@ void ScriptCachedMetadataHandler::Trace(blink::Visitor* visitor) {
   CachedMetadataHandler::Trace(visitor);
 }
 
-void ScriptCachedMetadataHandler::SetCachedMetadata(
-    uint32_t data_type_id,
+void ScriptCachedMetadataHandler::SetCachedMetadata(uint32_t data_type_id,
                                                     const uint8_t* data,
-    size_t size,
-    CachedMetadataHandler::CacheType cache_type) {
+                                                    size_t size) {
   // Currently, only one type of cached metadata per resource is supported. If
   // the need arises for multiple types of metadata per resource this could be
   // enhanced to store types of metadata in a map.
   DCHECK(!cached_metadata_);
   cached_metadata_ = CachedMetadata::Create(data_type_id, data, size);
-  if (cache_type == CachedMetadataHandler::kSendToPlatform)
+  if (!disable_send_to_platform_for_testing_)
     SendToPlatform();
 }
 
-void ScriptCachedMetadataHandler::ClearCachedMetadata(
-    CachedMetadataHandler::CacheType cache_type) {
+void ScriptCachedMetadataHandler::ClearCachedMetadata(CacheType cache_type) {
   cached_metadata_ = nullptr;
   if (cache_type == CachedMetadataHandler::kSendToPlatform)
     SendToPlatform();

third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.h

@@ -34,7 +34,7 @@ class PLATFORM_EXPORT ScriptCachedMetadataHandler final
                               std::unique_ptr<CachedMetadataSender>);
   ~ScriptCachedMetadataHandler() override = default;
   void Trace(blink::Visitor*) override;
-  void SetCachedMetadata(uint32_t, const uint8_t*, size_t, CacheType) override;
+  void SetCachedMetadata(uint32_t, const uint8_t*, size_t) override;
   void ClearCachedMetadata(CacheType) override;
   scoped_refptr<CachedMetadata> GetCachedMetadata(uint32_t) const override;
 

third_party/blink/renderer/platform/loader/fetch/source_keyed_cached_metadata_handler.cc

@@ -28,12 +28,11 @@ class SourceKeyedCachedMetadataHandler::SingleKeyHandler final
 
   void SetCachedMetadata(uint32_t data_type_id,
                          const uint8_t* data,
-                         size_t size,
-                         CacheType cache_type) override {
+                         size_t size) override {
     DCHECK(!parent_->cached_metadata_map_.Contains(key_));
     parent_->cached_metadata_map_.insert(
         key_, CachedMetadata::Create(data_type_id, data, size));
-    if (cache_type == CachedMetadataHandler::kSendToPlatform)
+    if (!disable_send_to_platform_for_testing_)
       parent_->SendToPlatform();
   }
 

third_party/blink/renderer/platform/loader/fetch/source_keyed_cached_metadata_handler_test.cc

@@ -293,8 +293,8 @@ TEST(SourceKeyedCachedMetadataHandlerTest, Serialize_SetWithNoSendDoesNotSend) {
       handler->HandlerForSource(source2);
 
   Vector<uint8_t> data1 = {1, 2, 3};
-  source1_handler->SetCachedMetadata(0xbeef, data1.data(), data1.size(),
-                                     CachedMetadataHandler::kCacheLocally);
+  source1_handler->DisableSendToPlatformForTesting();
+  source1_handler->SetCachedMetadata(0xbeef, data1.data(), data1.size());
 
   Vector<uint8_t> data2 = {3, 4, 5, 6};
   source2_handler->SetCachedMetadata(0x5eed, data2.data(), data2.size());