Add a timeout if the page doesn't call complete() in a timely fashion.

ios/chrome/browser/payments/resources/payment_request_manager.js

Index: ios/chrome/browser/payments/resources/payment_request_manager.js
diff --git a/ios/chrome/browser/payments/resources/payment_request_manager.js b/ios/chrome/browser/payments/resources/payment_request_manager.js
index 8ac217d94c94dbe41099754cd395eac47bfae455..b97cafba411b13df767001dfbf8c9334885bae90 100644
--- a/ios/chrome/browser/payments/resources/payment_request_manager.js
+++ b/ios/chrome/browser/payments/resources/payment_request_manager.js
@@ -23,7 +23,7 @@ __gCrWeb.paymentRequestManager = {
 
   /**
    * The pending PaymentResponse, if any. Used by the app side to invoke the
-   * associated resolve or reject function.
+   * associated resolve function.
    * @type {window.PaymentResponse}
    */
   pendingResponse: null
@@ -187,6 +187,7 @@ window.PaymentRequest.prototype.resolve = function(paymentResponseData) {
   this.resolve_ = null;
   this.reject_ = null;
   __gCrWeb['paymentRequestManager'].pendingRequest = null;
+  __gCrWeb['paymentRequestManager'].pendingResponse = paymentResponse;
 }
 
 /**
@@ -324,13 +325,6 @@ window.PaymentResponse = function(methodName, details) {
    * @private
    */
   this.resolve_ = null;
-
-  /**
-   * The pending reject function provided to the Promise returned by complete().
-   * @type {?function(string)}
-   * @private
-   */
-  this.reject_ = null;
 };
 
 /**
@@ -340,12 +334,6 @@ window.PaymentResponse = function(methodName, details) {
  *     been closed.
  */
 window.PaymentResponse.prototype.complete = function(success) {
-  if (__gCrWeb['paymentRequestManager'].pendingResponse) {

[Moe, 2017/01/16 21:52:58]

Should we have a check that __gCrWeb['paymentRequestManager'].pendingResponse
isn't null in case the page makes subsequent calls to complete()?

[Justin Donnelly, 2017/01/18 19:36:21]

I don't think that's possible, given that the only way to get a PaymentResponse
is by calling show()  and having the promise be resolved which sets
pendingResponse. I do think that it would be good to have a check here to make
sure the page isn't calling complete more than once. Since you're blocked on me
landing this CL, maybe you could add a variable that's set and checked here in
your CL?

[Moe, 2017/01/18 20:51:47]

Makes sense.

-    throw new Error(
-        'Internal PaymentRequest error: A response is already pending.');
-  }
-  __gCrWeb['paymentRequestManager'].pendingResponse = this;
-
   var message = {
     'command': 'paymentRequest.responseComplete'
   };
@@ -354,7 +342,9 @@ window.PaymentResponse.prototype.complete = function(success) {
   var self = this;
   return new Promise(function(resolve, reject) {
     self.resolve_ = resolve;
-    self.reject_ = reject;
+    // Any reject function provided is ignored because the spec includes no
+    // provision for rejecting the response promise. User agents are directed to
+    // always resolve the promise.

[Moe, 2017/01/16 21:52:58]

Does that mean we should resolve event if reject is called? or do nothing?
Also, the comment is a bit confusing. The reject function isn't really provided,
it is called on the promise.

[Justin Donnelly, 2017/01/18 19:36:21]

Only the user agent can call reject and we never do that. I generally find
Promises to be a great tool, but confusing. I always have to walk through what's
happening to make sure I'm getting it straight. But a reject function *is*
provided. Here's an example of how the promise API is used in a page:

    request.show()
        .then(function(paymentResponse) {
            instrumentResponse.complete('success');
        })
        .catch(function(err) {
          alert(err);
        });

The page calls show() which returns a Promise. The page calls then() on the
Promise and provides a function as an argument. That function is the resolver.
Likewise, what my comment refers to as the "reject function" is the anonymous
function provided by the page as an argument to catch(). We can ignore that
function because we (the user agent) are never directed by the spec to reject
the promise (i.e. call the reject function).

What always confuses me about this is that there's plumbing that seems slightly
magic that takes the arguments to then() and catch() and supplies them to me,
the creator of the Promise, via the anonymous function that I give to the
Promise constructor.

[Moe, 2017/01/18 20:51:47]

Thanks for the explanation. I was confused about who is providing and therefore
resolving/rejecting the promise. 

   });
 };
 
@@ -362,31 +352,16 @@ window.PaymentResponse.prototype.complete = function(success) {
  * Resolves the pending Promise. Should only be called by the app-side logic.
  */
 window.PaymentResponse.prototype.resolve = function() {
+  // If the page has not yet provided a resolve function, do nothing. This can

[Moe, 2017/01/16 21:52:58]

I also find this comment confusing. The page doesn't really provide a resolve
function, it resolves the promise returned by the complete() function. Could you
change this to something similar to:
"We can reach here if the UI times out while waiting for the page to call
complete(). In that case this.resolve_ is null. If so, do nothing and return."

[Justin Donnelly, 2017/01/18 19:36:21]

Similar comments as above. The page can call complete() but that doesn't
necessarily mean we'd have a value for resolve_. That value is provided by the
call to then() on the Promise returned by complete().

+  // happen in the case where the UI times out while waiting for the page to
+  // call complete().
   if (!this.resolve_) {
-    throw new Error("Internal PaymentRequest error: resolve function missing.");
+    return;
   }
 
   this.resolve_();
 
   this.resolve_ = null;
-  this.reject_ = null;
-  __gCrWeb['paymentRequestManager'].pendingResponse = null;
-}
-
-/**
- * Rejects the pending Promise. Should only be called by the app-side logic.
- * @param {!string} message An error message explaining why the Promise is being
- *     rejected.
- */
-window.PaymentResponse.prototype.reject = function(message) {
-  if (!this.reject_) {
-    throw new Error("Internal PaymentRequest error: reject function missing.");
-  }
-
-  this.reject_(message);
-
-  this.resolve_ = null;
-  this.reject_ = null;
   __gCrWeb['paymentRequestManager'].pendingResponse = null;
 }