Bug 1469433 - Match the spec's error reporting more closely. r=smaug
MozReview-Commit-ID: LI13cA0okeu
--- a/dom/payments/PaymentRequest.cpp
+++ b/dom/payments/PaymentRequest.cpp
@@ -644,33 +644,31 @@ already_AddRefed<Promise>
PaymentRequest::CanMakePayment(ErrorResult& aRv)
{
if (mState != eCreated) {
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
}
if (mResultPromise) {
+ // XXX This doesn't match the spec but does match Chromium.
aRv.Throw(NS_ERROR_DOM_NOT_ALLOWED_ERR);
return nullptr;
}
- nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
+ nsIGlobalObject* global = GetOwnerGlobal();
ErrorResult result;
RefPtr<Promise> promise = Promise::Create(global, result);
if (result.Failed()) {
aRv.Throw(NS_ERROR_FAILURE);
return nullptr;
}
RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
- if (NS_WARN_IF(!manager)) {
- aRv.Throw(NS_ERROR_FAILURE);
- return nullptr;
- }
+ MOZ_ASSERT(manager);
nsresult rv = manager->CanMakePayment(this);
if (NS_WARN_IF(NS_FAILED(rv))) {
promise->MaybeReject(NS_ERROR_FAILURE);
return promise.forget();
}
mResultPromise = promise;
return promise.forget();
}
@@ -682,48 +680,51 @@ PaymentRequest::RespondCanMakePayment(bo
mResultPromise->MaybeResolve(aResult);
mResultPromise = nullptr;
}
already_AddRefed<Promise>
PaymentRequest::Show(const Optional<OwningNonNull<Promise>>& aDetailsPromise,
ErrorResult& aRv)
{
- if (mState != eCreated) {
- aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
- return nullptr;
- }
-
if (!EventStateManager::IsHandlingUserInput()) {
aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
return nullptr;
}
- nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
+ nsIGlobalObject* global = GetOwnerGlobal();
+ nsCOMPtr<nsPIDOMWindowInner> win = do_QueryInterface(global);
+ MOZ_ASSERT(win);
+ nsIDocument* doc = win->GetExtantDoc();
+ if (!doc || !doc->IsCurrentActiveDocument()) {
+ aRv.Throw(NS_ERROR_DOM_ABORT_ERR);
+ return nullptr;
+ }
+
+ if (mState != eCreated) {
+ aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+ return nullptr;
+ }
+
ErrorResult result;
RefPtr<Promise> promise = Promise::Create(global, result);
if (result.Failed()) {
mState = eClosed;
aRv.Throw(NS_ERROR_FAILURE);
return nullptr;
}
- RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
- if (NS_WARN_IF(!manager)) {
- mState = eClosed;
- aRv.Throw(NS_ERROR_FAILURE);
- return nullptr;
- }
-
if (aDetailsPromise.WasPassed()) {
aDetailsPromise.Value().AppendNativeHandler(this);
mUpdating = true;
mDeferredShow = true;
}
+ RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
+ MOZ_ASSERT(manager);
nsresult rv = manager->ShowPayment(this);
if (NS_WARN_IF(NS_FAILED(rv))) {
if (rv == NS_ERROR_ABORT) {
promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
} else {
promise->MaybeReject(NS_ERROR_DOM_NOT_ALLOWED_ERR);
}
mState = eClosed;
@@ -789,35 +790,31 @@ already_AddRefed<Promise>
PaymentRequest::Abort(ErrorResult& aRv)
{
if (mState != eInteractive) {
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
}
if (mAbortPromise) {
- aRv.Throw(NS_ERROR_DOM_NOT_ALLOWED_ERR);
+ aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
return nullptr;
}
- nsCOMPtr<nsIGlobalObject> global = GetOwnerGlobal();
+ nsIGlobalObject* global = GetOwnerGlobal();
ErrorResult result;
RefPtr<Promise> promise = Promise::Create(global, result);
if (result.Failed()) {
aRv.Throw(NS_ERROR_FAILURE);
return nullptr;
}
RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
- if (NS_WARN_IF(!manager)) {
- aRv.Throw(NS_ERROR_FAILURE);
- return nullptr;
- }
-
- // It's possible for to call this between show and its promise resolving.
+ MOZ_ASSERT(manager);
+ // It's possible to be called between show and its promise resolving.
nsresult rv = manager->AbortPayment(this, mDeferredShow);
mDeferredShow = false;
if (NS_WARN_IF(NS_FAILED(rv))) {
aRv.Throw(NS_ERROR_FAILURE);
return nullptr;
}
mAbortPromise = promise;
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -597410,17 +597410,17 @@
"2280f0ef821cdc3093e10c2162d3756f5eeb78de",
"testharness"
],
"payment-request/payment-request-abort-method.https.html": [
"a9d811dc41487ba7a8e5d55319574364b93362aa",
"testharness"
],
"payment-request/payment-request-canmakepayment-method-manual.https.html": [
- "7531ea3b11f6e8de8cf71666b19861dbc5267cf7",
+ "11df3310832e043e533a11245b012b9de1c0bb26",
"manual"
],
"payment-request/payment-request-constructor-crash.https.html": [
"383d1c3f9505ee63d504bee87e13efa90ba49f3d",
"testharness"
],
"payment-request/payment-request-constructor.https.html": [
"511c4d25939d344f187fe4a8daa4a8ff926b613d",
--- a/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html
+++ b/testing/web-platform/tests/payment-request/payment-request-canmakepayment-method-manual.https.html
@@ -121,19 +121,22 @@ promise_test(async t => {
);
break;
}
}
}, `Optionally, at the user agent's discretion, return a promise rejected with a "NotAllowedError" DOMException.`);
function manualTest1(elem){
elem.disabled = true;
+
+ // NB: request.show has to be called outside of promise_test to ensure the
+ // user's click is still visible to PaymentRequest.show.
+ const request = new PaymentRequest(defaultMethods, defaultDetails);
+ const acceptPromise = request.show(); // Sets state to "interactive"
promise_test(async t => {
- const request = new PaymentRequest(defaultMethods, defaultDetails);
- const acceptPromise = request.show(); // Sets state to "interactive"
const canMakePaymentPromise = request.canMakePayment();
try {
const result = await canMakePaymentPromise;
assert_true(
false,
`canMakePaymentPromise should have thrown InvalidStateError`
);
} catch (err) {
@@ -143,21 +146,24 @@ function manualTest1(elem){
await promise_rejects(t, "AbortError", acceptPromise);
}
// The state should be "closed"
await promise_rejects(t, "InvalidStateError", request.canMakePayment());
}, elem.textContent.trim());
}
function manualTest2(elem){
- elem.disabled = true;
+ elem.disabled = true;
+
+ // See above for why it's important for these lines to be outside of
+ // promise_test.
+ const request = new PaymentRequest(defaultMethods, defaultDetails);
+ const acceptPromise = request.show(); // The state is now "interactive"
+ acceptPromise.catch(() => {}); // no-op, just to silence unhandled rejection in devtools.
promise_test(async t => {
- const request = new PaymentRequest(defaultMethods, defaultDetails);
- const acceptPromise = request.show(); // The state is now "interactive"
- acceptPromise.catch(() => {}); // no-op, just to silence unhandled rejection in devtools.
await request.abort(); // The state is now "closed"
await promise_rejects(t, "InvalidStateError", request.canMakePayment());
try {
const result = await request.canMakePayment();
assert_true(
false,
`should have thrown InvalidStateError, but instead returned "${result}"`
);