Make the empty fetch handler recognized and remembered.

In this CL, we made the empty fetch handler recognized at ServiceWorkerGlobalScope and remembered in ServiceWorkerDatabase. Since plumbling from the ServiceWorkerGlobalScope to ServiceWorkerDatabase has already been finished, the information can also be used in ServiceWorkerVersion to decide executing the fetch handler or not. Bug: 1347319 Bug: 1351246 Change-Id: I5c5bac2addb45b8380d9e1bfc1efbd0cfc5319a6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3805086 Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Cr-Commit-Position: refs/heads/main@{#1032908}
chromium · Aug 9, 2022 · 6306836 · 6306836
1 parent fd22e50
commit 6306836
Show file tree

Hide file tree

Showing 8 changed files with 208 additions and 24 deletions.
diff --git a/components/services/storage/service_worker/service_worker_database.cc b/components/services/storage/service_worker/service_worker_database.cc
@@ -1637,30 +1637,29 @@ ServiceWorkerDatabase::Status ServiceWorkerDatabase::ParseRegistrationData(
       (data.has_fetch_handler())
           ? blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable
           : blink::mojom::ServiceWorkerFetchHandlerType::kNoHandler;
-  if (data.has_fetch_handler_type()) {
+  if (data.has_fetch_handler_skippable_type()) {
     if (!data.has_fetch_handler()) {
-      DLOG(ERROR) << "has_fetch_handler must be true if fetch_handler_type"
-                  << " is set.";
+      DLOG(ERROR)
+          << "has_fetch_handler must be true if fetch_handler_skippable_type"
+          << " is set.";
       return Status::kErrorCorrupted;
     }
-    if (!ServiceWorkerRegistrationData_FetchHandlerType_IsValid(
-            data.fetch_handler_type())) {
-      DLOG(ERROR) << "Fetch handler type '" << data.fetch_handler_type()
-                  << "' is not valid.";
+    if (!ServiceWorkerRegistrationData_FetchHandlerSkippableType_IsValid(
+            data.fetch_handler_skippable_type())) {
+      DLOG(ERROR) << "Fetch handler type '"
+                  << data.fetch_handler_skippable_type() << "' is not valid.";
       return Status::kErrorCorrupted;
     }
-    switch (data.fetch_handler_type()) {
+    switch (data.fetch_handler_skippable_type()) {
       case ServiceWorkerRegistrationData::NOT_SKIPPABLE:
         (*out)->fetch_handler_type =
             blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable;
         break;
-      // TODO(crbug.com/1347319): implement other fetch_handler_type.
-      default:
-        // UNKNOWN_FETCH_HANDLER, which must not be stored, should also be
-        // handled here.
-        DLOG(ERROR) << "Fetch handler type '" << data.fetch_handler_type()
-                    << "' is not known.";
-        return Status::kErrorCorrupted;
+      case ServiceWorkerRegistrationData::SKIPPABLE_EMPTY_FETCH_HANDLER:
+        (*out)->fetch_handler_type =
+            blink::mojom::ServiceWorkerFetchHandlerType::kEmptyFetchHandler;
+        break;
+        // TODO(crbug.com/1347319): implement other fetch_handler_type.
     }
   }
   (*out)->last_update_check = base::Time::FromDeltaSinceWindowsEpoch(
@@ -1811,10 +1810,15 @@ void ServiceWorkerDatabase::WriteRegistrationDataInBatch(
   if (data.has_fetch_handler()) {
     switch (registration.fetch_handler_type) {
       case blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable:
-        data.set_fetch_handler_type(
+        data.set_fetch_handler_skippable_type(
             ServiceWorkerRegistrationData::NOT_SKIPPABLE);
         break;
+      case blink::mojom::ServiceWorkerFetchHandlerType::kEmptyFetchHandler:
+        data.set_fetch_handler_skippable_type(
+            ServiceWorkerRegistrationData::SKIPPABLE_EMPTY_FETCH_HANDLER);
+        break;
       // TODO(crbug.com/1347319): implement other fetch_handler_type.
+      // TODO(crbug.com/1351246): remove default if possible.
       default:
         DCHECK(false) << "Unknown fetch_handler_type is used."
                       << registration.fetch_handler_type;

diff --git a/components/services/storage/service_worker/service_worker_database.h b/components/services/storage/service_worker/service_worker_database.h
@@ -420,6 +420,8 @@ class ServiceWorkerDatabase {
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerDatabaseTest, InvalidWebFeature);
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerDatabaseTest,
                            NoCrossOriginEmbedderPolicyValue);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerDatabaseTest, NoFetchHandlerType);
+  FRIEND_TEST_ALL_PREFIXES(ServiceWorkerDatabaseTest, FetchHandlerType);
 };
 
 }  // namespace storage

diff --git a/components/services/storage/service_worker/service_worker_database.proto b/components/services/storage/service_worker/service_worker_database.proto
@@ -42,9 +42,10 @@ message ServiceWorkerRegistrationData {
     NORMAL_FRAME = 0;
     FENCED_FRAME = 1;
   }
-  enum FetchHandlerType {
-    UNKNOWN_FETCH_HANDLER = 0;
+  enum FetchHandlerSkippableType {
+    reserved 0;  // Deprecated UNKNOWN_FETCH_HANDLER.
     NOT_SKIPPABLE = 1;
+    SKIPPABLE_EMPTY_FETCH_HANDLER = 2;
   }
 
   required int64 registration_id = 1;
@@ -58,8 +59,7 @@ message ServiceWorkerRegistrationData {
 
   required bool is_active = 5;
   required bool has_fetch_handler = 6;
-  optional FetchHandlerType fetch_handler_type = 22
-      [default = UNKNOWN_FETCH_HANDLER];
+  optional FetchHandlerSkippableType fetch_handler_skippable_type = 22;
 
   // Serialized by Time::FromDeltaSinceWindowsEpoch().
   required int64 last_update_check_time = 7;

diff --git a/components/services/storage/service_worker/service_worker_database_unittest.cc b/components/services/storage/service_worker/service_worker_database_unittest.cc
@@ -2464,7 +2464,8 @@ TEST(ServiceWorkerDatabaseTest, InvalidWebFeature) {
   data.set_version_id(1);
   data.set_is_active(true);
   data.set_has_fetch_handler(true);
-  data.set_fetch_handler_type(ServiceWorkerRegistrationData::NOT_SKIPPABLE);
+  data.set_fetch_handler_skippable_type(
+      ServiceWorkerRegistrationData::NOT_SKIPPABLE);
   data.set_last_update_check_time(
       base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
 
@@ -2585,7 +2586,8 @@ TEST(ServiceWorkerDatabaseTest, NoCrossOriginEmbedderPolicyValue) {
   data.set_version_id(1);
   data.set_is_active(true);
   data.set_has_fetch_handler(true);
-  data.set_fetch_handler_type(ServiceWorkerRegistrationData::NOT_SKIPPABLE);
+  data.set_fetch_handler_skippable_type(
+      ServiceWorkerRegistrationData::NOT_SKIPPABLE);
   data.set_last_update_check_time(
       base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
 
@@ -2656,4 +2658,147 @@ TEST(ServiceWorkerDatabaseTest, StorageKeyImplCanReadPreviousOriginImplDB) {
   EXPECT_FALSE(resources_list.empty());
 }
 
+TEST(ServiceWorkerDatabaseTest, NoFetchHandlerType) {
+  std::unique_ptr<ServiceWorkerDatabase> database(CreateDatabaseInMemory());
+
+  ServiceWorkerRegistrationData data;
+  data.set_registration_id(1);
+  data.set_scope_url("https://example.com");
+  data.set_script_url("https://example.com/sw");
+  data.set_version_id(1);
+  data.set_is_active(true);
+  data.set_last_update_check_time(
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
+
+  database->next_avail_registration_id_ = 2;
+  database->next_avail_version_id_ = 2;
+
+  blink::StorageKey key =
+      blink::StorageKey::CreateFromStringForTesting(data.scope_url());
+
+  {
+    // has_fetch_handler = true.
+    data.set_has_fetch_handler(true);
+
+    // Write the serialization.
+    std::string value;
+    ASSERT_TRUE(data.SerializeToString(&value));
+
+    // Parse the serialized data. The kNotSkippable if has_fetch_handler is true
+    // and no fetch_handler_type.
+    RegistrationDataPtr registration;
+    ASSERT_EQ(ServiceWorkerDatabase::Status::kOk,
+              database->ParseRegistrationData(value, key, &registration));
+    EXPECT_EQ(blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable,
+              registration->fetch_handler_type);
+  }
+
+  {
+    // has_fetch_handler = false.
+    data.set_has_fetch_handler(false);
+
+    // Write the serialization.
+    std::string value;
+    ASSERT_TRUE(data.SerializeToString(&value));
+
+    // Parse the serialized data. The kNoHandler if has_fetch_handler is
+    // false and no fetch_handler_type.
+    RegistrationDataPtr registration;
+    ASSERT_EQ(ServiceWorkerDatabase::Status::kOk,
+              database->ParseRegistrationData(value, key, &registration));
+    EXPECT_EQ(blink::mojom::ServiceWorkerFetchHandlerType::kNoHandler,
+              registration->fetch_handler_type);
+  }
+}
+
+TEST(ServiceWorkerDatabaseTest, FetchHandlerType) {
+  std::unique_ptr<ServiceWorkerDatabase> database(CreateDatabaseInMemory());
+
+  ServiceWorkerRegistrationData data;
+  data.set_registration_id(1);
+  data.set_scope_url("https://example.com");
+  data.set_script_url("https://example.com/sw");
+  data.set_version_id(1);
+  data.set_is_active(true);
+  data.set_has_fetch_handler(true);
+  data.set_last_update_check_time(
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
+
+  database->next_avail_registration_id_ = 2;
+  database->next_avail_version_id_ = 2;
+
+  blink::StorageKey key =
+      blink::StorageKey::CreateFromStringForTesting(data.scope_url());
+
+  {
+    data.set_fetch_handler_skippable_type(
+        ServiceWorkerRegistrationData::NOT_SKIPPABLE);
+    // Write the serialization.
+    std::string value;
+    ASSERT_TRUE(data.SerializeToString(&value));
+
+    RegistrationDataPtr registration;
+    ASSERT_EQ(ServiceWorkerDatabase::Status::kOk,
+              database->ParseRegistrationData(value, key, &registration));
+    EXPECT_EQ(blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable,
+              registration->fetch_handler_type);
+  }
+
+  {
+    data.set_fetch_handler_skippable_type(
+        ServiceWorkerRegistrationData::SKIPPABLE_EMPTY_FETCH_HANDLER);
+    // Write the serialization.
+    std::string value;
+    ASSERT_TRUE(data.SerializeToString(&value));
+
+    // Parse the serialized data. The policy is kNone if it's not set.
+    RegistrationDataPtr registration;
+    ASSERT_EQ(ServiceWorkerDatabase::Status::kOk,
+              database->ParseRegistrationData(value, key, &registration));
+    EXPECT_EQ(blink::mojom::ServiceWorkerFetchHandlerType::kEmptyFetchHandler,
+              registration->fetch_handler_type);
+  }
+}
+
+TEST(ServiceWorkerDatabaseTest, FetchHandlerTypeStoreRestore) {
+  auto store_and_restore =
+      [](blink::mojom::ServiceWorkerFetchHandlerType type) {
+        GURL origin("https://example.com");
+        RegistrationData data;
+        data.registration_id = 123;
+        data.scope = URL(origin, "/foo");
+        data.key = blink::StorageKey(url::Origin::Create(data.scope));
+        data.script = URL(origin, "/script.js");
+        data.version_id = 456;
+        data.fetch_handler_type = type;
+        data.resources_total_size_bytes = 100;
+        data.cross_origin_embedder_policy = CrossOriginEmbedderPolicyNone();
+        std::vector<ResourceRecordPtr> resources;
+        resources.push_back(CreateResource(1, data.script, 100));
+
+        // Store.
+        std::unique_ptr<ServiceWorkerDatabase> database(
+            CreateDatabaseInMemory());
+        ServiceWorkerDatabase::DeletedVersion deleted_version;
+        ASSERT_EQ(
+            ServiceWorkerDatabase::Status::kOk,
+            database->WriteRegistration(data, resources, &deleted_version));
+
+        // Restore.
+        std::vector<mojom::ServiceWorkerRegistrationDataPtr> registrations;
+        std::vector<std::vector<ResourceRecordPtr>> resources_list;
+        EXPECT_EQ(ServiceWorkerDatabase::Status::kOk,
+                  database->GetRegistrationsForStorageKey(
+                      blink::StorageKey(url::Origin::Create(origin)),
+                      &registrations, &resources_list));
+
+        // The data must not have been altered.
+        VerifyRegistrationData(data, *registrations[0]);
+      };
+  store_and_restore(blink::mojom::ServiceWorkerFetchHandlerType::kNoHandler);
+  store_and_restore(blink::mojom::ServiceWorkerFetchHandlerType::kNotSkippable);
+  store_and_restore(
+      blink::mojom::ServiceWorkerFetchHandlerType::kEmptyFetchHandler);
+}
+
 }  // namespace storage
diff --git a/content/browser/service_worker/service_worker_version_browsertest.cc b/content/browser/service_worker/service_worker_version_browsertest.cc
@@ -939,6 +939,8 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
             blink::ServiceWorkerStatusCode::kOk);
   EXPECT_EQ(ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
             version_->fetch_handler_existence());
+  EXPECT_EQ(ServiceWorkerVersion::FetchHandlerType::kNotSkippable,
+            version_->fetch_handler_type());
 }
 
 IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
@@ -948,6 +950,19 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
             blink::ServiceWorkerStatusCode::kOk);
   EXPECT_EQ(ServiceWorkerVersion::FetchHandlerExistence::DOES_NOT_EXIST,
             version_->fetch_handler_existence());
+  EXPECT_EQ(ServiceWorkerVersion::FetchHandlerType::kNoHandler,
+            version_->fetch_handler_type());
+}
+
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
+                       InstallEmptyFetchHandler) {
+  StartServerAndNavigateToSetup();
+  ASSERT_EQ(Install("/service_worker/empty_fetch_event.js"),
+            blink::ServiceWorkerStatusCode::kOk);
+  EXPECT_EQ(ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
+            version_->fetch_handler_existence());
+  EXPECT_EQ(ServiceWorkerVersion::FetchHandlerType::kEmptyFetchHandler,
+            version_->fetch_handler_type());
 }
 
 // Check that fetch event handler added in the install event should result in a

diff --git a/content/test/data/service_worker/empty_fetch_event.js b/content/test/data/service_worker/empty_fetch_event.js
@@ -0,0 +1,5 @@
+// Copyright 2022 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.
+
+self.addEventListener('fetch', () => {});
diff --git a/third_party/blink/public/mojom/service_worker/service_worker_fetch_handler_type.mojom b/third_party/blink/public/mojom/service_worker/service_worker_fetch_handler_type.mojom
@@ -9,4 +9,5 @@ module blink.mojom;
 enum ServiceWorkerFetchHandlerType {
   kNoHandler,
   kNotSkippable,
+  kEmptyFetchHandler,
 };
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
@@ -58,6 +58,7 @@
 #include "third_party/blink/public/platform/web_url.h"
 #include "third_party/blink/public/platform/web_v8_value_converter.h"
 #include "third_party/blink/renderer/bindings/core/v8/callback_promise_adapter.h"
+#include "third_party/blink/renderer/bindings/core/v8/js_based_event_listener.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
 #include "third_party/blink/renderer/bindings/core/v8/source_location.h"
@@ -2588,8 +2589,19 @@ ServiceWorkerGlobalScope::FetchHandlerType() {
   if (!elv) {
     return mojom::blink::ServiceWorkerFetchHandlerType::kNoHandler;
   }
-  // TODO(crbug.com/1347319): implement nop handler detection.
-  return mojom::blink::ServiceWorkerFetchHandlerType::kNotSkippable;
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  v8::HandleScope handle_scope(isolate);
+  // TODO(crbug.com/1349613): revisit the way to implement this.
+  // The following code returns kEmptyFetchHandler if all handlers are nop.
+  for (RegisteredEventListener& e : *elv) {
+    EventTarget* et = EventTarget::Create(ScriptController()->GetScriptState());
+    v8::Local<v8::Value> v =
+        To<JSBasedEventListener>(e.Callback())->GetEffectiveFunction(*et);
+    if (!v.As<v8::Function>()->Experimental_IsNopFunction()) {
+      return mojom::blink::ServiceWorkerFetchHandlerType::kNotSkippable;
+    }
+  }
+  return mojom::blink::ServiceWorkerFetchHandlerType::kEmptyFetchHandler;
 }
 
 }  // namespace blink