service worker: Wrap Register callback so it runs on destruction.

This is a short-term workaround to get a browser test passing
when ServiceWorkerOnUI is enabled.

When ServiceWorkerContextCore is destroyed (i.e., browser shut down or
DeleteAndStartOver() is called), Mojo callbacks can be destroyed without
being run, which causes Mojo to DCHECK.

In the failing browser test
(PendingAppManagerImplBrowserTest.InstallChromeURLFails) the callback
for Register gets destroyed when ServiceWorkerJobCoordinator is
destroyed and destroys the ServiceWorkerRegisterJob that owns the
callback.

This test failure happens for Register job on when ServiceWorkerOnUI is
enabled, but it looks like this can happen for any Mojo callback that
goes through ServiceWorkerContextCore regardless of that feature.

Medium to longer term fixes are:
- Make the ServiceWorkerProviderHosts destruct before
  ServiceWorkerJobCoordinator and other objects that can hold callbacks
  like ServiceWorkerStorage. The provider hosts hold the connections to
  the renderer, and dropping the connections allows the callbacks to be
  dropped without DCHECK.
- However, that won't help the case of destruction due to
  DeleteAndStartOver, because currently the provider hosts and Mojo
  connections survive while the job coordinator and other objects
  are destroyed. We should drop Mojo connections on shutdown (see
  issue 877356).

Bug: 1002776, 877356, 824858
Change-Id: I8d99e7c37eb51eda7ca2baf5f32be25e1c399b79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1795645Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Auto-Submit: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695822}

content/browser/service_worker/service_worker_provider_host.cc

@@ -41,6 +41,7 @@
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/origin_util.h"
+#include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "net/base/url_util.h"
 #include "services/network/public/cpp/resource_request_body.h"
@@ -928,11 +929,25 @@ void ServiceWorkerProviderHost::Register(
   TRACE_EVENT_ASYNC_BEGIN2(
       "ServiceWorker", "ServiceWorkerProviderHost::Register", trace_id, "Scope",
       options->scope.spec(), "Script URL", script_url.spec());
+
+  // Wrap the callback with default invoke before passing it, since
+  // RegisterServiceWorker() can drop the callback on service worker
+  // context core shutdown (i.e., browser session shutdown or
+  // DeleteAndStartOver()) and a DCHECK would happen.
+  // TODO(crbug.com/1002776): Remove this wrapper and have the Mojo connections
+  // drop during shutdown, so the callback can be dropped without crash. Note
+  // that we currently would need to add this WrapCallback to *ALL* Mojo
+  // callbacks that go through ServiceWorkerContextCore or its members like
+  // ServiceWorkerStorage. We're only adding it to Register() now because a
+  // browser test fails without it.
+  auto wrapped_callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+      std::move(callback), blink::mojom::ServiceWorkerErrorType::kUnknown,
+      std::string(), nullptr);
   context_->RegisterServiceWorker(
       script_url, *options,
       base::BindOnce(&ServiceWorkerProviderHost::RegistrationComplete,
                      AsWeakPtr(), GURL(script_url), GURL(options->scope),
-                     std::move(callback), trace_id,
+                     std::move(wrapped_callback), trace_id,
                      mojo::GetBadMessageCallback()));
 }