service worker: Add instrumentation for ServiceWorkerProviderHost/ObjectHost crash.

Bug: 838410, 854993
Change-Id: I8babef268fe2dd299370904f30517aa1cacc0b72
Reviewed-on: https://chromium-review.googlesource.com/1132910Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574120}

content/browser/service_worker/service_worker_object_host.cc

@@ -18,6 +18,8 @@ namespace content {
 
 namespace {
 
+const int kMaxDebugLogSize = 256;
+
 using StatusCallback = base::OnceCallback<void(blink::ServiceWorkerStatusCode)>;
 using SetExtendableMessageEventSourceCallback =
     base::OnceCallback<bool(mojom::ExtendableMessageEventPtr*)>;
@@ -169,12 +171,15 @@ ServiceWorkerObjectHost::ServiceWorkerObjectHost(
     : context_(context),
       provider_host_(provider_host),
       provider_origin_(url::Origin::Create(provider_host->document_url())),
-      provider_id_(provider_host->provider_id()),
+      provider_type_(provider_host->provider_type()),
       version_(std::move(version)),
       weak_ptr_factory_(this) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(context_ && provider_host_ && version_);
   DCHECK(context_->GetLiveRegistration(version_->registration_id()));
   version_->AddObserver(this);
+  AddToDebugLog(base::StringPrintf("ObjCtor:prov=%p,type=%d,this=%p",
+                                   provider_host_, provider_type_, this));
   bindings_.set_connection_error_handler(base::BindRepeating(
       &ServiceWorkerObjectHost::OnConnectionError, base::Unretained(this)));
 }
@@ -182,13 +187,23 @@ ServiceWorkerObjectHost::ServiceWorkerObjectHost(
 ServiceWorkerObjectHost::~ServiceWorkerObjectHost() {
   // TODO(crbug.com/838410): These CHECKs are temporary debugging for the linked
   // bug.
+  AddToDebugLog(base::StringPrintf("ObjDtor:prov=%p,type=%d,this=%p",
+                                   provider_host_, provider_type_, this));
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  CHECK(!in_dtor_);
+  if (in_dtor_) {
+    CrashOnDoubleDelete();
+    return;
+  }
   in_dtor_ = true;
-
   version_->RemoveObserver(this);
 }
 
+void ServiceWorkerObjectHost::CrashOnDoubleDelete() {
+  std::string log = ServiceWorkerObjectHost::GetDebugLogString();
+  DEBUG_ALIAS_FOR_CSTR(debug_log, log.c_str(), 2048);
+  CHECK(false);
+}
+
 void ServiceWorkerObjectHost::OnVersionStateChanged(
     ServiceWorkerVersion* version) {
   DCHECK(version);
@@ -298,7 +313,34 @@ void ServiceWorkerObjectHost::OnConnectionError() {
   if (!bindings_.empty())
     return;
   // Will destroy |this|.
+  AddToDebugLog(base::StringPrintf("HostErr:this=%p", this));
   provider_host_->RemoveServiceWorkerObjectHost(version_->version_id());
 }
 
+// static
+ServiceWorkerObjectHost::DebugLog*
+ServiceWorkerObjectHost::GetDebugLogInstance() {
+  static base::NoDestructor<DebugLog> g_log;
+  return g_log.get();
+}
+
+// static
+void ServiceWorkerObjectHost::AddToDebugLog(const std::string& event) {
+  DebugLog* log = GetDebugLogInstance();
+  log->push_back(event);
+  if (log->size() > kMaxDebugLogSize)
+    log->pop_front();
+}
+
+// static
+std::string ServiceWorkerObjectHost::GetDebugLogString() {
+  DebugLog* log = GetDebugLogInstance();
+  std::string result;
+  // Traverse from the end so if the string gets truncated we still
+  // have the most recent events.
+  for (auto iter = log->rbegin(); iter != log->rend(); ++iter)
+    result += *iter + "\n";
+  return result;
+}
+
 }  // namespace content

content/browser/service_worker/service_worker_object_host.h

@@ -7,9 +7,11 @@
 
 #include <memory>
 
+#include "base/containers/circular_deque.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/no_destructor.h"
 #include "content/browser/service_worker/service_worker_version.h"
 #include "content/common/content_export.h"
 #include "content/common/service_worker/service_worker_types.h"
@@ -46,6 +48,8 @@ class CONTENT_EXPORT ServiceWorkerObjectHost
                           scoped_refptr<ServiceWorkerVersion> version);
   ~ServiceWorkerObjectHost() override;
 
+  void CrashOnDoubleDelete();
+
   // ServiceWorkerVersion::Observer overrides.
   void OnVersionStateChanged(ServiceWorkerVersion* version) override;
 
@@ -78,11 +82,16 @@ class CONTENT_EXPORT ServiceWorkerObjectHost
       blink::mojom::ServiceWorkerObjectAssociatedPtrInfo remote_object_ptr_info,
       blink::mojom::ServiceWorkerState sent_state);
 
-  int provider_id() const { return provider_id_; }
   ServiceWorkerVersion* version() { return version_.get(); }
 
   base::WeakPtr<ServiceWorkerObjectHost> AsWeakPtr();
 
+  // TODO(crbug.com/838410): Instrumentation for the linked bug.
+  using DebugLog = base::circular_deque<std::string>;
+  static DebugLog* GetDebugLogInstance();
+  static void AddToDebugLog(const std::string& event);
+  static std::string GetDebugLogString();
+
  private:
   friend class service_worker_object_host_unittest::ServiceWorkerObjectHostTest;
 
@@ -103,13 +112,13 @@ class CONTENT_EXPORT ServiceWorkerObjectHost
   base::WeakPtr<ServiceWorkerContextCore> context_;
   // |provider_host_| is valid throughout lifetime of |this| because it owns
   // |this|.
-  ServiceWorkerProviderHost* provider_host_;
+  ServiceWorkerProviderHost* const provider_host_;
   // The origin of the |provider_host_|. Note that this is const because once a
   // JavaScript ServiceWorker object is created for an execution context, we
   // don't expect that context to change origins and still hold on to the
   // object.
   const url::Origin provider_origin_;
-  const int provider_id_;
+  const blink::mojom::ServiceWorkerProviderType provider_type_;
   scoped_refptr<ServiceWorkerVersion> version_;
   // Typically both |bindings_| and |remote_objects_| contain only one Mojo
   // connection, corresponding to the content::WebServiceWorkerImpl in the

content/browser/service_worker/service_worker_provider_host.cc

@@ -265,6 +265,13 @@ ServiceWorkerProviderHost::~ServiceWorkerProviderHost() {
   CHECK(!in_dtor_);
   in_dtor_ = true;
 
+  if (!service_worker_object_hosts_.empty()) {
+    // Just log when hosts isn't empty. It'd be too noisy to record all provider
+    // ctor/dtor otherwise.
+    ServiceWorkerObjectHost::AddToDebugLog(
+        base::StringPrintf("ProvDtor:prov=%p,type=%d", this, info_.type));
+  }
+
   if (context_)
     context_->UnregisterProviderHostByClientID(client_uuid_);
 
@@ -527,7 +534,8 @@ void ServiceWorkerProviderHost::RemoveServiceWorkerRegistrationObjectHost(
 
 void ServiceWorkerProviderHost::RemoveServiceWorkerObjectHost(
     int64_t version_id) {
-  DCHECK(base::ContainsKey(service_worker_object_hosts_, version_id));
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  CHECK(base::ContainsKey(service_worker_object_hosts_, version_id));
   service_worker_object_hosts_.erase(version_id);
 }