service worker: Remove instrumentation for redundant controller

Bug: 951571, 963626
Change-Id: I6d625c4a728d5cad402fc70c0878e5e46ee2dc71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1636655Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664623}

content/browser/service_worker/service_worker_provider_host.cc

@@ -11,7 +11,6 @@
 #include "base/debug/alias.h"
 #include "base/guid.h"
 #include "base/memory/ptr_util.h"
-#include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
@@ -316,10 +315,9 @@ bool ServiceWorkerProviderHost::IsContextSecureForServiceWorker() const {
 }
 
 ServiceWorkerVersion* ServiceWorkerProviderHost::controller() const {
-  // TODO(crbug.com/951571): Remove this instrumentation logic once the bug is
-  // debugged. Limit crash rate at 20%.
-  bool should_crash = base::RandInt(0, 9) < 2;
-  CheckControllerConsistency(should_crash);
+#if DCHECK_IS_ON()
+  CheckControllerConsistency(false);
+#endif  // DCHECK_IS_ON()
   return controller_.get();
 }
 
@@ -861,6 +859,7 @@ bool ServiceWorkerProviderHost::IsControllerDecided() const {
   return true;
 }
 
+#if DCHECK_IS_ON()
 void ServiceWorkerProviderHost::CheckControllerConsistency(
     bool should_crash) const {
   if (!controller_) {
@@ -898,6 +897,7 @@ void ServiceWorkerProviderHost::CheckControllerConsistency(
       break;
   }
 }
+#endif  // DCHECK_IS_ON()
 
 void ServiceWorkerProviderHost::Register(
     const GURL& script_url,

content/browser/service_worker/service_worker_provider_host.h

@@ -509,9 +509,9 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // controller has not yet been decided.
   bool IsControllerDecided() const;
 
-  // TODO(crbug.com/951571): Put this check function behind DCHECK_IS_ON() once
-  // we figured out the cause of invalid controller status.
+#if DCHECK_IS_ON()
   void CheckControllerConsistency(bool should_crash) const;
+#endif  // DCHECK_IS_ON()
 
   // Implements blink::mojom::ServiceWorkerContainerHost.
   void Register(const GURL& script_url,

content/browser/service_worker/service_worker_registration.cc

@@ -37,33 +37,6 @@ ServiceWorkerVersionInfo GetVersionInfo(ServiceWorkerVersion* version) {
   return version->GetInfo();
 }
 
-void CrashBecauseIsController(
-    scoped_refptr<ServiceWorkerVersion> activating_version,
-    scoped_refptr<ServiceWorkerVersion> exiting_version,
-    ServiceWorkerProviderHost* provider_host) {
-  std::string debug_log = base::StringPrintf(
-      "sw:%d/ex_regid:%" PRId64 "/ac_regid:%" PRId64 "/pr_id:%d/ca:%d",
-      activating_version->skip_waiting(), exiting_version->registration_id(),
-      activating_version->registration_id(), provider_host->provider_id(),
-      provider_host->IsSetControllerRegistrationAllowed());
-  DEBUG_ALIAS_FOR_CSTR(debug_log_copy, debug_log.c_str(), 128);
-  CHECK(false) << "A controllee has a controller which became redundant.";
-}
-
-void CrashBecauseHasControllee(
-    scoped_refptr<ServiceWorkerVersion> activating_version,
-    scoped_refptr<ServiceWorkerVersion> exiting_version) {
-  std::string debug_log = base::StringPrintf(
-      "sw:%d/ex_regid:%" PRId64 "/ac_regid:%" PRId64 "/cl_ids:",
-      activating_version->skip_waiting(), exiting_version->registration_id(),
-      activating_version->registration_id());
-  for (auto& pair : exiting_version->controllee_map()) {
-    debug_log += pair.second->client_uuid() + ",";
-  }
-  DEBUG_ALIAS_FOR_CSTR(debug_log_copy, debug_log.c_str(), 1024);
-  CHECK(false) << "Redundant service worker has controllee(s).";
-}
-
 }  // namespace
 
 ServiceWorkerRegistration::ServiceWorkerRegistration(
@@ -465,32 +438,6 @@ void ServiceWorkerRegistration::ActivateWaitingVersion(bool delay) {
       observer.OnSkippedWaiting(this);
   }
 
-  // TODO(crbug.com/951571): Remove this instrumentation logic once the bug is
-  // debugged.
-  if (exiting_version.get()) {
-    for (std::unique_ptr<ServiceWorkerContextCore::ProviderHostIterator> it =
-             context_->GetClientProviderHostIterator(
-                 scope_.GetOrigin(), false /* include_reserved_clients */);
-         !it->IsAtEnd(); it->Advance()) {
-      ServiceWorkerProviderHost* host = it->GetProviderHost();
-      if (!host->IsContextSecureForServiceWorker())
-        continue;
-      if (host->MatchRegistration() != this)
-        continue;
-      ServiceWorkerVersion* controller = host->controller();
-      if (!controller)
-        continue;
-      if (controller == exiting_version) {
-        CrashBecauseIsController(activating_version, exiting_version, host);
-        return;
-      }
-    }
-    if (exiting_version->HasControllee()) {
-      CrashBecauseHasControllee(activating_version, exiting_version);
-      return;
-    }
-  }
-
   // "10. Queue a task to fire an event named activate..."
   // The browser could be shutting down. To avoid spurious start worker
   // failures, wait a bit before continuing.