This bug was filed from the Socorro interface and is report bp-58f7bb00-bc2b-4aff-9e42-47f400181008. ============================================================= Seen while looking at Mac nightly crash stats - although only crash so far, I thought it was interesting to file since I don't see many Web Payments crashes. Top 10 frames of crashing thread: 0 XUL mozilla::dom::PaymentRequest::Show xpcom/base/nsCOMPtr.h:919 1 XUL mozilla::dom::PaymentRequest_Binding::show_promiseWrapper dom/bindings/PaymentRequestBinding.cpp:3181 2 XUL bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ConvertExceptionsToPromises> dom/bindings/BindingUtils.cpp:3315 3 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:461 4 XUL js::ForwardingProxyHandler::call const js/src/vm/Interpreter.cpp:626 5 XUL js::CrossCompartmentWrapper::call const js/src/proxy/CrossCompartmentWrapper.cpp:355 6 XUL js::Proxy::call js/src/proxy/Proxy.cpp:560 7 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:528 8 XUL Interpret js/src/vm/Interpreter.cpp:613 9 XUL js::RunScript js/src/vm/Interpreter.cpp:440 =============================================================

Eden: can you take a look at this?

The root cause is accessing the deleted PaymentRequest object. So JS code cannot find the binding PaymentRequest, then it gets crash.

In the current design, payment method response data is passed between processes through a simple nsString. It means a special encoder/decoder is needed for special response data, ex. BasicCardResponse, to serialize/deserialize into/from the nsString. However, when a token splitter, ':', ';' and '@', is used in response data, it makes the encoder/decoder can not work normally. It is hard to define a suitable token splitter set for the encoder/decoder. So instead of using an error-prone encoder/decoder, this patch defines a new IPC structure for response data.

Comment on attachment 9018296 [details] [diff] [review] Define a new IPC structure for response data Review of attachment 9018296 [details] [diff] [review]: ----------------------------------------------------------------- much better! Thanks. ::: dom/payments/PaymentRequestManager.cpp @@ +661,5 @@ > switch (response.status()) { > case nsIPaymentActionResponse::PAYMENT_ACCEPTED: { > rejectedReason = NS_OK; > + NS_ENSURE_SUCCESS(ConvertResponseData(aRequest->GetOwner(), > + response.data(), indentation

Comment on attachment 9028586 [details] [diff] [review] Reject PaymentRequest actions when its owner global object is null by navigating to other pages. Review of attachment 9028586 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/payments/PaymentRequest.cpp @@ +730,2 @@ > nsCOMPtr<nsPIDOMWindowInner> win = do_QueryInterface(global); > + if (!win) { This cannot happen.

Update patch according to reviewer's comment. Update patch with new C++ coding style change.

Pushed by ccoroiu@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2738bb9f6d54 Rejecting PaymentRequest::API calls when the PaymentRequest is not in fully active document. r=baku https://hg.mozilla.org/integration/mozilla-inbound/rev/7ba89c2d426d Mochitest for PaymentRequest in not fully active document. r=baku

https://hg.mozilla.org/mozilla-central/rev/2738bb9f6d54 https://hg.mozilla.org/mozilla-central/rev/7ba89c2d426d