Cherry pick PR #1287: Remove thread_id from most watchdog client name…

…s (#1319) Refer to the original PR: youtube/cobalt#1287 We can have multiple service workers and dedicated workers at the same time, so we need to add "thread_id" in watchdog client name to make the name unique. However, we shouldn't need this for the other threads such as "MainWebModule". b/287302949 Co-authored-by: Sherry Zhou <[email protected]>
youtube · Aug 18, 2023 · ae1c315 · ae1c315
1 parent 8f2b7e1
commit ae1c315
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 70 deletions.
diff --git a/cobalt/h5vcc/h5vcc_storage.cc b/cobalt/h5vcc/h5vcc_storage.cc
@@ -25,7 +25,7 @@
 #include "cobalt/cache/cache.h"
 #include "cobalt/persistent_storage/persistent_settings.h"
 #include "cobalt/storage/storage_manager.h"
-#include "cobalt/worker/service_worker_consts.h"
+#include "cobalt/worker/worker_consts.h"
 #include "net/base/completion_once_callback.h"
 #include "net/disk_cache/cobalt/cobalt_backend_impl.h"
 #include "net/disk_cache/cobalt/resource_type.h"
@@ -419,7 +419,7 @@ void H5vccStorage::ClearServiceWorkerCache() {
                   kSbFileMaxPath);
   base::FilePath service_worker_file_path =
       base::FilePath(storage_dir.data())
-          .Append(worker::ServiceWorkerConsts::kSettingsJson);
+          .Append(worker::WorkerConsts::kSettingsJson);
   base::DeleteFile(service_worker_file_path, /*recursive=*/false);
 }
 

diff --git a/cobalt/web/agent.cc b/cobalt/web/agent.cc
@@ -43,6 +43,7 @@
 #include "cobalt/worker/service_worker_object.h"
 #include "cobalt/worker/service_worker_registration.h"
 #include "cobalt/worker/service_worker_registration_object.h"
