bug 1501823 - Force to call PaymentUIService.showPayment() when PaymentRequest.show called with a promise parameter. r=baku
authorEden Chuang <echuang@mozilla.com>
Wed, 21 Nov 2018 11:03:21 +0100
changeset 504100 9671513ce7e7f7eaa22553fadb8e32c53dcc4686
parent 504099 298dbb64883121de67570112df6bb2c90d1b889a
child 504101 90a1e84edaa0bd93e949ae797bec18efa2419d76
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1501823
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1501823 - Force to call PaymentUIService.showPayment() when PaymentRequest.show called with a promise parameter. r=baku 1. Adding a new completeStatus 'initial' for nsIPaymentRequest.completeStatus to indicate the status of the showing PaymentRequest for Payment UI component. 2. Removing the PaymentRequest::mDeferredShow and calling PaymentRequestManager::showPayment() when PaymentRequest::Show() called with a detailsUpdate parameter to inform UI component to support better user experience.
dom/payments/MerchantValidationEvent.cpp
dom/payments/PaymentRequest.cpp
dom/payments/PaymentRequest.h
dom/payments/PaymentRequestManager.cpp
dom/payments/PaymentRequestManager.h
dom/payments/PaymentRequestService.cpp
dom/payments/PaymentRequestService.h
dom/payments/PaymentRequestUpdateEvent.cpp
dom/payments/ipc/PPaymentRequest.ipdl
--- a/dom/payments/MerchantValidationEvent.cpp
+++ b/dom/payments/MerchantValidationEvent.cpp
@@ -129,30 +129,30 @@ MerchantValidationEvent::ResolvedCallbac
   // If we eventually end up supporting merchant validation
   // we would validate `aValue` here, as per:
   // https://w3c.github.io/payment-request/#validate-merchant-s-details-algorithm
   //
   // Right now, MerchantValidationEvent is only implemented for standards
   // conformance, which is why at this point we throw a
   // NS_ERROR_DOM_NOT_SUPPORTED_ERR.
 
-  mRequest->AbortUpdate(NS_ERROR_DOM_NOT_SUPPORTED_ERR, false);
+  mRequest->AbortUpdate(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
   mRequest->SetUpdating(false);
 }
 
 void
 MerchantValidationEvent::RejectedCallback(JSContext* aCx,
                                           JS::Handle<JS::Value> aValue)
 {
   MOZ_ASSERT(mRequest);
   if (!mWaitForUpdate) {
     return;
   }
   mWaitForUpdate = false;
-  mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR, false);
+  mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR);
   mRequest->SetUpdating(false);
 }
 
 void
 MerchantValidationEvent::Complete(Promise& aPromise, ErrorResult& aRv)
 {
   if (!IsTrusted()) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
--- a/dom/payments/PaymentRequest.cpp
+++ b/dom/payments/PaymentRequest.cpp
@@ -659,17 +659,16 @@ PaymentRequest::CreatePaymentRequest(nsP
 }
 
 PaymentRequest::PaymentRequest(nsPIDOMWindowInner* aWindow, const nsAString& aInternalId)
   : DOMEventTargetHelper(aWindow)
   , mInternalId(aInternalId)
   , mShippingAddress(nullptr)
   , mUpdating(false)
   , mRequestShipping(false)
-  , mDeferredShow(false)
   , mUpdateError(NS_OK)
   , mState(eCreated)
   , mIPC(nullptr)
 {
   MOZ_ASSERT(aWindow);
   RegisterActivityObserver();
 }
 
@@ -751,17 +750,16 @@ PaymentRequest::Show(const Optional<Owni
     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);
@@ -853,19 +851,17 @@ PaymentRequest::Abort(ErrorResult& aRv)
   RefPtr<Promise> promise = Promise::Create(global, result);
   if (result.Failed()) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
   MOZ_ASSERT(manager);
-  // It's possible to be called between show and its promise resolving.
-  nsresult rv = manager->AbortPayment(this, mDeferredShow);
-  mDeferredShow = false;
+  nsresult rv = manager->AbortPayment(this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     aRv.Throw(NS_ERROR_FAILURE);
     return nullptr;
   }
 
   mAbortPromise = promise;
   return promise.forget();
 }
