[Payments] Improve the closing of the PR dialog.

components/payments/payment_request.cc

Index: components/payments/payment_request.cc
diff --git a/components/payments/payment_request.cc b/components/payments/payment_request.cc
index 282b3fa38afad3d8ef83f6db57f9f05c532ca0ad..912fc258519d0f0023ed983743d339c5546c9a96 100644
--- a/components/payments/payment_request.cc
+++ b/components/payments/payment_request.cc
@@ -23,8 +23,13 @@ PaymentRequest::PaymentRequest(
       delegate_(std::move(delegate)),
       manager_(manager),
       binding_(this, std::move(request)) {
-  binding_.set_connection_error_handler(
-      base::Bind(&PaymentRequest::OnError, base::Unretained(this)));
+  // OnConnectionTerminated will be called when the Mojo pipe is closed. This
+  // will happen as a result of many renderer-side events (both successful and
+  // erroneous in nature).
+  // TODO(crbug.com/683636): Investigate using
+  // set_connection_error_with_reason_handler with Binding::CloseWithReason.
+  binding_.set_connection_error_handler(base::Bind(
+      &PaymentRequest::OnConnectionTerminated, base::Unretained(this)));
 }
 
 PaymentRequest::~PaymentRequest() {}
@@ -38,8 +43,7 @@ void PaymentRequest::Init(
   std::string error;
   if (!payments::validatePaymentDetails(details, &error)) {
     LOG(ERROR) << error;
-    OnError();
-    client_.reset();
+    OnConnectionTerminated();
     return;
   }
   client_ = std::move(client);
@@ -48,18 +52,45 @@ void PaymentRequest::Init(
 
 void PaymentRequest::Show() {
   if (!client_.is_bound() || !binding_.is_bound()) {
-    OnError();
+    LOG(ERROR) << "Attempted Show(), but binding(s) missing.";
+    OnConnectionTerminated();
     return;
   }
-  delegate_->ShowPaymentRequestDialog(this);
+  delegate_->ShowDialog(this);
 }
 
-void PaymentRequest::Cancel() {
+void PaymentRequest::Abort() {
+  // The API user has decided to abort. We return a successful abort message to
+  // the renderer, which closes the Mojo message pipe, which triggers
+  // PaymentRequest::OnConnectionTerminated, which destroys this object.
+  if (client_.is_bound())
+    client_->OnAbort(true /* aborted_successfully */);
+}
+
+void PaymentRequest::UserCancelled() {
+  // If |client_| is not bound, then the object is already being destroyed as
+  // a result of a renderer event.
+  if (!client_.is_bound())
+    return;
+
+  // This sends an error to the renderer, which informs the API user.
   client_->OnError(payments::mojom::PaymentErrorReason::USER_CANCEL);
+
+  // We close all bindings and ask to be destroyed.
+  client_.reset();
+  binding_.Close();
+  manager_->DestroyRequest(this);
 }
 
-void PaymentRequest::OnError() {
+void PaymentRequest::OnConnectionTerminated() {
+  // We are here because of a browser-side error, or likely as a result of the
+  // connection_error_handler on |binding_|, which can mean that the renderer
+  // has decided to close the pipe for various reasons (see all uses of
+  // PaymentRequest::clearResolversAndCloseMojoConnection() in Blink). We close
+  // the binding and the dialog, and ask to be deleted.
+  client_.reset();
   binding_.Close();
+  delegate_->CloseDialog();
   manager_->DestroyRequest(this);
 }
 
@@ -71,7 +102,6 @@ CurrencyFormatter* PaymentRequest::GetOrCreateCurrencyFormatter(
     currency_formatter_.reset(
         new CurrencyFormatter(currency_code, currency_system, locale_name));
   }
-
   return currency_formatter_.get();
 }