+#include "cobalt/worker/worker_consts.h"
 
 namespace cobalt {
 namespace web {
@@ -547,7 +548,7 @@ void Agent::Stop() {
   }
 
   watchdog::Watchdog* watchdog = watchdog::Watchdog::GetInstance();
-  if (watchdog) {
+  if (watchdog && watchdog_registered_) {
     watchdog_registered_ = false;
     watchdog->Unregister(watchdog_name_);
   }
@@ -577,12 +578,15 @@ void Agent::Run(const Options& options, InitializeCallback initialize_callback,
   if (!thread_.StartWithOptions(thread_options)) return;
   DCHECK(message_loop());
 
+  // Registers service worker thread as a watchdog client.
   watchdog::Watchdog* watchdog = watchdog::Watchdog::GetInstance();
 
-  // Registers service worker thread as a watchdog client.
   if (watchdog) {
-    watchdog_name_ =
-        thread_.thread_name() + std::to_string(thread_.GetThreadId());
+    watchdog_name_ = thread_.thread_name();
+    if (watchdog_name_ == worker::WorkerConsts::kServiceWorkerName ||
+        watchdog_name_ == worker::WorkerConsts::kDedicatedWorkerName) {
+      watchdog_name_ += std::to_string(thread_.GetThreadId());
+    }
     watchdog_registered_ = true;
     watchdog->Register(watchdog_name_, watchdog_name_,
                        base::kApplicationStateStarted, kWatchdogTimeInterval,

diff --git a/cobalt/worker/BUILD.gn b/cobalt/worker/BUILD.gn
@@ -36,8 +36,6 @@ static_library("worker") {
     "navigation_preload_manager.h",
     "service_worker.cc",
     "service_worker.h",
-    "service_worker_consts.cc",
-    "service_worker_consts.h",
     "service_worker_container.cc",
     "service_worker_container.h",
     "service_worker_context.cc",
@@ -60,6 +58,8 @@ static_library("worker") {
     "window_client.h",
     "worker.cc",
     "worker.h",
+    "worker_consts.cc",
+    "worker_consts.h",
     "worker_global_scope.cc",
     "worker_global_scope.h",
     "worker_location.h",

diff --git a/cobalt/worker/dedicated_worker.cc b/cobalt/worker/dedicated_worker.cc
@@ -28,10 +28,6 @@
 namespace cobalt {
 namespace worker {
 
-namespace {
-const char kDedicatedWorkerName[] = "DedicatedWorker";
-}  // namespace
-
 DedicatedWorker::DedicatedWorker(script::EnvironmentSettings* settings,
                                  const std::string& scriptURL,
                                  script::ExceptionState* exception_state)
@@ -111,7 +107,7 @@ void DedicatedWorker::Initialize(script::ExceptionState* exception_state) {
     options.construction_location.file_path =
         environment_settings()->creation_url().spec();
   }
-  worker_.reset(new Worker(kDedicatedWorkerName, options));
+  worker_.reset(new Worker(WorkerConsts::kDedicatedWorkerName, options));
   // 10. Return worker.
 }
 

diff --git a/cobalt/worker/service_worker_jobs.cc b/cobalt/worker/service_worker_jobs.cc
@@ -325,8 +325,7 @@ void ServiceWorkerJobs::Register(Job* job) {
         PromiseErrorData(
             web::DOMException::kSecurityErr,
             base::StringPrintf(
-                ServiceWorkerConsts::
-                    kServiceWorkerRegisterScriptOriginNotSameError,
+                WorkerConsts::kServiceWorkerRegisterScriptOriginNotSameError,
                 job->script_url.spec().c_str(), job->referrer.spec().c_str())));
     // 2.2. Invoke Finish Job with job and abort these steps.
     FinishJob(job);
@@ -343,8 +342,7 @@ void ServiceWorkerJobs::Register(Job* job) {
         PromiseErrorData(
             web::DOMException::kSecurityErr,
             base::StringPrintf(
-                ServiceWorkerConsts::
-                    kServiceWorkerRegisterScopeOriginNotSameError,
+                WorkerConsts::kServiceWorkerRegisterScopeOriginNotSameError,
                 job->scope_url.spec().c_str(), job->referrer.spec().c_str())));
 
     // 3.2. Invoke Finish Job with job and abort these steps.
@@ -491,12 +489,11 @@ bool ServiceWorkerJobs::UpdateOnResponseStarted(
     if (content_type.empty()) {
       RejectJobPromise(
           state->job,
-          PromiseErrorData(
-              web::DOMException::kSecurityErr,
-              ServiceWorkerConsts::kServiceWorkerRegisterNoMIMEError));
+          PromiseErrorData(web::DOMException::kSecurityErr,
+                           WorkerConsts::kServiceWorkerRegisterNoMIMEError));
       return true;
     }
-    for (auto mime_type : ServiceWorkerConsts::kJavaScriptMimeTypes) {
+    for (auto mime_type : WorkerConsts::kJavaScriptMimeTypes) {
       if (net::MatchesMimeType(mime_type, content_type)) {
         mime_type_is_javascript = true;
         break;
@@ -511,16 +508,15 @@ bool ServiceWorkerJobs::UpdateOnResponseStarted(
         state->job,
         PromiseErrorData(
             web::DOMException::kSecurityErr,
-            base::StringPrintf(
-                ServiceWorkerConsts::kServiceWorkerRegisterBadMIMEError,
-                content_type.c_str())));
+            base::StringPrintf(WorkerConsts::kServiceWorkerRegisterBadMIMEError,
+                               content_type.c_str())));
     return true;
   }
   //   8.8.  Let serviceWorkerAllowed be the result of extracting header list
   //         values given `Service-Worker-Allowed` and response’s header list.
   std::string service_worker_allowed;
   bool service_worker_allowed_exists = headers->GetNormalizedHeader(
-      ServiceWorkerConsts::kServiceWorkerAllowed, &service_worker_allowed);
+      WorkerConsts::kServiceWorkerAllowed, &service_worker_allowed);
   //   8.9.  Set policyContainer to the result of creating a policy container
   //         from a fetch response given response.
   state->script_headers = headers;
@@ -564,11 +560,10 @@ bool ServiceWorkerJobs::UpdateOnResponseStarted(
     //   8.16.2. Asynchronously complete these steps with a network error.
     RejectJobPromise(
         state->job,
-        PromiseErrorData(
-            web::DOMException::kSecurityErr,
-            base::StringPrintf(
-                ServiceWorkerConsts::kServiceWorkerRegisterBadScopeError,
-                scope_string.c_str())));
+        PromiseErrorData(web::DOMException::kSecurityErr,
+                         base::StringPrintf(
+                             WorkerConsts::kServiceWorkerRegisterBadScopeError,
+                             scope_string.c_str())));
     return true;
   }
   return true;
@@ -722,7 +717,7 @@ void ServiceWorkerJobs::UpdateOnLoadingComplete(
 
   // 11. Let worker be a new service worker.
   ServiceWorkerObject::Options options(
-      "ServiceWorker", state->job->client->web_settings(),
+      WorkerConsts::kServiceWorkerName, state->job->client->web_settings(),
       state->job->client->network_module(), state->registration);
   options.web_options.platform_info = state->job->client->platform_info();
   options.web_options.service_worker_context =
@@ -1014,9 +1009,9 @@ void ServiceWorkerJobs::Unregister(Job* job) {
     // 1.1. Invoke Reject Job Promise with job and "SecurityError" DOMException.
     RejectJobPromise(
         job,
-        PromiseErrorData(web::DOMException::kSecurityErr,
-                         ServiceWorkerConsts::
-                             kServiceWorkerUnregisterScopeOriginNotSameError));
+        PromiseErrorData(
+            web::DOMException::kSecurityErr,
+            WorkerConsts::kServiceWorkerUnregisterScopeOriginNotSameError));
 
     // 1.2. Invoke Finish Job with job and abort these steps.
     FinishJob(job);

diff --git a/cobalt/worker/service_worker_persistent_settings.cc b/cobalt/worker/service_worker_persistent_settings.cc
@@ -30,11 +30,11 @@
 #include "cobalt/script/promise.h"
 #include "cobalt/script/script_value.h"
 #include "cobalt/web/cache_utils.h"
-#include "cobalt/worker/service_worker_consts.h"
 #include "cobalt/worker/service_worker_context.h"
 #include "cobalt/worker/service_worker_jobs.h"
 #include "cobalt/worker/service_worker_registration_object.h"
 #include "cobalt/worker/service_worker_update_via_cache.h"
+#include "cobalt/worker/worker_consts.h"
 #include "cobalt/worker/worker_global_scope.h"
 #include "net/base/completion_once_callback.h"
 #include "net/disk_cache/cobalt/resource_type.h"
@@ -88,7 +88,7 @@ ServiceWorkerPersistentSettings::ServiceWorkerPersistentSettings(
     const Options& options)
     : options_(options) {
   persistent_settings_.reset(new cobalt::persistent_storage::PersistentSettings(
-      ServiceWorkerConsts::kSettingsJson));
+      WorkerConsts::kSettingsJson));
   persistent_settings_->ValidatePersistentSettings();
   DCHECK(persistent_settings_);
 }
@@ -110,7 +110,7 @@ void ServiceWorkerPersistentSettings::ReadServiceWorkerRegistrationMapSettings(
         persistent_settings_->GetPersistentSettingAsDictionary(key_string);
     if (dict.empty()) {
       DLOG(INFO) << "Key: " << key_string << " does not exist in "
-                 << ServiceWorkerConsts::kSettingsJson;
+                 << WorkerConsts::kSettingsJson;
       continue;
     }
     if (!CheckPersistentValue(key_string, kSettingsStorageKeyKey, dict,

diff --git a/cobalt/worker/service_worker_consts.cc → cobalt/worker/worker_consts.cc b/cobalt/worker/service_worker_consts.cc → cobalt/worker/worker_consts.cc
@@ -12,41 +12,36 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "cobalt/worker/service_worker_consts.h"
+#include "cobalt/worker/worker_consts.h"
 
 namespace cobalt {
 namespace worker {
-const char ServiceWorkerConsts::kServiceWorkerRegisterBadMIMEError[] =
+const char WorkerConsts::kServiceWorkerRegisterBadMIMEError[] =
     "Service Worker Register failed: The script has an unsupported MIME type "
     "('%s').";
 
-const char ServiceWorkerConsts::kServiceWorkerRegisterNoMIMEError[] =
+const char WorkerConsts::kServiceWorkerRegisterNoMIMEError[] =
     "Service Worker Register failed: The script does not have a MIME type.";
 
-const char
-    ServiceWorkerConsts::kServiceWorkerRegisterScriptOriginNotSameError[] =
-        "Service Worker Register failed: Script URL ('%s') and referrer ('%s') "
-        "origin are not the same.";
+const char WorkerConsts::kServiceWorkerRegisterScriptOriginNotSameError[] =
+    "Service Worker Register failed: Script URL ('%s') and referrer ('%s') "
+    "origin are not the same.";
 
-const char
-    ServiceWorkerConsts::kServiceWorkerRegisterScopeOriginNotSameError[] =
-        "Service Worker Register failed: Scope URL ('%s') and referrer ('%s') "
-        "origin are not the same.";
+const char WorkerConsts::kServiceWorkerRegisterScopeOriginNotSameError[] =
+    "Service Worker Register failed: Scope URL ('%s') and referrer ('%s') "
+    "origin are not the same.";
 
-const char ServiceWorkerConsts::kServiceWorkerRegisterBadScopeError[] =
+const char WorkerConsts::kServiceWorkerRegisterBadScopeError[] =
     "Service Worker Register failed: Scope ('%s') is not allowed.";
 
-const char
-    ServiceWorkerConsts::kServiceWorkerUnregisterScopeOriginNotSameError[] =
-        "Service Worker Unregister failed: Scope origin does not match.";
+const char WorkerConsts::kServiceWorkerUnregisterScopeOriginNotSameError[] =
+    "Service Worker Unregister failed: Scope origin does not match.";
 
-const char ServiceWorkerConsts::kServiceWorkerAllowed[] =
-    "Service-Worker-Allowed";
+const char WorkerConsts::kServiceWorkerAllowed[] = "Service-Worker-Allowed";
 
-const char ServiceWorkerConsts::kSettingsJson[] =
-    "service_worker_settings.json";
+const char WorkerConsts::kSettingsJson[] = "service_worker_settings.json";
 
-const char* const ServiceWorkerConsts::kJavaScriptMimeTypes[16] = {
+const char* const WorkerConsts::kJavaScriptMimeTypes[16] = {
     "application/ecmascript",
     "application/javascript",
     "application/x-ecmascript",
@@ -63,5 +58,9 @@ const char* const ServiceWorkerConsts::kJavaScriptMimeTypes[16] = {
     "text/livescript",
     "text/x-ecmascript",
     "text/x-javascript"};
+
+const char WorkerConsts::kServiceWorkerName[] = "ServiceWorker";
+
+const char WorkerConsts::kDedicatedWorkerName[] = "DedicatedWorker";
 }  // namespace worker
 }  // namespace cobalt
diff --git a/cobalt/worker/service_worker_consts.h → cobalt/worker/worker_consts.h b/cobalt/worker/service_worker_consts.h → cobalt/worker/worker_consts.h
@@ -12,13 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifndef COBALT_WORKER_SERVICE_WORKER_CONSTS_H_
-#define COBALT_WORKER_SERVICE_WORKER_CONSTS_H_
+#ifndef COBALT_WORKER_WORKER_CONSTS_H_
+#define COBALT_WORKER_WORKER_CONSTS_H_
 
 namespace cobalt {
 namespace worker {
 
-struct ServiceWorkerConsts {
+struct WorkerConsts {
   // Constants for error messages.
   static const char kServiceWorkerRegisterBadMIMEError[];
   static const char kServiceWorkerRegisterNoMIMEError[];
@@ -34,9 +34,15 @@ struct ServiceWorkerConsts {
   // Array of JavaScript mime types, according to the MIME Sniffinc spec:
   //   https://mimesniff.spec.whatwg.org/#javascript-mime-type
   static const char* const kJavaScriptMimeTypes[16];
+
+  // The name of a Service Worker thread.
+  static const char kServiceWorkerName[];
+
+  // The name of a Dedicated Worker thread.
+  static const char kDedicatedWorkerName[];
 };
 
 }  // namespace worker
 }  // namespace cobalt
 
-#endif  // COBALT_WORKER_SERVICE_WORKER_CONSTS_H_
+#endif  // COBALT_WORKER_WORKER_CONSTS_H_
diff --git a/cobalt/worker/worker_global_scope.cc b/cobalt/worker/worker_global_scope.cc
@@ -31,8 +31,8 @@
 #include "cobalt/web/user_agent_platform_info.h"
 #include "cobalt/web/window_or_worker_global_scope.h"
 #include "cobalt/web/window_timers.h"
-#include "cobalt/worker/service_worker_consts.h"
 #include "cobalt/worker/service_worker_object.h"
+#include "cobalt/worker/worker_consts.h"
 #include "cobalt/worker/worker_location.h"
 #include "cobalt/worker/worker_navigator.h"
 #include "net/base/mime_util.h"
@@ -169,21 +169,21 @@ class ScriptLoader : public base::MessageLoop::DestructionObserver {
     std::string content_type;
     bool mime_type_is_javascript = false;
     if (headers->GetNormalizedHeader("Content-type", &content_type)) {
-      for (auto mime_type : ServiceWorkerConsts::kJavaScriptMimeTypes) {
+      for (auto mime_type : WorkerConsts::kJavaScriptMimeTypes) {
         if (net::MatchesMimeType(mime_type, content_type)) {
           mime_type_is_javascript = true;
           break;
         }
       }
     }
     if (content_type.empty()) {
-      error->reset(new std::string(base::StringPrintf(
-          ServiceWorkerConsts::kServiceWorkerRegisterNoMIMEError,
-          content_type.c_str())));
+      error->reset(new std::string(
+          base::StringPrintf(WorkerConsts::kServiceWorkerRegisterNoMIMEError,
+                             content_type.c_str())));
     } else if (!mime_type_is_javascript) {
-      error->reset(new std::string(base::StringPrintf(
-          ServiceWorkerConsts::kServiceWorkerRegisterBadMIMEError,
-          content_type.c_str())));
+      error->reset(new std::string(
+          base::StringPrintf(WorkerConsts::kServiceWorkerRegisterBadMIMEError,
+                             content_type.c_str())));
     }
     return true;
   }

diff --git a/cobalt/worker/worker_global_scope.h b/cobalt/worker/worker_global_scope.h
@@ -35,7 +35,7 @@
 #include "cobalt/web/user_agent_platform_info.h"
 #include "cobalt/web/window_or_worker_global_scope.h"
 #include "cobalt/web/window_timers.h"
-#include "cobalt/worker/service_worker_consts.h"
+#include "cobalt/worker/worker_consts.h"
 #include "cobalt/worker/worker_location.h"
 #include "cobalt/worker/worker_navigator.h"
 #include "net/http/http_response_headers.h"