@@ -900,47 +896,45 @@ PaymentRequest::RespondAbortPayment(bool
     RejectShowPayment(NS_ERROR_DOM_ABORT_ERR);
   } else {
     mAbortPromise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
     mAbortPromise = nullptr;
   }
 }
 
 nsresult
-PaymentRequest::UpdatePayment(JSContext* aCx, const PaymentDetailsUpdate& aDetails,
-                              bool aDeferredShow)
+PaymentRequest::UpdatePayment(JSContext* aCx, const PaymentDetailsUpdate& aDetails)
 {
   NS_ENSURE_ARG_POINTER(aCx);
   if (mState != eInteractive) {
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
   RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
   if (NS_WARN_IF(!manager)) {
     return NS_ERROR_FAILURE;
   }
-  nsresult rv = manager->UpdatePayment(aCx, this, aDetails, mRequestShipping,
-                                       aDeferredShow);
+  nsresult rv = manager->UpdatePayment(aCx, this, aDetails, mRequestShipping);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
 void
-PaymentRequest::AbortUpdate(nsresult aRv, bool aDeferredShow)
+PaymentRequest::AbortUpdate(nsresult aRv)
 {
   MOZ_ASSERT(NS_FAILED(aRv));
 
   if (mState != eInteractive) {
     return;
   }
   // Close down any remaining user interface.
   RefPtr<PaymentRequestManager> manager = PaymentRequestManager::GetSingleton();
   MOZ_ASSERT(manager);
-  nsresult rv = manager->AbortPayment(this, aDeferredShow);
+  nsresult rv = manager->AbortPayment(this);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
   // Remember update error |aRv| and do the following steps in RespondShowPayment.
   // 1. Set target.state to closed
   // 2. Reject the promise target.acceptPromise with exception "aRv"
   // 3. Abort the algorithm with update error
@@ -1133,43 +1127,39 @@ PaymentRequest::ResolvedCallback(JSConte
   mUpdating = false;
   if (NS_WARN_IF(!aValue.isObject())) {
     return;
   }
 
   // Converting value to a PaymentDetailsUpdate dictionary
   PaymentDetailsUpdate details;
   if (!details.Init(aCx, aValue)) {
-    AbortUpdate(NS_ERROR_DOM_TYPE_ERR, mDeferredShow);
+    AbortUpdate(NS_ERROR_DOM_TYPE_ERR);
     JS_ClearPendingException(aCx);
     return;
   }
 
   nsresult rv = IsValidDetailsUpdate(details, mRequestShipping);
   if (NS_FAILED(rv)) {
-    AbortUpdate(rv, mDeferredShow);
+    AbortUpdate(rv);
     return;
   }
 
   // Update the PaymentRequest with the new details
-  if (NS_FAILED(UpdatePayment(aCx, details, mDeferredShow))) {
-    AbortUpdate(NS_ERROR_DOM_ABORT_ERR, mDeferredShow);
+  if (NS_FAILED(UpdatePayment(aCx, details))) {
+    AbortUpdate(NS_ERROR_DOM_ABORT_ERR);
     return;
   }
-
-  mDeferredShow = false;
 }
 
 void
 PaymentRequest::RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue)
 {
-  MOZ_ASSERT(mDeferredShow);
   mUpdating = false;
-  AbortUpdate(NS_ERROR_DOM_ABORT_ERR, mDeferredShow);
-  mDeferredShow = false;
+  AbortUpdate(NS_ERROR_DOM_ABORT_ERR);
 }
 
 void
 PaymentRequest::RegisterActivityObserver()
 {
   if (nsPIDOMWindowInner* window = GetOwner()) {
     nsCOMPtr<nsIDocument> doc = window->GetExtantDoc();
     if (doc) {
--- a/dom/payments/PaymentRequest.h
+++ b/dom/payments/PaymentRequest.h
@@ -138,19 +138,18 @@ public:
 
   void SetShippingOption(const nsAString& aShippingOption);
   void GetShippingOption(nsAString& aRetVal) const;
   void GetOptions(PaymentOptions& aRetVal) const;
   void SetOptions(const PaymentOptions& aOptions);
   nsresult UpdateShippingOption(const nsAString& aShippingOption);
 
   nsresult UpdatePayment(JSContext* aCx,
-                         const PaymentDetailsUpdate& aDetails,
-                         bool aDeferredShow);
-  void AbortUpdate(nsresult aRv, bool aDeferredShow);
+                         const PaymentDetailsUpdate& aDetails);
+  void AbortUpdate(nsresult aRv);
 
   void SetShippingType(const Nullable<PaymentShippingType>& aShippingType);
   Nullable<PaymentShippingType> GetShippingType() const;
 
   inline void ShippingWasRequested() { mRequestShipping = true; }
 
   void ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override;
   void RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override;
@@ -205,20 +204,16 @@ protected:
   // "true" when there is a pending updateWith() call to update the payment
   // request and "false" otherwise.
   bool mUpdating;
 
   // Whether shipping was requested. This models [[options]].requestShipping,
   // but we don't actually store the full [[options]] internal slot.
   bool mRequestShipping;
 
-  // True if the user passed a promise to show, causing us to defer telling the
-  // front end about it.
-  bool mDeferredShow;
-
   // The error is set in AbortUpdate(). The value is NS_OK by default.
   nsresult mUpdateError;
 
   enum
   {
     eUnknown,
     eCreated,
     eInteractive,
--- a/dom/payments/PaymentRequestManager.cpp
+++ b/dom/payments/PaymentRequestManager.cpp
@@ -532,43 +532,35 @@ PaymentRequestManager::CreatePayment(JSC
 }
 
 nsresult
 PaymentRequestManager::CanMakePayment(PaymentRequest* aRequest)
 {
   nsAutoString requestId;
   aRequest->GetInternalId(requestId);
   IPCPaymentCanMakeActionRequest action(requestId);
-
   return SendRequestPayment(aRequest, action);
 }
 
 nsresult
 PaymentRequestManager::ShowPayment(PaymentRequest* aRequest)
 {
-  nsresult rv = NS_OK;
-  if (!aRequest->IsUpdating()) {
-    nsAutoString requestId;
-    aRequest->GetInternalId(requestId);
-    IPCPaymentShowActionRequest action(requestId);
-    rv = SendRequestPayment(aRequest, action);
-  }
-  return rv;
+  nsAutoString requestId;
+  aRequest->GetInternalId(requestId);
+  IPCPaymentShowActionRequest action(requestId, aRequest->IsUpdating());
+  return SendRequestPayment(aRequest, action);
 }
 
 nsresult
-PaymentRequestManager::AbortPayment(PaymentRequest* aRequest, bool aDeferredShow)
+PaymentRequestManager::AbortPayment(PaymentRequest* aRequest)
 {
   nsAutoString requestId;
   aRequest->GetInternalId(requestId);
   IPCPaymentAbortActionRequest action(requestId);
-
-  // If aDeferredShow is true, then show was called with a promise that was
-  // rejected. In that case, we need to remember that we called show earlier.
-  return SendRequestPayment(aRequest, action, aDeferredShow);
+  return SendRequestPayment(aRequest, action);
 }
 
 nsresult
 PaymentRequestManager::CompletePayment(PaymentRequest* aRequest,
                                        const PaymentComplete& aComplete,
                                        bool aTimedOut)
 {
   nsString completeStatusString(NS_LITERAL_STRING("unknown"));
@@ -580,26 +572,24 @@ PaymentRequestManager::CompletePayment(P
       completeStatusString.AssignASCII(
         PaymentCompleteValues::strings[completeIndex].value);
     }
   }
 
   nsAutoString requestId;
   aRequest->GetInternalId(requestId);
   IPCPaymentCompleteActionRequest action(requestId, completeStatusString);
-
   return SendRequestPayment(aRequest, action, false);
 }
 
 nsresult
 PaymentRequestManager::UpdatePayment(JSContext* aCx,
                                      PaymentRequest* aRequest,
                                      const PaymentDetailsUpdate& aDetails,
-                                     bool aRequestShipping,
-                                     bool aDeferredShow)
+                                     bool aRequestShipping)
 {
   NS_ENSURE_ARG_POINTER(aCx);
   IPCPaymentDetails details;
   nsresult rv = ConvertDetailsUpdate(aCx, aDetails, details, aRequestShipping);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
@@ -608,20 +598,17 @@ PaymentRequestManager::UpdatePayment(JSC
   if (aRequestShipping) {
     GetSelectedShippingOption(aDetails, shippingOption);
     aRequest->SetShippingOption(shippingOption);
   }
 
   nsAutoString requestId;
   aRequest->GetInternalId(requestId);
   IPCPaymentUpdateActionRequest action(requestId, details, shippingOption);
-
-  // If aDeferredShow is true, then this call serves as the ShowUpdate call for
-  // this request.
-  return SendRequestPayment(aRequest, action, aDeferredShow);
+  return SendRequestPayment(aRequest, action, false);
 }
 
 nsresult
 PaymentRequestManager::ClosePayment(PaymentRequest* aRequest)
 {
   // for the case, the payment request is waiting for response from user.
   if (auto entry = mActivePayments.Lookup(aRequest)) {
     NotifyRequestDone(aRequest);
--- a/dom/payments/PaymentRequestManager.h
+++ b/dom/payments/PaymentRequestManager.h
@@ -43,25 +43,24 @@ public:
                 nsIPrincipal* aTopLevelPrincipal,
                 const Sequence<PaymentMethodData>& aMethodData,
                 const PaymentDetailsInit& aDetails,
                 const PaymentOptions& aOptions,
                 PaymentRequest** aRequest);
 
   nsresult CanMakePayment(PaymentRequest* aRequest);
   nsresult ShowPayment(PaymentRequest* aRequest);
-  nsresult AbortPayment(PaymentRequest* aRequest, bool aDeferredShow);
+  nsresult AbortPayment(PaymentRequest* aRequest);
   nsresult CompletePayment(PaymentRequest* aRequest,
                            const PaymentComplete& aComplete,
                            bool aTimedOut = false);
   nsresult UpdatePayment(JSContext* aCx,
                          PaymentRequest* aRequest,
                          const PaymentDetailsUpdate& aDetails,
-                         bool aRequestShipping,
-                         bool aDeferredShow);
+                         bool aRequestShipping);
   nsresult ClosePayment(PaymentRequest* aRequest);
   nsresult RetryPayment(JSContext* aCx,
                         PaymentRequest* aRequest,
                         const PaymentValidationErrors& aErrors);
 
   nsresult RespondPayment(PaymentRequest* aRequest,
                           const IPCPaymentActionResponse& aResponse);
   nsresult ChangeShippingAddress(PaymentRequest* aRequest,
--- a/dom/payments/PaymentRequestService.cpp
+++ b/dom/payments/PaymentRequestService.cpp
@@ -274,17 +274,18 @@ PaymentRequestService::RequestPayment(co
       }
       rv = RespondPayment(canMakeResponse.get());
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       break;
     }
     case IPCPaymentActionRequest::TIPCPaymentShowActionRequest: {
-      rv = ShowPayment(aRequestId);
+      const IPCPaymentShowActionRequest& action = aAction;
+      rv = ShowPayment(aRequestId, action.isUpdating());
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       break;
     }
     case IPCPaymentActionRequest::TIPCPaymentAbortActionRequest: {
       MOZ_ASSERT(request);
       request->SetState(payments::PaymentRequest::eInteractive);
@@ -311,31 +312,28 @@ PaymentRequestService::RequestPayment(co
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       MOZ_ASSERT(request);
       rv = request->UpdatePaymentDetails(details, action.shippingOption());
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
-
-      // mShowingRequest exists and it equals to the updated PaymentRequest
-      // Call UI::UpdatePayment
-      if (mShowingRequest && mShowingRequest == request) {
-        rv = LaunchUIAction(aRequestId, type);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return rv;
-        }
-      } else {
-        // mShowingRequest does not equal to the updated PaymentRequest, try to
-        // show the updated one.
-        rv = ShowPayment(aRequestId);
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return rv;
-        }
+      nsAutoString completeStatus;
+      rv = request->GetCompleteStatus(completeStatus);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
+      if (completeStatus.Equals(NS_LITERAL_STRING("initial"))) {
+        request->SetCompleteStatus(EmptyString());
+      }
+      MOZ_ASSERT(mShowingRequest && mShowingRequest == request);
+      rv = LaunchUIAction(aRequestId, type);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
       }
       break;
     }
     case IPCPaymentActionRequest::TIPCPaymentCloseActionRequest: {
       rv = LaunchUIAction(aRequestId, type);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
@@ -511,26 +509,29 @@ PaymentRequestService::CanMakePayment(co
   /*
    *  TODO: Check third party payment app support by traversing all
    *        registered third party payment apps.
    */
   return IsBasicCardPayment(aRequestId);
 }
 
 nsresult
-PaymentRequestService::ShowPayment(const nsAString& aRequestId)
+PaymentRequestService::ShowPayment(const nsAString& aRequestId, bool aIsUpdating)
 {
   nsresult rv;
   RefPtr<payments::PaymentRequest> request;
   rv = GetPaymentRequestById(aRequestId, getter_AddRefs(request));
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   MOZ_ASSERT(request);
   request->SetState(payments::PaymentRequest::eInteractive);
+  if (aIsUpdating) {
+    request->SetCompleteStatus(NS_LITERAL_STRING("initial"));
+  }
 
   if (mShowingRequest || !CanMakePayment(aRequestId)) {
     uint32_t responseStatus;
     if (mShowingRequest) {
       responseStatus = nsIPaymentActionResponse::PAYMENT_REJECTED;
     } else {
       responseStatus = nsIPaymentActionResponse::PAYMENT_NOTSUPPORTED;
     }
--- a/dom/payments/PaymentRequestService.h
+++ b/dom/payments/PaymentRequestService.h
@@ -45,17 +45,17 @@ private:
 
   nsresult
   LaunchUIAction(const nsAString& aRequestId, uint32_t aActionType);
 
   bool
   CanMakePayment(const nsAString& aRequestId);
 
   nsresult
-  ShowPayment(const nsAString& aRequestId);
+  ShowPayment(const nsAString& aRequestId, bool aIsUpdating);
 
   bool
   IsBasicCardPayment(const nsAString& aRequestId);
 
   FallibleTArray<RefPtr<payments::PaymentRequest>> mRequestQueue;
 
   nsCOMPtr<nsIPaymentUIService> mTestingUIService;
 
--- a/dom/payments/PaymentRequestUpdateEvent.cpp
+++ b/dom/payments/PaymentRequestUpdateEvent.cpp
@@ -59,47 +59,47 @@ PaymentRequestUpdateEvent::ResolvedCallb
 
   if (NS_WARN_IF(!aValue.isObject()) || !mWaitForUpdate) {
     return;
   }
 
   // Converting value to a PaymentDetailsUpdate dictionary
   PaymentDetailsUpdate details;
   if (!details.Init(aCx, aValue)) {
-    mRequest->AbortUpdate(NS_ERROR_TYPE_ERR, false);
+    mRequest->AbortUpdate(NS_ERROR_TYPE_ERR);
     JS_ClearPendingException(aCx);
     return;
   }
 
   // Validate and canonicalize the details
   // requestShipping must be true here. PaymentRequestUpdateEvent is only
   // dispatched when shippingAddress/shippingOption is changed, and it also means
   // Options.RequestShipping must be true while creating the corresponding
   // PaymentRequest.
   nsresult rv = mRequest->IsValidDetailsUpdate(details, true/*aRequestShipping*/);
   if (NS_FAILED(rv)) {
-    mRequest->AbortUpdate(rv, false);
+    mRequest->AbortUpdate(rv);
     return;
   }
 
   // Update the PaymentRequest with the new details
-  if (NS_FAILED(mRequest->UpdatePayment(aCx, details, false))) {
-    mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR, false);
+  if (NS_FAILED(mRequest->UpdatePayment(aCx, details))) {
+    mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR);
     return;
   }
   mWaitForUpdate = false;
   mRequest->SetUpdating(false);
 }
 
 void
 PaymentRequestUpdateEvent::RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue)
 {
   MOZ_ASSERT(mRequest);
 
-  mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR, false);
+  mRequest->AbortUpdate(NS_ERROR_DOM_ABORT_ERR);
   mWaitForUpdate = false;
   mRequest->SetUpdating(false);
 }
 
 void
 PaymentRequestUpdateEvent::UpdateWith(Promise& aPromise, ErrorResult& aRv)
 {
   if (!IsTrusted()) {
--- a/dom/payments/ipc/PPaymentRequest.ipdl
+++ b/dom/payments/ipc/PPaymentRequest.ipdl
@@ -82,16 +82,17 @@ struct IPCPaymentCreateActionRequest
 struct IPCPaymentCanMakeActionRequest
 {
   nsString requestId;
 };
 
 struct IPCPaymentShowActionRequest
 {
   nsString requestId;
+  bool isUpdating;
 };
 
 struct IPCPaymentAbortActionRequest
 {
   nsString requestId;
 };
 
 struct IPCPaymentCompleteActionRequest