From cb0f2f4a1065c3ba86854c6fd475321c3c3bf07e Mon Sep 17 00:00:00 2001 From: Andrew Sutherland Date: Thu, 17 Oct 2019 22:52:35 +0000 Subject: [PATCH] Bug 1587794 - Do not assume global exists. r=perry This is effectively a reversion of the change made in https://hg.mozilla.org/mozilla-central/rev/89c938649297#l1.39 when DOMMozPromiseRequestHolder was introduced. I've tried to add some comments to contextualize what's happening there and why it differs from other similar callsites. Longer term we might move to just deleting the underlying actor when we are disconnected. Those actors were written assuming an execution model where letting either end delete the actor would result in intentional process crashes when a message was received for a destroyed actor. That is no longer the case. Differential Revision: https://phabricator.services.mozilla.com/D49671 --HG-- extra : moz-landing-system : lando --- .../ServiceWorkerRegistration.cpp | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/dom/serviceworkers/ServiceWorkerRegistration.cpp b/dom/serviceworkers/ServiceWorkerRegistration.cpp index 6ce9dbd34beb..976dae99e480 100644 --- a/dom/serviceworkers/ServiceWorkerRegistration.cpp +++ b/dom/serviceworkers/ServiceWorkerRegistration.cpp @@ -240,7 +240,25 @@ already_AddRefed ServiceWorkerRegistration::Update(ErrorResult& aRv) { mInner->Update( [outer, self](const ServiceWorkerRegistrationDescriptor& aDesc) { nsIGlobalObject* global = self->GetParentObject(); - MOZ_DIAGNOSTIC_ASSERT(global); + // It's possible this binding was detached from the global. In cases + // where we use IPC with Promise callbacks, we use + // DOMMozPromiseRequestHolder in order to auto-disconnect the promise + // that would hold these callbacks. However in bug 1466681 we changed + // this call to use (synchronous) callbacks because the use of + // MozPromise introduced an additional runnable scheduling which made + // it very difficult to maintain ordering required by the standard. + // + // If we were to delete this actor at the time of DETH detaching, we + // would not need to do this check because the IPC callback of the + // RemoteServiceWorkerRegistrationImpl lambdas would never occur. + // However, its actors currently depend on asking the parent to delete + // the actor for us. Given relaxations in the IPC lifecyle, we could + // potentially issue a direct termination, but that requires additional + // evaluation. + if (!global) { + outer->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR); + return; + } RefPtr ref = global->GetOrCreateServiceWorkerRegistration(aDesc); if (!ref) {