Bug 1469433 - Handle shutdown more gracefully. r=smaug
If we end up shutting down the content process while there's a request active
we'll end up "leaking" the request and asserting. This makes sure that we
let go of the PaymentRequest in that edge case. I can cause this by forceably
closing the payment request window on Linux without hitting OK or cancel.
MozReview-Commit-ID: 6XDYIcqNkC6
--- a/dom/payments/PaymentRequest.cpp
+++ b/dom/payments/PaymentRequest.cpp
@@ -1052,17 +1052,19 @@ PaymentRequest::RejectedCallback(JSConte
mUpdating = false;
AbortUpdate(NS_ERROR_DOM_ABORT_ERR, mDeferredShow);
mDeferredShow = false;
}
PaymentRequest::~PaymentRequest()
{
if (mIPC) {
- mIPC->MaybeDelete();
+ // If we're being destroyed, the PaymentRequestManager isn't holding any
+ // references to us and we can't be waiting for any replies.
+ mIPC->MaybeDelete(false);
}
}
JSObject*
PaymentRequest::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
{
return PaymentRequestBinding::Wrap(aCx, this, aGivenProto);
}
--- a/dom/payments/PaymentRequestManager.cpp
+++ b/dom/payments/PaymentRequestManager.cpp
@@ -304,16 +304,27 @@ PaymentRequestManager::NotifyRequestDone
MOZ_ASSERT(entry.Data() > 0);
uint32_t count = --entry.Data();
if (count == 0) {
entry.Remove();
}
}
+void
+PaymentRequestManager::RequestIPCOver(PaymentRequest* aRequest)
+{
+ // This must only be called from ActorDestroy or if we're sure we won't
+ // receive any more IPC for aRequest.
+ mActivePayments.Remove(aRequest);
+ if (aRequest == mShowingRequest) {
+ mShowingRequest = nullptr;
+ }
+}
+
already_AddRefed<PaymentRequestManager>
PaymentRequestManager::GetSingleton()
{
if (!gPaymentManager) {
gPaymentManager = new PaymentRequestManager();
ClearOnShutdown(&gPaymentManager);
}
RefPtr<PaymentRequestManager> manager = gPaymentManager;
--- a/dom/payments/PaymentRequestManager.h
+++ b/dom/payments/PaymentRequestManager.h
@@ -59,16 +59,20 @@ public:
nsresult RespondPayment(PaymentRequest* aRequest,
const IPCPaymentActionResponse& aResponse);
nsresult ChangeShippingAddress(PaymentRequest* aRequest,
const IPCPaymentAddress& aAddress);
nsresult ChangeShippingOption(PaymentRequest* aRequest,
const nsAString& aOption);
+ // Called to ensure that we don't "leak" aRequest if we shut down while it had
+ // an active request to the parent.
+ void RequestIPCOver(PaymentRequest* aRequest);
+
private:
PaymentRequestManager() = default;
~PaymentRequestManager()
{
MOZ_ASSERT(mActivePayments.Count() == 0);
}
PaymentRequestChild* GetPaymentChild(PaymentRequest* aRequest);
--- a/dom/payments/ipc/PaymentRequestChild.cpp
+++ b/dom/payments/ipc/PaymentRequestChild.cpp
@@ -81,43 +81,46 @@ PaymentRequestChild::RecvChangeShippingO
}
return IPC_OK();
}
void
PaymentRequestChild::ActorDestroy(ActorDestroyReason aWhy)
{
if (mRequest) {
- DetachFromRequest();
+ DetachFromRequest(true);
}
}
void
-PaymentRequestChild::MaybeDelete()
+PaymentRequestChild::MaybeDelete(bool aCanBeInManager)
{
if (mRequest) {
- DetachFromRequest();
+ DetachFromRequest(aCanBeInManager);
Send__delete__(this);
}
}
bool
PaymentRequestChild::SendRequestPayment(const IPCPaymentActionRequest& aAction)
{
return PPaymentRequestChild::SendRequestPayment(aAction);
}
void
-PaymentRequestChild::DetachFromRequest()
+PaymentRequestChild::DetachFromRequest(bool aCanBeInManager)
{
MOZ_ASSERT(mRequest);
- nsAutoString id;
- mRequest->GetInternalId(id);
- RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
- MOZ_ASSERT(manager);
+ if (aCanBeInManager) {
+ RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
+ MOZ_ASSERT(manager);
+
+ RefPtr<PaymentRequest> request(mRequest);
+ manager->RequestIPCOver(request);
+ }
mRequest->SetIPC(nullptr);
mRequest = nullptr;
}
} // end of namespace dom
} // end of namespace mozilla
--- a/dom/payments/ipc/PaymentRequestChild.h
+++ b/dom/payments/ipc/PaymentRequestChild.h
@@ -14,17 +14,17 @@ namespace dom {
class PaymentRequest;
class PaymentRequestChild final : public PPaymentRequestChild
{
public:
explicit PaymentRequestChild(PaymentRequest* aRequest);
- void MaybeDelete();
+ void MaybeDelete(bool aCanBeInManager);
nsresult RequestPayment(const IPCPaymentActionRequest& aAction);
protected:
mozilla::ipc::IPCResult
RecvRespondPayment(const IPCPaymentActionResponse& aResponse) override;
mozilla::ipc::IPCResult
@@ -36,16 +36,16 @@ protected:
const nsString& aOption) override;
void ActorDestroy(ActorDestroyReason aWhy) override;
private:
~PaymentRequestChild() = default;
bool SendRequestPayment(const IPCPaymentActionRequest& aAction);
- void DetachFromRequest();
+ void DetachFromRequest(bool aCanBeInManager);
PaymentRequest* MOZ_NON_OWNING_REF mRequest;
};
} // end of namespace dom
} // end of namespace mozilla
#endif